Skip to content

Call pgconn_set_internal_encoding_index in all branches of pgconn_set_default_encoding#541

Merged
larskanis merged 1 commit intoged:masterfrom
drdrsh:mostafa_set-internal-encoding
Aug 31, 2023
Merged

Call pgconn_set_internal_encoding_index in all branches of pgconn_set_default_encoding#541
larskanis merged 1 commit intoged:masterfrom
drdrsh:mostafa_set-internal-encoding

Conversation

@drdrsh
Copy link
Copy Markdown
Contributor

@drdrsh drdrsh commented Aug 30, 2023

This PR reintroduces a call to pgconn_set_internal_encoding_index in pgconn_set_default_encoding. This call was removed in version 1.3.0 (specifically in this PR).

When Encoding.default_internal is set, ruby-pg sends a SET client_encoding TO ... query, if that query succeeds everything works great. If it fails and the connection is closed, well nothing breaks.

Now assuming Ruby and Postgres are both using UTF-8 encoding, if that query fails AND the connection remains open, we end up with desynchronized encoding between Ruby and Ruby-pg where Ruby's encoding is UTF-8, Postgres Encoding is UTF-8 but Ruby-pg falls back to SQL_ASCII (I think?) causing it to encode the incoming UTF-8 strings from Postgres into ASCII.

Version 1.2.3 handles encoding query failure gracefully. The following output shows this behavior in action in versions 1.2.3, 1.5.3, and a patched version of 1.5.3

[1.2.3 Response] [{"user_id"=>"1", "username"=>"™™™™™™"}, {"user_id"=>"2", "username"=>"™™™™™™"}]
[1.5.3 Response] [{"user_id"=>"1", "username"=>"\xE2\x84\xA2\xE2\x84\xA2\xE2\x84\xA2\xE2\x84\xA2\xE2\x84\xA2\xE2\x84\xA2"}, {"user_id"=>"2", "username"=>"\xE2\x84\xA2\xE2\x84\xA2\xE2\x84\xA2\xE2\x84\xA2\xE2\x84\xA2\xE2\x84\xA2"}]
[1.5.3-fix Response] [{"user_id"=>"1", "username"=>"™™™™™™"}, {"user_id"=>"2", "username"=>"™™™™™™"}]

This repo has a docker stack that reproduces that behavior and demonestrates the fix.

@drdrsh drdrsh changed the title Call pgconn_set_internal_encoding_index in all branches of pgconn_set… Call pgconn_set_internal_encoding_index in all branches of pgconn_set_default_encoding Aug 30, 2023
@drdrsh drdrsh marked this pull request as ready for review August 30, 2023 19:48
@larskanis larskanis merged commit 766c155 into ged:master Aug 31, 2023
@larskanis
Copy link
Copy Markdown
Collaborator

Thank you very much for providing patch and repro example! This is part of pg-1.5.4 which I just released.

@drdrsh
Copy link
Copy Markdown
Contributor Author

drdrsh commented Sep 1, 2023

Thank you for the quick turnaround, Lars!

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 this pull request may close these issues.

2 participants