Replace usage of old node:url api with WHATWG one in pushgateway.js#744
Replace usage of old node:url api with WHATWG one in pushgateway.js#744yvon-morice-wmx wants to merge 2 commits into
Conversation
|
|
||
| function legacyUrlFromWhatwgUrl(whatwgUrl) { | ||
| const legacyUrl = { | ||
| hash: whatwgUrl.hash, |
There was a problem hiding this comment.
If you're going to go through the trouble of unwinding URL->url attribute copying it'd be better off to work with URL internally and just do the jiggery pokery for forwarding the old style requestOptions over the top, so we don't round-trip to the legacyUrlResolve function twice.
5e0afa6 to
88ee2ba
Compare
Signed-off-by: Yvon Morice <yvon.morice@winamax.fr>
Signed-off-by: Yvon Morice <yvon.morice@winamax.fr>
c8c2194 to
1079eb3
Compare
There was a problem hiding this comment.
Still hasn't removed the old url, just uses it less.
Until the http request is being made with URL I don't believe this PR achieves the goal of replacing the legacy url parser.
Also this PR will need pinning tests.
So what I'd want to see is:
- no urls, full stop
- No manual interpolation. I'm a stickler about this an every time I haven't been has turned into a security hole. And you shouldn't need it at any rate if we stop going back and forth between.
- splitting the URL and the request Options before calling
http.request(url, options)removes the old format and any translation issues lurking there, and also solves the no interpolation problem handily.
This is a Kernighan's Law situation. It's a "there are no obvious bugs" vs "there are obviously no bugs" (Hoare) scenario. The new version should be obviously correct or we haven't really fixed the hole, just punched new ones.
Related to #743
Replaces usage of
url.urlParseandurl.resolvefromnode:urlbynew url.Urlfrom same package as these functions have been deprecated and trigger a warning on usage.Changes based on code and documentation available at https://nodejs.org/api/url.html#urlresolvefrom-to and https://github.com/nodejs/userland-migrations/tree/main/recipes/node-url-to-whatwg-url