Skip to content

Revert error packet parsing changes - #508

Merged
methane merged 1 commit into
PyMySQL:masterfrom
dianaclarke:issue/507
Sep 1, 2016
Merged

Revert error packet parsing changes#508
methane merged 1 commit into
PyMySQL:masterfrom
dianaclarke:issue/507

Conversation

@dianaclarke

Copy link
Copy Markdown
Contributor

Pull request for issue #507.

Revert the changes made in commit 02910b7 to the mysql error packet
parsing, as it didn't take the CLIENT_PROTOCOL_41 case into account.

It appears that the non-CLIENT_PROTOCOL_41 case was off by one as well
(sliced data on 4 vs 3).

I also added two new tests to cover those cases.

See: https://dev.mysql.com/doc/internals/en/packet-ERR_Packet.html

@dianaclarke

dianaclarke commented Aug 31, 2016

Copy link
Copy Markdown
Contributor Author

PS. working on the py26 and py3 cases now - fixed.

Comment thread pymysql/tests/test_err.py Outdated
class TestRaiseException(TestCase):

def test_raise_mysql_exception(self):
data = "\xff\x15\x04Access denied"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use b"" string.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup ;) On it, silly me.

Revert the changes made in commit 02910b7 to the mysql error packet
parsing, as it didn't take the CLIENT_PROTOCOL_41 case into account.

It appears that the non-CLIENT_PROTOCOL_41 case was off by one as well
(sliced data on 4 vs 3).

I also added two new tests to cover those cases.

See: https://dev.mysql.com/doc/internals/en/packet-ERR_Packet.html
@methane
methane merged commit 73e88b8 into PyMySQL:master Sep 1, 2016
@methane

methane commented Sep 1, 2016

Copy link
Copy Markdown
Member

Thanks!

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants