Skip to content

Adds support for protocol relative URLs in location header#1298

Open
bruderstein wants to merge 1 commit intohttp-party:masterfrom
bruderstein:add-protocol-relative-url-support
Open

Adds support for protocol relative URLs in location header#1298
bruderstein wants to merge 1 commit intohttp-party:masterfrom
bruderstein:add-protocol-relative-url-support

Conversation

@bruderstein
Copy link
Copy Markdown

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.

Copy link
Copy Markdown

@tjenkinson tjenkinson left a comment

Choose a reason for hiding this comment

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

👏👏 👏

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

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown

@tjenkinson tjenkinson left a comment

Choose a reason for hiding this comment

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

👏👏 👏

`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.
@odanado
Copy link
Copy Markdown

odanado commented Feb 15, 2021

Are there any updates?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants