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

Allow redirects in fetch-iana script #263

Conversation

mathiasbynens
Copy link
Contributor

@mathiasbynens mathiasbynens commented May 8, 2022

Certain URLs of the form https://tools.ietf.org/rfc/${RFC_ID}.txt redirect multiple times before arriving to the canonical resource. Example:

$ curl -sI 'https://tools.ietf.org/rfc/rfc9239.txt' | grep Location
Location: https://rfc-editor.org/rfc/rfc9239.txt

$ curl -sI 'https://rfc-editor.org/rfc/rfc9239.txt' | grep Location
Location: https://www.rfc-editor.org/rfc/rfc9239.txt

$ curl -sI 'https://www.rfc-editor.org/rfc/rfc9239.txt' | grep HTTP
HTTP/1.1 200 OK

cogent’s implicit default is to only follow 1 redirect. This patch explicitly configures cogent to follow up to two redirects.

This is required to properly fetch RFC 9239 and update the JavaScript MIME type entries.

Issue: #194, #262

Certain URLs of the form `https://tools.ietf.org/rfc/${RFC_ID}.txt` redirect multiple times before arriving to the canonical resource. Example:

    $ curl -sI 'https://tools.ietf.org/rfc/rfc9239.txt' | grep Location
    Location: https://rfc-editor.org/rfc/rfc9239.txt

    $ curl -sI 'https://rfc-editor.org/rfc/rfc9239.txt' | grep Location
    Location: https://www.rfc-editor.org/rfc/rfc9239.txt

    $ curl -sI 'https://www.rfc-editor.org/rfc/rfc9239.txt' | grep HTTP
    HTTP/1.1 200 OK

`cogent`’s implicit default is to only follow 1 redirect. This patch explicitly configures `cogent` to follow up to two redirects.

This is required to properly fetch RFC 9239 and update the JavaScript MIME type entries.

Issue: jshttp#194, jshttp#262
@mathiasbynens mathiasbynens force-pushed the allow-redirects-in-fetch-iana branch from 3e934c1 to aa6fb2c Compare May 8, 2022 09:41
mathiasbynens added a commit to mathiasbynens/mime-db that referenced this pull request May 8, 2022
This allows us to have a clean diff to see the impact of the patch in jshttp#263.

Issue: jshttp#194, jshttp#262
@mathiasbynens
Copy link
Contributor Author

#264 should be merged first. Then I can update this patch to include only the src/iana-types.json diff caused by these changes.

@dougwilson
Copy link
Contributor

Hello, and thank you for this PR. I'm not seeing any different behavior before or after your PR, and the default redirects for the cogent module we used in 3, which is larger than you are setting here. Is there a particular reason this should only allow up to 2 redirects instead of the default 3 redirects? https://github.com/cojs/cogent/blob/master/lib/index.js#L20

@mathiasbynens
Copy link
Contributor Author

Ah, as stated in the commit message, my assumption was that cogent defaults to following only 1 redirect. I got that from this README, which states:

redirects - resolve redirects, default 1

If the true default is indeed 3 then we don’t actually need this. Feel free to close this.

@dougwilson
Copy link
Contributor

Ah, yes, hmm... so I pulled down this PR and ran the scripts and there doesn't seem to be any different behavior. Was this supposed to fix something / what difference should I see with this change?

@mathiasbynens
Copy link
Contributor Author

I assumed that this was the cause of the newer entries w.r.t. RFC 9239 not showing up. I guess that was wrong :)

@mathiasbynens mathiasbynens deleted the allow-redirects-in-fetch-iana branch May 12, 2022 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants