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

Serialize base URL's host when a string is required #250

Merged
merged 1 commit into from
Jan 9, 2025

Conversation

jeremyroman
Copy link
Collaborator

@jeremyroman jeremyroman commented Jan 8, 2025

If the URL's host is an IP address, the string representation is required.

This corrects/clarifies existing behavior and partially resolves #242.


Preview | Diff

If the URL's host is an IP address, the string representation is
required.

This corrects/clarifies existing behavior.
@jeremyroman jeremyroman requested a review from sisidovski January 8, 2025 21:44
@jeremyroman jeremyroman merged commit 7564e2d into whatwg:main Jan 9, 2025
2 checks passed
@jeremyroman jeremyroman deleted the hostname-string branch January 9, 2025 15:47
@@ -1858,7 +1858,7 @@ To <dfn>convert a modifier to a string</dfn> given a [=part/modifier=] |modifier
1. If |type| is not "`pattern`" and |init| [=map/contains=] none of "{{URLPatternInit/protocol}}", "{{URLPatternInit/hostname}}", "{{URLPatternInit/port}}" and "{{URLPatternInit/username}}", then set |result|["{{URLPatternInit/username}}"] to the result of [=processing a base URL string=] given |baseURL|'s [=url/username=] and |type|.
1. If |type| is not "`pattern`" and |init| [=map/contains=] none of "{{URLPatternInit/protocol}}", "{{URLPatternInit/hostname}}", "{{URLPatternInit/port}}", "{{URLPatternInit/username}}" and "{{URLPatternInit/password}}", then set |result|["{{URLPatternInit/password}}"] to the result of [=processing a base URL string=] given |baseURL|'s [=url/password=] and |type|.
1. If |init| [=map/contains=] neither "{{URLPatternInit/protocol}}" nor "{{URLPatternInit/hostname}}", then:
1. Let |baseHost| be |baseURL|'s [=url/host=].
1. Let |baseHost| be the [=host serializer|serialization=] of |baseURL|'s [=url/host=].
Copy link

@shannonbooth shannonbooth Jan 10, 2025

Choose a reason for hiding this comment

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

Sorry, I didn't happen to catch this MR - but I don't believe the fix here is fully correct. URL's host may be null, and it is not defined in the URL spec how to serialize a null host. This also makes the second step not quite correct, as the serialization of host always returns an ASCII string, so can never be null.

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

Successfully merging this pull request may close these issues.

Process a URLPatternInit doesn't call basic URL parser on base URL
3 participants