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

IPV6 error with percent devicename (for link-local ranges / zone id) #12410

Closed
MasterJames opened this issue Apr 14, 2017 · 12 comments
Closed

IPV6 error with percent devicename (for link-local ranges / zone id) #12410

MasterJames opened this issue Apr 14, 2017 · 12 comments
Labels
url Issues and PRs related to the legacy built-in url module.

Comments

@MasterJames
Copy link

MasterJames commented Apr 14, 2017

The url parse function does not function properly when a percent sign (%) deviceName is included in the square brackets of an ipv6 address.
The example you will find here... [fe80::a00:234f:fe1e:29af%enp0s3]
theturtle32/WebSocket-Node#278

On an Ubuntu Server VM with nodejs you see hostname empty after parse thinks the percent means path not host.

host: ""
hostname: ""
href: "wss:///[fe80::a00:380f:fe2f:67fd%enp0s3]/srvlnk"
path: "/[fe80::a00:380f:fe2f:67fd%enp0s3]/srvlnk"
pathname: "/[fe80::a00:380f:fe2f:67fd%enp0s3]/srvlnk"

extra root path marker extra.
Also Note: that I could not get chrome debugging working on 7.8.0 so I reverted.

@addaleax addaleax added the url Issues and PRs related to the legacy built-in url module. label Apr 14, 2017
@bnoordhuis
Copy link
Member

WHATWG URL intentionally omits support for link-local addresses (ref).

I don't know if we have decided to freeze url.parse() for all eternity but it would be somewhat incongruent to allow them in url.parse() but not in new url.URL().

On the other hand, we do accept link-local addresses in e.g. net.connect() and net.Server#listen() (where we simply ignore it if the interface doesn't exist - but that aside.)

@MasterJames
Copy link
Author

MasterJames commented Apr 14, 2017

Ya with the turtle tool I made this PR to patch it for now so I got that working. Proving it was the case We discuss what you already know about link-local. The need is apparently real, there was no easier route to create the server. listen barfed EINVAL without the %Zone ID etc. The use case is server to server with secondary IPs on same multi CPU machines as well as same zone, with ipv6 that could intelligently route (least route etc.) globally across zones I suppose.
theturtle32/WebSocket-Node#279

@MasterJames MasterJames changed the title IPV6 error with percent devicename (for local fe80 ranges) IPV6 error with percent devicename (for local link-local ranges / zone id) Apr 14, 2017
@MasterJames MasterJames changed the title IPV6 error with percent devicename (for local link-local ranges / zone id) IPV6 error with percent devicename (for link-local ranges / zone id) Apr 15, 2017
@MasterJames
Copy link
Author

Maybe I'm missing something about how to connect server to server but with the tool I mentioned, which is pretty streamline, I would get errors without the devName in the ipv6 address when calling listen (see links). Then only after hacking a temporary patch did it connect fine, works perfectly (and no other way seemed to work). I think the local block is the correct ipv6 range where it should work, as it's the default for the primary ipv6 address.
So let's not say in 2017 Node 7?8...! can not connect server to server in a data centre or is therefore not ipv6 ready, compliant or not. I would like to see this feature request as an acceptable allowance. Call it function over form if you will, but plain "working" is better in my opinion.
Thank you for you time I'm hopeful it's a simple easy patch on your end.

@sam-github
Copy link
Contributor

@MasterJames maybe you can describe you use-case in more detail, network setup, device names, how you were using URL, and what didn't work?

This may be a mismatch between what webbrowsers want from a URL library, and what Node users want, but you'll have to layout your argument in a much more detailed way to convince the WhatWG.

@MasterJames
Copy link
Author

It was documented through the previous referenced article that was mentioned above.
Basically it describes the errors you get trying to connect any other combination of ways, and how it wonderfully does connect finally after inserting it after the "connect" call that melds it into the path with extra characters and is clearly completely missing error handling and filtering for the critically needed feature.
FE80 linked-local addresses need to allow percent sign device name references on the platform initiated with this post, (from my experience and observations). We are talking server to server and they are listening on ipv6 addresses but you get errors if you don't put the device name in with the function 'listen', subsequently needed is the same to connect from a neighboring VM.
Anyway my apologies if we still need more details to see, how it seems to be needed from my "server to server" messaging experiments with the some what still newer to me's ipv6 abilities.
For the moment they are VM to VM Ubuntu Server.

If anyone has any other ideas, how to go about doing it differently, I do appreciate and welcome it.

Possibilities I've considered, the problem will go away/change in Data Centre on AWS because of how Secondary IPs are handled, etc. and so? taking an externally bounced IP makes more sense? No I come back to this being an oversight in the specs somewhere and should be clarified between associations, and may be a big issue. I think it's pretty important but I personally have a simple work around (shown in links package relevant) or two so I'm not that concerned.
I would check it out, try to verify, maybe make it working (sounds like a simple regex chance to me) and close another issue people are having as ipv6 unfolds use into the unknown future.
Thank you again for you time and energy, in addressing this important issue of functionality in our quickly modernizing ways.

@bnoordhuis
Copy link
Member

I wouldn't get your hopes up. As mentioned, the WHATWG URL specification explicitly prohibits link-local addresses for security reasons. That means we can't add it to new url.URL() without violating the spec (which we won't do.)

The security argument also applies to node's own url.parse() but that could, to some extent, be mitigated by making it opt-in. However, we also plan to freeze and deprecate url.parse(), meaning new features are unlikely to get added.

You are welcome to try your hand at a pull request but you should be prepared for rejection. If all else fails, you can simply copy out node's lib/url.js and publish it with your changes as an npm module.

@MasterJames
Copy link
Author

MasterJames commented Apr 20, 2017

I've just read the article you referenced. It seems it's fully recognized as important, but later considered not typical enough for the complications it causes? In light of it being "difficult" and "challenging", in the end he says there's some concern over how other browsers might handle them differently. Sounds strange to me. Also I was concerned it makes for bad gateway bouncing traffic maybe, if your using a public IPV6 address instead of local FE80 range, to talk server to server inside the data centre? So personally I wasn't that impressed with the arguments, but understand them. My patch has worked fine, is super simple could be better, it's not a devastating issue. I also think the introductory package I'm working with (links above) may blur the lines server side which is using a client connect to access local addresses, maybe it's more browser side code tangled in and I don't see that from the package/wrapper level. Anyway again still seems important to me.
In light of the helpful link to the reasoning behind all this I again think it's understood well (so I don't need to explain my situation), and it's recognized as important but just not worth the client side problems.
Things to consider...
Global Variable to toggle ability, if not an argument to the function, and possibly getting FF, and specs, on board or if it's even really an issue with FF etc. prove or disprove that.
Handling the 'occurrence' better/differently/inventively then interpreting the ipv6 address as part of the path, without interrupting the understood need that is working. This really can't be as difficult as he says (I feel like my patch shows the light there), but I think a setting, option or global variable is a good idea. If the so called limited need is recognized as important (as the linked doc read to me), then having a way to enable it is important.

Sometimes the code wants to evolve the spec, we should let it. -MJ

@Trott
Copy link
Member

Trott commented Aug 2, 2017

@nodejs/url Should this remain open?

@bnoordhuis
Copy link
Member

I'll close it out. PR welcome but no guarantee it will be accepted.

@MasterJames
Copy link
Author

There is a simple work around as far as I'm concerned. Just took the time to assess what gives. Seemed like a simple to fix no Brainer to me at the time. Kinda still does. Thanks for listening sorry to bother.

@Bartel-C8
Copy link

Bartel-C8 commented Jul 9, 2021

My workaround after long hours of trying this approach:

import { URL } from "url";

class CustomURL extends URL {
  constructor(url: string) {
    super(url);
  }
  // eslint-disable-next-line @typescript-eslint/ban-ts-comment
  // @ts-ignore
  get host() {
    const context = Object.getOwnPropertySymbols(this)[0];
    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
    // @ts-ignore
    const ctx = this[context];
    let ret = ctx.host || "";
    if (ctx.port !== null) ret += `:${ctx.port}`;
    return ret;
  }
  set host(host) {
    const context = Object.getOwnPropertySymbols(this)[0];
    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
    // @ts-ignore
    const ctx = this[context];
    // toUSVString is not needed.
    host = `${host}`;
    ctx.host = host;
  }
}

And to use it in a WebSocket (which only accepts a url of type URL):

    const ipv6Url = new CustomURL("ws://[fe80::a00:380f:fe2f:67fd]:8080/ws");
    ipv6Url.host = "[fe80::a00:380f:fe2f:67fd%enp0s3]"
    ws = new WebSocket(ipv6Url) as WebSocket;

Hacky, but, IT WORKS.
Mind the first creation of CustomURL to not include the scope-id / interface name!

@TimothyGu
Copy link
Member

@Bartel-C8 Be careful. Please do not depend on symbol properties on URL objects. They are intentionally hidden for a reason, and your subclass can break any time. Node.js may very well start using private properties soon, which cannot be overridden this way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
url Issues and PRs related to the legacy built-in url module.
Projects
None yet
Development

No branches or pull requests

7 participants