Bug #106252 Connector/J client hangs after prepare & execute process with old version server
Submitted: 24 Jan 2022 8:04 Modified: 1 Feb 2023 3:26
Reporter: Zhe Huang (OCA) Email Updates:
Status: Closed Impact on me:
None 
Category:Connector / J Severity:S3 (Non-critical)
Version:v6.0.4 - v8.0 OS:Any
Assigned to: CPU Architecture:Any
Tags: Contribution

[24 Jan 2022 8:04] Zhe Huang
Description:
In one word, this is a capability bug and it happens when:

1. Old version MySQL (<5.7.5) or other databases that are compatible with the MySQL protocol, and those that do not complete the WL#7766 [Deprecate the EOF packet].

2. The client sets useServerPrepStmts true.

3. The field num_columns in the response prepare_ok packet is bigger than zero.

4. The response contains too much data and will be split in the TCP layer into many segments. 

5. There is a delay between the two segments caused by the network latency or the Negale Algorithm.

The reason is that the connector does not check or handle the EOF packet in the ServerPreparedQuery function. So that, the EOF packet remains in the buffer after the function. Then the program goes on and invokes the execute function.

Most of the time, it is ok since the program will clear the buffer before reading the response from the execute the statement. 

However, if the second segment comes a little bit late, after the clearing stage, it would cause the client to hang indefinitely because the program treats the last few bytes from the segment as the header of the MySQL packet.

How to repeat:
It is difficult to repeat since one of the issues is in the network layer. 

However, I believe that it is easy to understand if we take a look at the source code, where the function has been reconstructed while the EOF handling part was missing.

Even though the checkEOF process has been invoked inside the read ColumnDefinition function, it is used to handle the SPSS situation and inside the loop. Thus the problem remains.

Also, a bug can be found during handling the EOF packet in the params part, where the code should be placed after the loop. 

I reproduced the code by changing the code in the server side, making it send a broken EOF packet which discarded the last four bytes. So that it looked like : 0x 05 00 00 05 fe. The rest part was 0x 00 00 02 00. 

Then the connector before v6.0.4 will hang and wait the rest part of the packet, which is an expected behaviour. However, the connector after v6.0.4 continues, which is abnormal.

Suggested fix:
To be submitted on GitHub.

Also I would suggest reconstructing the related code that extracts the checkEOF procedure into a function. If it gets approval I would add a pull request.
[8 Feb 2022 17:50] OCA Admin
Contribution submitted via Github - Fix bug#106252 
(*) Contribution by Zhe Huang (Github onlyacat, mysql-connector-j/pull/73#issuecomment-1032734465): I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

Contribution: git_patch_830108759.txt (text/plain), 1.80 KiB.

[17 Feb 2022 9:53] Zhe Huang
Guess it would help to locate the bug easier through the unit test :)

The test code will be submitted on Github.

The server version I used to test was 5.6.51.

In the test function a prepared statement with SQL `SELECT CONCAT(?, ?) as col1;` will be performed and the function `serverPrepare` will be invoked.

Af first, the `query.getParameterFields()[1]` would give a null, since the procedure during handling the Parameter Definition Block skips the first packet, which is supposed to deal with the EOF packet. 
Therefore, I placed the block after the handling process in the fix.

Then, the message sequence should be six: one description packet; two Parameter definition packets and one EOF packet in Parameter Definition Block; one Column definition packet and one EOF packet in Column Definition Block.
However, the test gives a result of 5. If we check the remaining packet in the buffer, it is the EOF packet, meaning the procedure does not handle the last EOF packet in the Column Definition Block. Therefore, I add the EOF handling process in the fix.
[17 Feb 2022 14:22] OCA Admin
Contribution submitted via Github - add a test `testEOFPacketForPrepareStmt` to help locate the bug#106252 
(*) Contribution by Zhe Huang (Github onlyacat, mysql-connector-j/pull/74#issuecomment-1042920783): I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

Contribution: git_patch_855259375.txt (text/plain), 2.51 KiB.

[16 Mar 2022 13:32] MySQL Verification Team
Thank you very much for your patch contribution, we appreciate it!
[1 Jul 2022 7:05] bossjie bossjie
Not sure what's the ETA/fix plan as I have a project that pending on this fix. thanks in advance
[19 Sep 2022 7:45] Zhe Huang
Hello, is there any progress in the issue or is there any possible workaround? Thanks!
[20 Dec 2022 19:56] Daniel So
Posted by developer:
 
Added the following entry to the Connector/J 8.0.32 changelog: 

"When working with MySQL Server 5.7.5 or earlier, if server-side prepared statements were used, Connector/J sometimes hung after the prepare phase for a statement. It was caused by Connector/J mistaking a metadata packet as an EOF packet and discarding it by mistake. With this fix, the mistake is avoided by improving the procedure for handling EOF packets. Thanks to Zhe Huang for contributing to the fix."
[31 Jan 2023 17:23] Björn Voigt
The patch breaks MySQL 5.5. See Bug #109864.
[1 Feb 2023 3:26] Zhe Huang
Took a look at the new version and found the reason.

In the official release, the code of my patch had been modified while mistaking the EOF packet during the Field Count Handling.

It can be found in src/main/core-impl/java/com/mysql/cj/ServerPreparedQuery.java,
line 164-166.

This `if` block is supposed to be placed inside the if (fieldCount > 0) condition sequence, discarding the EOF packet if necessary.

However, in the release, it had been moved outside of the Column Definition Block handling procedure. When the number of columns is zero, the client will try to read the packet inside the socket buffer, which is empty for now and forever.

This will affect all old version servers(<5.7.5) while preparing a statement with zero columns of info.
[1 Feb 2023 9:19] Daniël van Eeden
Direct link to the source on GitHub:

https://github.com/mysql/mysql-connector-j/blob/8.0.32/src/main/core-impl/java/com/mysql/c...
[1 Feb 2023 9:56] Björn Voigt
Handle skipPacket within the fieldCount condition

Attachment: skipPacket-within-fieldCount-condition.patch (text/x-patch), 1.05 KiB.

[1 Feb 2023 10:01] Björn Voigt
I can confirm that with the suggested 'if' block fix from Zhe Huang Connector/J works with MySQL 5.5.62, 5.7.41 and 8.0.32.

It is not the original patch from Zhe Huang, but a combination of the official commit from Filipe Silva and the partial revert from Zhe Huang.
[4 Feb 2023 0:50] Filipe Silva
Indeed, the if block was misplaced in the patch released originally.

Thank you all for pointing this out.