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.

Sign up for GitHub

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

Jump to bottom

Treat 307 and 308 as redirects in HTTPS imports #43679

Closed
kidonng opened this issue Jul 4, 2022 · 4 comments · Fixed by #43689
Closed

Treat 307 and 308 as redirects in HTTPS imports #43679

kidonng opened this issue Jul 4, 2022 · 4 comments · Fixed by #43689
Labels
feature request Issues that request new features to be added to Node.js. loaders Issues and PRs related to ES module loaders

Comments

@kidonng
Copy link
Contributor

kidonng commented Jul 4, 2022

What is the problem this feature will solve?

Quoting #36328 (comment):

We should probably think of adding support for HTTP status code 307 and 308 at some point.

What is the feature you are proposing to solve the problem?

Per RFC 7231 and 7238, HTTP 307 and 308 status code are also for redirect responses.

What alternatives have you considered?

N/A
@kidonng kidonng added the feature request Issues that request new features to be added to Node.js. label Jul 4, 2022
@aduh95
Copy link
Contributor

aduh95 commented Jul 5, 2022

Would you like to send a PR to address this?

@aduh95 aduh95 added the loaders Issues and PRs related to ES module loaders label Jul 5, 2022
@kidonng kidonng mentioned this issue Jul 5, 2022
Merged
@hemanth
Copy link
Contributor

hemanth commented Jul 6, 2022 •
edited
Loading

I came it bit late on this. Does this look good?

@LiviaMedeiros
Copy link
Contributor

@hemanth Yes, your approach also looks good. However, the code is about fetching modules (i.e. importing over network), not about fetch. This makes test changes unrelated to this issue, but they could still be useful as generic test improvement.

@hemanth
Copy link
Contributor

hemanth commented Jul 7, 2022

Ah, HTTPS imports. Can we emulate them as well?

nodejs-github-bot pushed a commit that referenced this issue Jul 9, 2022
@kidonng
esm: treat 307 and 308 as redirects in HTTPS imports
3d575a4 
Per RFC 7231 and 7238, HTTP `307` and `308` status code are also for
redirect responses.

Fixes: #43679
Refs: https://datatracker.ietf.org/doc/html/rfc7231#section-6.4.7
Refs: https://datatracker.ietf.org/doc/html/rfc7238

PR-URL: #43689
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
targos pushed a commit that referenced this issue Jul 12, 2022
@kidonng @targos
esm: treat 307 and 308 as redirects in HTTPS imports
80ced1a 
Per RFC 7231 and 7238, HTTP `307` and `308` status code are also for
redirect responses.

Fixes: #43679
Refs: https://datatracker.ietf.org/doc/html/rfc7231#section-6.4.7
Refs: https://datatracker.ietf.org/doc/html/rfc7238

PR-URL: #43689
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
targos pushed a commit that referenced this issue Jul 20, 2022
@kidonng @targos
esm: treat 307 and 308 as redirects in HTTPS imports
2d51256 
Per RFC 7231 and 7238, HTTP `307` and `308` status code are also for
redirect responses.

Fixes: #43679
Refs: https://datatracker.ietf.org/doc/html/rfc7231#section-6.4.7
Refs: https://datatracker.ietf.org/doc/html/rfc7238

PR-URL: #43689
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
targos pushed a commit that referenced this issue Jul 31, 2022
@kidonng @targos
esm: treat 307 and 308 as redirects in HTTPS imports
953e663 
Per RFC 7231 and 7238, HTTP `307` and `308` status code are also for
redirect responses.

Fixes: #43679
Refs: https://datatracker.ietf.org/doc/html/rfc7231#section-6.4.7
Refs: https://datatracker.ietf.org/doc/html/rfc7238

PR-URL: #43689
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
guangwong pushed a commit to noslate-project/node that referenced this issue Oct 10, 2022
@kidonng @guangwong
esm: treat 307 and 308 as redirects in HTTPS imports
065b9cf 
Per RFC 7231 and 7238, HTTP `307` and `308` status code are also for
redirect responses.

Fixes: nodejs/node#43679
Refs: https://datatracker.ietf.org/doc/html/rfc7231#section-6.4.7
Refs: https://datatracker.ietf.org/doc/html/rfc7238

PR-URL: nodejs/node#43689
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature request Issues that request new features to be added to Node.js. loaders Issues and PRs related to ES module loaders
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

esm: treat 307 and 308 as redirects in HTTPS imports
4 participants
@hemanth @aduh95 @kidonng @LiviaMedeiros