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

fix(gateway)!: no duplicate payload during subdomain redirects #9913

Merged
merged 4 commits into from
Jun 2, 2023

Conversation

hacdias
Copy link
Member

@hacdias hacdias commented Jun 1, 2023

Rationale

Based on ipfs/boxo#326 (comment):

We have made significant progress on writing specifications of the Path Gateway to Subdomain Gateway. It is time to remove hacks and unidiomatic HTTP behavior from Kubo's Gateway.

Previously, when performing redirects from Path gw to Subdomain gw, the response body would include the data. However, we have decided to discontinue this behavior for multiple reasons:

  • Firstly, it was identified that this approach was a hack and not a recommended practice, antipattern when looking at HTTP Semantics. To ensure consistency and discourage others from implementing this non-standard behavior, we have removed it from the gateway-conformance requirements too (fix: remove body check for subdomain redirection gateway-conformance#65).

  • Secondly, ipfs.io did not become a Subdomain Gateway. We've decoded to use a separate domain name for that, dweb.link. Thus, the redirect body hack was never deployed to ipfs.io.

  • Additionally, including the CID data in the response body was found to be inefficient and could lead to increased bandwidth usage and latency for clients unable to drop the connection after receiving the Location header. In such cases, client would receive data twice.

If any automation or process relies on the old behavior, the response body will now include a text/html message, whichwill inform the user about the redirect and provide the URL of the new location. This change makes it easier for legacy users to identify and troubleshoot any issues that may arise due to the modified behavior.

Overall, these changes aim to improve the performance, consistency, and maintainability of the Path Gateway to Subdomain Gateway redirects, while providing clear information to users in case of any unexpected behavior.

@hacdias hacdias self-assigned this Jun 1, 2023
@hacdias hacdias marked this pull request as ready for review June 1, 2023 10:17
@hacdias hacdias requested review from lidel and a team as code owners June 1, 2023 10:17
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

lgtm: I've added changelog and restored sharness test to confirm we have human-readable body.
Feel free to merge once boxo issue below is resolved.


@hacdias FYI I was not able to bubble up commit form boxo main branch to this PR today because ipfs/boxo#273 got merged first and introduced breaking changes:

$ go mod tidy
[..]
github.com/ipfs/kubo/core/commands imports
	github.com/ipfs/boxo/provider/batched: package github.com/ipfs/boxo/provider/batched provided by github.com/ipfs/boxo at latest version v0.8.1 but not at required version v0.8.2-0.20230601215904-1464d19f3dba
[..]

My understanding is we need to merge #9886 first, then rebase this PR and bumping boxo should work again.

@lidel lidel changed the title feat(gateway)!: do not send body when redirecting fix(gateway)!: no duplicate payload during subdomain redirects Jun 1, 2023
@hacdias hacdias merged commit eb265f7 into master Jun 2, 2023
@hacdias hacdias deleted the feat/just-redirect branch June 2, 2023 06:14
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.

2 participants