jshttp / mime-db

Media Type Database
MIT License
1.12k stars 260 forks source link

Allow redirects in fetch-iana script #263

Closed mathiasbynens closed 2 years ago

mathiasbynens commented 2 years ago

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

mathiasbynens commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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