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

Adds support for protocol relative URLs in location header #1298

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@bruderstein
Copy link

commented Oct 10, 2018

url.parse doesn't parse protocol relative URLs, but they are valid
in location headers according to RFC7231
(https://tools.ietf.org/html/rfc7231#section-7.1.2, and specified in
RFC3986 https://tools.ietf.org/html/rfc3986#section-4.2)

This commit fixes that by temporarily adding an http: to a URL that
begins with // before parsing and removing the protocol before
formatting.

@tjenkinson
Copy link

left a comment

👏👏 👏

// remove it so we retain the protocol relative
// of the response URL
if (protocolPrefix) {
u.protocol = '';

This comment has been minimized.

Copy link
@tjenkinson

tjenkinson Oct 11, 2018

Maybe it should not be cleared if the 'protocolRewrite' option is used?

@tjenkinson
Copy link

left a comment

👏👏 👏

Adds support for protocol relative URLs in location header
`url.parse` doesn't parse protocol relative URLs, but they are valid
in location headers according to RFC7231
(https://tools.ietf.org/html/rfc7231#section-7.1.2, and specified in
RFC3986 https://tools.ietf.org/html/rfc3986#section-4.2)

This commit fixes that by temporarily adding an `http:` to a URL that
begins with `//` before parsing and removing the protocol before
formatting.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.