Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The behavior of the readResultSetHeaderPacket differs from that of the MySQL client #1478

Closed
ShenFeng312 opened this issue Sep 15, 2023 · 1 comment · Fixed by #1481
Closed

Comments

@ShenFeng312
Copy link
Contributor

ShenFeng312 commented Sep 15, 2023

// http://dev.mysql.com/doc/internals/en/com-query-response.html#packet-ProtocolText::Resultset
func (mc *okHandler) readResultSetHeaderPacket() (int, error) {
	// handleOkPacket replaces both values; other cases leave the values unchanged.
	mc.result.affectedRows = append(mc.result.affectedRows, 0)
	mc.result.insertIds = append(mc.result.insertIds, 0)

	data, err := mc.conn().readPacket()
	if err == nil {
		switch data[0] {

		case iOK:
			return 0, mc.handleOkPacket(data)

		case iERR:
			return 0, mc.conn().handleErrorPacket(data)

		case iLocalInFile:
			return 0, mc.handleInFileRequest(string(data[1:]))
		}

		// column count
		num, _, n := readLengthEncodedInteger(data)
		if n-len(data) == 0 {
			return int(num), nil
		}

		return 0, ErrMalformPkt
	}
	return 0, err
}

In Go projects, we check the packet's length. However, in the MySQL client and other language drivers, this is not done. They only extract the necessary content from the packet without checking its length. This approach provides the MySQL protocol with sufficient flexibility. Therefore, I believe we should remove the length check here.

		if n-len(data) == 0 {
			return int(num), nil
		}
@ShenFeng312
Copy link
Contributor Author

ShenFeng312 commented Sep 15, 2023

I attempted to add some additional content to this packet, and it threw the following exception:
image
However, I can obtain the correct results on other clients and drivers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant