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

URL parse fails #25879

Closed
pt-rick opened this issue Feb 1, 2019 · 21 comments
Closed

URL parse fails #25879

pt-rick opened this issue Feb 1, 2019 · 21 comments
Labels
i18n-api Issues and PRs related to the i18n implementation. url Issues and PRs related to the legacy built-in url module.

Comments

@pt-rick
Copy link

pt-rick commented Feb 1, 2019

  • Version: 11.6.0
  • Platform: FreeBSD 11.2-RELEASE-p4
  • Subsystem: url

Simple parse of a URL fails...

test.js
const url = require('url');

const test = new URL("https://www.test.com/test");

error
internal/url.js:243
throw error;
^

TypeError [ERR_INVALID_URL]: Invalid URL: https://www.test.com/test
at onParseError (internal/url.js:241:17)
at new URL (internal/url.js:319:5)
at Object. (/shared/rick/pushy/test.js:3:14)
at Module._compile (internal/modules/cjs/loader.js:721:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:732:10)
at Module.load (internal/modules/cjs/loader.js:620:32)
at tryModuleLoad (internal/modules/cjs/loader.js:560:12)
at Function.Module._load (internal/modules/cjs/loader.js:552:3)
at Function.Module.runMain (internal/modules/cjs/loader.js:774:12)
at executeUserCode (internal/bootstrap/node.js:342:17)

When using the deprecated url.parse, this is the result:

test.js
const url = require('url');

const test = url.parse("https://www.test.com/test");

error
url.js:385
this.hostname = toASCII(this.hostname, true);
^

Error: Cannot convert name to ASCII
at Url.parse (url.js:385:23)
at Object.urlParse [as parse] (url.js:149:13)
at Object. (/shared/rick/pushy/test.js:3:18)
at Module._compile (internal/modules/cjs/loader.js:721:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:732:10)
at Module.load (internal/modules/cjs/loader.js:620:32)
at tryModuleLoad (internal/modules/cjs/loader.js:560:12)
at Function.Module._load (internal/modules/cjs/loader.js:552:3)
at Function.Module.runMain (internal/modules/cjs/loader.js:774:12)
at executeUserCode (internal/bootstrap/node.js:342:17)

@mscdex
Copy link
Contributor

mscdex commented Feb 1, 2019

Both work for me on node master and node v10.15.0.

@devsnek
Copy link
Member

devsnek commented Feb 1, 2019

new URL(...) throws for me, as it should, since test.com/test is treated like a relative url, and URL objects are absolute.

url.parse(...) returns an object with pathname = "test.com/test", which is expected.

@bnoordhuis bnoordhuis added invalid Issues and PRs that are invalid. url Issues and PRs related to the legacy built-in url module. labels Feb 2, 2019
@bnoordhuis
Copy link
Member

I'll close this out, can reopen if necessary.

@pt-rick
Copy link
Author

pt-rick commented Feb 4, 2019

This is too simple to not work so was thinking it perhaps has something to do with my environment. But even when removing everything from node_modules, I get this error. So perhaps is there some configuration or environment setting or missing package that I should look for that might be causing this problem? Node on Free BSD perhaps?

To @mscdex comment, if I make the url a relative url (without 'https://') it doesn't throw for me if I use the deprecated url.parse but does, as you say, throw on new URL. However, both throw for me on an absolute URL as shown in the code above.

@sam-github
Copy link
Contributor

% node -p 'require("url").parse("https://www.test.com/test")'; node --version
Url {
  protocol: 'https:',
  slashes: true,
  auth: null,
  host: 'www.test.com',
  port: null,
  hostname: 'www.test.com',
  hash: null,
  search: null,
  query: null,
  pathname: '/test',
  path: '/test',
  href: 'https://www.test.com/test' }
v11.6.0

Works for me. Could be something about your system, but its hard to know. Try to strip back to something as simple as possible, like above. Maybe even build node from source if you are using a package. Its hard to know what else to suggest.

@pt-rick
Copy link
Author

pt-rick commented Feb 4, 2019

Thanks @sam-github for some ideas. I'm actually using a library that is calling url.parse so I managed to strip this down to as far as I can - 2 lines and no node_modules. Doing a cut and paste of your command, I get this:

node -p 'require("url").parse("https://www.test.com/test")'; node --version

url.js:385
this.hostname = toASCII(this.hostname, true);
^

Error: Cannot convert name to ASCII
at Url.parse (url.js:385:23)
at Object.urlParse [as parse] (url.js:149:13)
at [eval]:1:16
at Script.runInThisContext (vm.js:123:20)
at Object.runInThisContext (vm.js:312:38)
at Object. ([eval]-wrapper:6:22)
at Module._compile (internal/modules/cjs/loader.js:721:30)
at evalScript (internal/bootstrap/node.js:720:27)
at executeUserCode (internal/bootstrap/node.js:313:7)
at startExecution (internal/bootstrap/node.js:276:5)
v11.6.0

In stepping through the debugger, I now can see that toASCII doesn't seem to be getting resolved correctly and this is causing it to fail. The definition on line 24 or url.js is as follows:

const { toASCII } = internalBinding('config').hasIntl ? internalBinding('icu') : require('punycode');

internalBinding('config').hasIntl is undefined so it is trying to destruct to punycode but that seems to be where the failure is occurring as 'toASCII' is defined as ƒ toASCII() { [native code] }. I see that punycode has been deprecated so not sure if that is the issue.

Likewise when I change my code to new URL("https://www.test.com/test"), it fails on the call to 'parse' on line 319 of internal/url.js. 'parse' is defined as ƒ parse() { [native code] } and that throws the error there.

@ChALkeR ChALkeR added the i18n-api Issues and PRs related to the i18n implementation. label Feb 4, 2019
@ChALkeR
Copy link
Member

ChALkeR commented Feb 4, 2019

@bnoordhuis perhaps this needs reopening?

@sam-github
Copy link
Contributor

That's more interesting. Could you give your node -p process.versions output? I wonder if what you are running into is because of ICU? Perhaps with the right configure options it would be reproducable on other sytems, there are a number of ICU build/configure options.

Also, where did you get node from? We don't distribute pre-built binaries for FreeBSD. If its a pre-built package, it might be linked against an ICU library that isn't compatible with node. I would suggest building node from source yourself and seeing if you can repro with your own node, that might allow you to isolate at what point this problem got introduced.

@sam-github sam-github reopened this Feb 4, 2019
@sam-github
Copy link
Contributor

If this is a problem with how someone built node, it might not be a node issue, but I'll reopen while @pt-rick gathers a bit more information, since it looks like he can reproduce with just node and no external deps.

@pt-rick
Copy link
Author

pt-rick commented Feb 4, 2019

@sam-github here's the answer to the process versions. I'll have to get back to you about the FreeBSD package and source later.
{ node: '11.6.0',
v8: '7.0.276.38-node.13',
uv: '1.24.1',
zlib: '1.2.11',
ares: '1.14.0',
modules: '67',
nghttp2: '1.35.1',
napi: '3',
llhttp: '1.0.1',
http_parser: '2.8.0',
openssl: '1.1.0j',
icu: '63.1',
unicode: '11.0',
cldr: undefined,
tz: undefined }

@pt-rick
Copy link
Author

pt-rick commented Feb 4, 2019

@sam-github Node was installed using the FreeBSD 'pkg' command. It was the latest port available.

Adding @bradleythughes as I believe he contributes to FreeBSD ports.

@bnoordhuis bnoordhuis removed the invalid Issues and PRs that are invalid. label Feb 4, 2019
@sam-github
Copy link
Contributor

@pt-rick have you tried doing your own build? Doing so will definitively show it to be a problem with the package. It should just be an untar, ./configure, make, maybe a bit slow to build, but easy.

@pt-rick
Copy link
Author

pt-rick commented Feb 5, 2019

@sam-github we will give that a try and I'll post the results when available.

@pt-rick
Copy link
Author

pt-rick commented Feb 5, 2019

@sam-github we're running FreeBSD 11.2 and here are the results...

It’s not too promising that it doesn’t compile. For starters, it says it requires a version of OpenSSL that is newer than what’s on FreeBSD 11.2 (it’s the version from FreeBSD 12.0). But it has a “use bundled OpenSSL” so I enabled that and it took off compiling for a long, long time. Eventually, though, it got this:

c++ -o /usr/ports/www/node/work/node-v11.8.0/out/Release/obj.target/node_lib/src/node_stat_watcher.o ../src/node_stat_watcher.cc '-DNODE_ARCH="x64"' '-DNODE_PLATFORM="freebsd"' '-DNODE_WANT_INTERNALS=1' '-DV8_DEPRECATION_WARNINGS=1' '-DNODE_OPENSSL_SYSTEM_CERT_PATH=""' '-DHAVE_INSPECTOR=1' '-DHAVE_DTRACE=1' '-DNODE_REPORT' '-D__POSIX__' '-DNODE_USE_V8_PLATFORM=1' '-DNODE_HAVE_I18N_SUPPORT=1' '-DHAVE_OPENSSL=1' '-DHTTP_PARSER_STRICT=0' -I../src -I/usr/ports/www/node/work/node-v11.8.0/out/Release/obj/gen -I/usr/ports/www/node/work/node-v11.8.0/out/Release/obj/gen/include -I/usr/ports/www/node/work/node-v11.8.0/out/Release/obj/gen/src -I../deps/v8/include -I../deps/http_parser -I../deps/llhttp/include -I../deps/brotli/c/include -I../deps/openssl/openssl/include -pthread -Wall -Wextra -Wno-unused-parameter -m64 -Wall -Wextra -Wno-unused-parameter -O3 -fno-omit-frame-pointer -fno-rtti -fno-exceptions -std=gnu++1y -MMD -MF /usr/ports/www/node/work/node-v11.8.0/out/Release/.deps//usr/ports/www/node/work/node-v11.8.0/out/Release/obj.target/node_lib/src/node_stat_watcher.o.d.raw -isystem /usr/local/include -O2 -pipe -fstack-protector -isystem /usr/local/include -fno-strict-aliasing -isystem /usr/local/include -c
../src/node_os.cc:107:3: error: unknown type name 'uv_utsname_t'
uv_utsname_t info;
^
1 error generated.

We're going to try on FreeBSD 12.0 and see if we can get a build.

@sam-github
Copy link
Contributor

it says it requires a version of OpenSSL that is newer than what’s on FreeBSD 11.2 (it’s the version from FreeBSD 12.0). But it has a “use bundled OpenSSL” so I enabled that

That makes me nervous. What are you building? I don't think its https://nodejs.org/dist/v11.9.0/node-v11.9.0.tar.gz Nodejs defaults to building against its own bundled OpenSSL, so if you are having to "enable" that option, something has gone wrong.

unknown type name 'uv_utsname_t' -- node also builds against its own version of libuv by default, so this means you are not using default configure options, and are trying to build against a system libuv.

Possibly you are rebuilding the FreeBSD package, NOT our vanilla tarball.

Btw, we build and test green against freebsd 10 and 11 in CI. Every commit, every release.

@pt-rick
Copy link
Author

pt-rick commented Feb 5, 2019

@sam-github We were building the FreeBSD port. Apparently the incompatibility must be there. If we do a port on FreeBSD 12, url.parse succeeds so the issue has to do with the 11.2 port. Thank you for your help!

@pt-rick pt-rick closed this as completed Feb 5, 2019
@bradleythughes
Copy link
Member

@pt-rick: Since this seems to be an issue with Node.js on 11.2-RELEASE, can you open a bug for it on the FreeBSD project's Bugzilla?

@bradleythughes
Copy link
Member

@pt-rick Also, for what it's worth, I cannot reproduce this problem on FreeBSD 11.2-RELEASE with Node.js 11.8.0 installed via pkg. As I mentioned, file a bug with the project, and we can discuss and debug your problem there. :)

@pt-rick
Copy link
Author

pt-rick commented Feb 7, 2019

@bradleythughes prior to @sam-github suggestion, we have only used the pkg port so not sure why ours breaks. It does work as expected under FreeBSD 12. I submitted a bug report to Bugzilla.

@elcheco
Copy link

elcheco commented Dec 31, 2020

I am trying to set up GitLab on FreeBSD and I ended up on here discussed issue. I am now trying to rebuild the Node to 15.4 and also the latest yarn versions because, on the current, it produces exactly the same above described error even without any modules.
Let's see after the update, but it seems that the error could be still present. The port for node offers also built-in OpenSSL and not the system one. Could this be the thing?

❯ pkg version -vIL=
node-15.3.0                        <   needs updating (index has 15.4.0)
yarn-1.22.4_1                      <   needs updating (index has 1.22.10)

Versions:

❯ freebsd-version
12.2-RELEASE

❯ node --version
v15.3.0

❯ npm -v
6.14.8

❯ node -p process.versions
{
  node: '15.3.0',
  v8: '8.6.395.17-node.22',
  uv: '1.40.0',
  zlib: '1.2.11',
  brotli: '1.0.9',
  ares: '1.16.1',
  modules: '88',
  nghttp2: '1.41.0',
  napi: '7',
  llhttp: '2.1.3',
  openssl: '1.1.1h-freebsd',
  cldr: '37.0',
  icu: '67.1',
  tz: '2019c',
  unicode: '13.0'
}

❯ node -p 'require("url").parse("https://www.test.com/test")';
node:url:390
      this.hostname = toASCII(this.hostname, true);
                      ^

TypeError: Cannot convert name to ASCII
    at Url.parse (node:url:390:23)
    at Object.urlParse [as parse] (node:url:155:13)
    at [eval]:1:16
    at Script.runInThisContext (node:vm:133:18)
    at Object.runInThisContext (node:vm:310:38)
    at node:internal/process/execution:77:19
    at [eval]-wrapper:6:22
    at evalScript (node:internal/process/execution:76:60)
    at node:internal/main/eval_string:23:3 {
  code: 'ERR_INVALID_ARG_VALUE'
}

@elcheco
Copy link

elcheco commented Jan 1, 2021

After NodeJs upgrade to version 15.4.0, it works ;-)

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

No branches or pull requests

8 participants