Skip to content

fix(gateway): question marks in url.Path when redirecting#313

Merged
lidel merged 2 commits into
mainfrom
issues/9882
Jun 2, 2023
Merged

fix(gateway): question marks in url.Path when redirecting#313
lidel merged 2 commits into
mainfrom
issues/9882

Conversation

@hacdias

@hacdias hacdias commented May 24, 2023

Copy link
Copy Markdown
Member

Fixes ipfs/kubo#9882, fixes ipfs/ipfs-companion#1201. I updated toSubdomainURL to use the url.URL struct from Go to build the final URL. The problem was that rest was the original, decoded, r.URL.Path and it contains a ?. When Go parses it, it'd consider it as a query element, rightly so.

Kubo PR: ipfs/kubo#9894

@codecov

codecov Bot commented May 24, 2023

Copy link
Copy Markdown

Codecov Report

Merging #313 (77afd71) into main (1464d19) will decrease coverage by 0.50%.
The diff coverage is 78.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #313      +/-   ##
==========================================
- Coverage   50.62%   50.13%   -0.50%     
==========================================
  Files         280      280              
  Lines       33724    33714      -10     
==========================================
- Hits        17074    16901     -173     
- Misses      14870    15029     +159     
- Partials     1780     1784       +4     
Impacted Files Coverage Δ
gateway/hostname.go 68.01% <78.57%> (+0.73%) ⬆️

... and 17 files with indirect coverage changes

@hacdias hacdias force-pushed the issues/9882 branch 2 times, most recently from c1c828e to 28b1b54 Compare May 24, 2023 13:33
Comment thread gateway/hostname.go Outdated
@BigLep BigLep mentioned this pull request May 31, 2023
5 tasks

@lidel lidel 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've tested this in Kubo (ipfs/kubo#9894) and works as expected.
I've resolved conflicts in 77afd71 and tested again locally, still good.

Merging, to bubble up to ipfs/kubo#9894.
@hacdias please see my comment there for next steps 🙏

@lidel lidel changed the title fix(gateway): correctly handle question marks in url.Path when redirecting fix(gateway): question marks in url.Path when redirecting Jun 2, 2023
@lidel lidel merged commit 20e2aae into main Jun 2, 2023
@lidel lidel deleted the issues/9882 branch June 2, 2023 00:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

Kubo mishandles ? in path names in IPNS context, confusing standards-conformant browsers IPFS Companion cannot handle a ? in the title of a document.

3 participants