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

Bug: ipfs:// drops query and hash parts #13722

Closed
lidel opened this issue Jan 22, 2021 · 6 comments · Fixed by brave/brave-core#7693
Closed

Bug: ipfs:// drops query and hash parts #13722

lidel opened this issue Jan 22, 2021 · 6 comments · Fixed by brave/brave-core#7693

Comments

@lidel
Copy link

lidel commented Jan 22, 2021

To reproduce have IPFS enabled and local node is running.

#fragment

  • enter ipfs://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq/wiki/Vincent_van_Gogh.html#Emerging_artist via address bar
    • 💔 it loads, but does not jump to #Emerging_artist header, and the fragment is also removed from address bar

This makes it impossible for people to link to specific sections of documents

?query=params

  • ipfs://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq/I/m/Vincent_van_Gogh_-_Self-Portrait_-_Google_Art_Project_(454045).jpg?filename=test.jpg&download=true
    • 💔 query parameters are not forwarded to the gateway, which in turns produces gateway response missing Content-Disposition header

This makes it impossible to create a link which triggers download instead of having file rendered in browser.

@autonome @bbondy I think we want to fix both, but lmk if there is a reason to keep current behavior.

@lidel lidel added OS/Android Fixes related to Android browser functionality OS/Desktop labels Jan 22, 2021
@bbondy
Copy link
Member

bbondy commented Jan 22, 2021

Nice find!

@bbondy bbondy added this to Untriaged in IPFS via automation Jan 22, 2021
@bbondy bbondy added the priority/P2 A bad problem. We might uplift this to the next planned release. label Jan 22, 2021
@bbondy bbondy moved this from Untriaged to Backlog P1/P2 in IPFS Jan 22, 2021
@yrliou yrliou self-assigned this Jan 22, 2021
@yrliou yrliou removed the OS/Android Fixes related to Android browser functionality label Jan 25, 2021
@yrliou
Copy link
Member

yrliou commented Jan 25, 2021

With brave/brave-core#7693, we won't drop query parameters anymore, but currently we get Content-Disposition: inline; filename*=UTF-8''test.jpg instead of attachment for ipfs://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq/I/m/Vincent_van_Gogh_-Self-Portrait-Google_Art_Project(454045).jpg?filename=test.jpg&download=true, so brave won't open a download dialog but only renders it in the browser.
This is because ipfs/kubo#7677 is merged after go-ipfs 0.7.0 release which brave is currently using.

@yrliou
Copy link
Member

yrliou commented Jan 25, 2021

Test plan specified in brave/brave-core#7693

IPFS automation moved this from Backlog P1/P2 to Done Jan 25, 2021
@yrliou yrliou added this to the 1.21.x - Nightly milestone Jan 25, 2021
@stephendonner
Copy link

stephendonner commented Feb 11, 2021

Verified FIXED by following the testplan in brave/brave-core#7693 using

Brave 1.21.50 Chromium: 88.0.4324.152 (Official Build) beta (x86_64)
Revision 6579930fc53b4dc589c042bec9d0a3778326974d-refs/branch-heads/4324@{#2106}
OS macOS Version 11.2.1 (Build 20D74)

Steps:

  1. Visited ipfs://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq/wiki/Vincent_van_Gogh.html#Emerging_artist, which jumped to the "Emerging artist" section of the article.

Screen Shot 2021-02-10 at 5 14 57 PM

  1. Opened a new tab, and opened developer tools's Network tab.
  2. Visited ipfs://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq/I/m/Vincent_van_Gogh_-Self-Portrait-Google_Art_Project(454045).jpg?filename=test.jpg&download=true; filename=test.jpg&download=true should be kept in the address bar.
  3. Examined the network response using developer tools's Network tab, and saw Content-Disposition: inline; filename*=UTF-8''test.jpg in the header, as below.

Screen Shot 2021-02-10 at 5 05 16 PM


Verification passed on

Brave 1.21.52 Chromium: 88.0.4324.152 (Official Build) dev (64-bit)
Revision 6579930fc53b4dc589c042bec9d0a3778326974d-refs/branch-heads/4324@{#2106}
OS Ubuntu 18.04 LTS

Verified test plan from brave/brave-core#7693 and the description
#fragment
Verified jump to #Emerging_artist when on gateway
image
Verified jump to #Emerging_artist after enabling local ipfs node
image
?query=params
Verified params are kept in addressbar
image
Verified the network response using developer tools's Network tab, and saw Content-Disposition: inline; filename*=UTF-8''test.jpg
image


Verification passed on

Brave | 1.21.68 Chromium: 89.0.4389.58 (Official Build) (64-bit)
-- | --
Revision | 1a139f28ecc27719439e37c6b1533cee999cb802-refs/branch-heads/4389@{#1134}
OS | Windows 10 OS Version 2004 (Build 19041.804)

Visitedhttps://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq.ipfs.dweb.link/wiki/Vincent_van_Gogh.html#Emerging_artist and verified jump to #Emerging_artist section
image

  • ?query=params Verified params are kept in address bar

Examined the network response using developer tools' Network tab, and saw Content-Disposition: inline; filename*=UTF-8''test.jpg in the header
image

@autonome
Copy link

👋🏼 @stephendonner 😁

@stephendonner
Copy link

👋🏼 @stephendonner 😁

Small 🌎 ; fancy meeting you here! Brought a smile to my face to know another awesome Mozillian was "at the other end." <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment