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

Add test for content paths that have percent-encoded parts #115

Closed
lidel opened this issue Jul 26, 2023 · 0 comments · Fixed by #160
Closed

Add test for content paths that have percent-encoded parts #115

lidel opened this issue Jul 26, 2023 · 0 comments · Fixed by #160
Labels
test:coverage Improves the spec covered by this test suite

Comments

@lidel
Copy link
Member

lidel commented Jul 26, 2023

Found this edge case in ipfs-inactive/bifrost-gateway#160

Content Paths with special characters are usually encoded before sending to the gateway.
However there is an edge case where a content path has already encoded parts.
We do not want it to be mangled during transport OR things like subdomain redirects.

There should be a conformance test based on below example (we dont want to pull-in entire wikipedia, but its fine to reuse the single file from /I directory as-is).

Example

The original content path is /ipns/en.wikipedia-on-ipfs.org/I/Auditorio_de_Tenerife%2C_Santa_Cruz_de_Tenerife%2C_España%2C_2012-12-15%2C_DD_02.jpg.webp (note ñ in España%2C)

But the URL path used in HTTP request is percent-encoded:

curl -v "https://en-wikipedia--on--ipfs-org.ipns.dweb.link/I/Auditorio_de_Tenerife%252C_Santa_Cruz_de_Tenerife%252C_Espa%C3%B1a%252C_2012-12-15%252C_DD_02.jpg.webp"

Test with curl (incl. subdomain redirect):

curl -v -L "http://localhost:8081/ipns/en.wikipedia-on-ipfs.org/I/Auditorio_de_Tenerife%252C_Santa_Cruz_de_Tenerife%252C_Espa%C3%B1a%252C_2012-12-15%252C_DD_02.jpg.webp"

For now, I've added basic unit test in ipfs-inactive/bifrost-gateway@982535a#diff-84d756d48159dd2e742a98880c23ce6d22bd80bf878409ddcf1d4dc81bc114acR38, but we should have a conformance test for this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test:coverage Improves the spec covered by this test suite
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants