Skip to content

test: cover encoded UTF-8 URI resolution - #200

Open
Poliklot wants to merge 1 commit into
fastify:mainfrom
Poliklot:test/encoded-uri-resolution
Open

test: cover encoded UTF-8 URI resolution#200
Poliklot wants to merge 1 commit into
fastify:mainfrom
Poliklot:test/encoded-uri-resolution

Conversation

@Poliklot

@Poliklot Poliklot commented Aug 6, 2026

Copy link
Copy Markdown

Summary

  • add regression coverage for #157, ensuring resolve() retains UTF-8 percent-encoded path segments instead of producing legacy unescape()-style Latin-1 output;
  • cover the corresponding JSON Schema reference fixture from sagold/json-schema-library#82, where uri-js had been used as a workaround;
  • lock in the current implementation's correct behaviour without adding a dependency or changing public API.

The commit credits the original report and fixture research:

Co-authored-by: Dmitry Arkhipov <2089893+ekzobrain@users.noreply.github.com>

Closes #157.

Validation

  • npm test
  • npm run lint
  • npm run test:typescript
  • npm run test:browser:chromium

Add coverage for the encoded Cyrillic reference from fastify#157 and the corresponding JSON Schema URI fixture from sagold/json-schema-library#82.

Co-authored-by: Dmitry Arkhipov <2089893+ekzobrain@users.noreply.github.com>
Signed-off-by: Poliklot <poliklot@vk.com>
@Poliklot
Poliklot marked this pull request as ready for review August 6, 2026 15:53
@Tony133
Tony133 requested a review from mcollina August 11, 2026 14:11

@mcollina mcollina 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.

lgtm

@mcollina mcollina 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.

I'm sorry but I don't understand this, how can we close that issue with just a test?

@Poliklot

Copy link
Copy Markdown
Author

@mcollina You’re absolutely right: this PR only adds regression coverage; it does not implement the functional fix.

I verified that the reported reproducer returns http://host/�� in v3.1.0, while v3.1.1 returns the correct http://host/%D1%8B%D1%8B. The underlying fix had already landed in 876ce79, which replaces the unescape() path handling with percent-encoding normalization.

So Closes #157 is misleading here: #200 should only reference the issue, while #157 can be closed separately as already fixed. Thanks for catching this.

@Tony133
Tony133 requested a review from mcollina August 18, 2026 09:57
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.

bug: Breaks encoded URI's with non-latin characters in browser

2 participants