Skip to content

fix: loading utf-8 file when locale encoding is set to ascii#28

Merged
steve-chavez merged 1 commit into
PostgREST:masterfrom
taimoorzaeem:fix/load-utf-8
Oct 9, 2025
Merged

fix: loading utf-8 file when locale encoding is set to ascii#28
steve-chavez merged 1 commit into
PostgREST:masterfrom
taimoorzaeem:fix/load-utf-8

Conversation

@taimoorzaeem

Copy link
Copy Markdown
Member

Configurator-pg fails to load UTF-8 encoded files when the locale settings are set to ASCII chars. This commit fixes this by using a safer and faster UTF-8 decoding when reading files.

See: PostgREST/postgrest#4386 (comment) for more info on this.

Comment thread tests/resources/err-import.cfg.err
Comment thread tests/Test.hs
tests =
[ testCase "read-simple" $ readTest "simple.cfg"
, testCase "read-pathological" $ readTest "pathological.cfg"
, testCase "read-utf-8-with-ascii-locale" $ readTestWithLocale "utf-8.cfg" E.ascii

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.

Q: Prior to the change, did this one fail with:

hGetContents: invalid argument (cannot decode byte sequence starting from ..

Same as PostgREST/postgrest#4379 (comment)?

@taimoorzaeem taimoorzaeem Oct 9, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, exactly! If I remove this change and re-run tests, I get:

read-utf-8-with-ascii-locale: [Failed]
ERROR: tests/resources/utf-8.cfg: hGetContents: invalid argument (cannot decode byte sequence starting from 195)

EDIT: A couple other tests slightly change in output, so I modified accordingly.

@taimoorzaeem taimoorzaeem marked this pull request as draft October 9, 2025 05:41
Configurator-pg fails to load UTF-8 encoded files when the locale
settings are set to ASCII chars. This commit fixes this by using a
safer and faster UTF-8 decoding when reading files.

See: PostgREST/postgrest#4386 (comment)
for more info on this.

Signed-off-by: Taimoor Zaeem <taimoorzaeem@gmail.com>
@taimoorzaeem taimoorzaeem marked this pull request as ready for review October 9, 2025 05:49

@steve-chavez steve-chavez left a comment

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.

Awesome work 🥇

@steve-chavez steve-chavez merged commit 975a9d4 into PostgREST:master Oct 9, 2025
7 checks passed
@steve-chavez

Copy link
Copy Markdown
Member

@laurenceisla I saw you did a release previously, could you make a new release for this fix? 🙏

@taimoorzaeem taimoorzaeem deleted the fix/load-utf-8 branch October 9, 2025 18:20
@laurenceisla

Copy link
Copy Markdown
Member

@laurenceisla I saw you did a release previously, could you make a new release for this fix? 🙏

Yup, will do.

@laurenceisla laurenceisla mentioned this pull request Oct 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants