Bug 204414 - Incorrect association of the URL object with the value port
Summary: Incorrect association of the URL object with the value port
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: Safari 13
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-11-20 09:54 PST by Ivan Demidov
Modified: 2019-12-02 10:56 PST (History)
8 users (show)

See Also:


Attachments
Patch (4.06 KB, patch)
2019-11-22 18:41 PST, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (9.66 KB, patch)
2019-11-22 21:07 PST, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ivan Demidov 2019-11-20 09:54:43 PST
const {hostname, port} = location;
const pathname = 'websocket';
const protocol = 'wss:'
const target = {hostname, pathname, port, protocol, search: ''};

Object.assign(new URL(location), target);

as a result of merging the above code, 0 value gets to the port

URL {
  hash: ""
  host: "bugs.webkit.org:0"
  hostname: "bugs.webkit.org"
  href: "wss://bugs.webkit.org:0/websocket"
  origin: "wss://bugs.webkit.org:0"
  password: ""
  pathname: "/websocket"
  port: "0"
  protocol: "wss:"
  search: ""
  searchParams: URLSearchParams {append: function, delete: function, get: function, getAll: function, has: function, …}
  username: ""
}
Comment 1 Radar WebKit Bug Importer 2019-11-22 16:00:43 PST
<rdar://problem/57444249>
Comment 2 Alex Christensen 2019-11-22 18:41:14 PST
Created attachment 384223 [details]
Patch
Comment 3 Alex Christensen 2019-11-22 21:07:16 PST
Created attachment 384230 [details]
Patch
Comment 4 Sam Weinig 2019-11-25 17:55:49 PST
Comment on attachment 384230 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=384230&action=review

> Source/WebCore/html/URLUtils.h:236
> +    unsigned port = value.toUInt(&success);

Not related to this change, but it's a bit odd we use toUInt() here, rather than toUIntStrict(), since the former allows trailing garbage. Probably worth testing, but not in this change. (I also kind of want to change the toNumber functions to return Optionals).
Comment 5 Alex Christensen 2019-12-02 10:13:09 PST
Chromium allows trailing garbage, Firefox does not, the spec is a little unclear and seems to me that it would allow url.port="123?query" to override the path and query, but no browser allows that.  I'm going to commit this as-is right now to save the toUIntStrict discussion for another day, because it doesn't seem to be causing compatibility problems in practice now.
Comment 6 WebKit Commit Bot 2019-12-02 10:56:56 PST
Comment on attachment 384230 [details]
Patch

Clearing flags on attachment: 384230

Committed r252998: <https://trac.webkit.org/changeset/252998>
Comment 7 WebKit Commit Bot 2019-12-02 10:56:57 PST
All reviewed patches have been landed.  Closing bug.