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 parser does not validate ipv4 with more than 4 parts #42914

Closed
anonrig opened this issue Apr 29, 2022 · 3 comments · Fixed by #42915 or #43190
Closed

URL parser does not validate ipv4 with more than 4 parts #42914

anonrig opened this issue Apr 29, 2022 · 3 comments · Fixed by #42915 or #43190
Labels
url Issues and PRs related to the legacy built-in url module.

Comments

@anonrig
Copy link
Member

anonrig commented Apr 29, 2022

Version

v18.0.0

Platform

Darwin Yagizs-MBP 21.4.0 Darwin Kernel Version 21.4.0: Fri Mar 18 00:46:32 PDT 2022; root:xnu-8020.101.4~15/RELEASE_ARM64_T6000 arm64

Subsystem

No response

What steps will reproduce the bug?

This works well

Welcome to Node.js v18.0.0.
Type ".help" for more information.
> new URL('http://256.256.256.256')
Uncaught TypeError [ERR_INVALID_URL]: Invalid URL
    at __node_internal_captureLargerStackTrace (node:internal/errors:465:5)
    at new NodeError (node:internal/errors:372:5)
    at URL.onParseError (node:internal/url:563:9)
    at new URL (node:internal/url:643:5) {
  input: 'http://256.256.256.256',
  code: 'ERR_INVALID_URL'
}

but this does not throw an error:

> new URL('https://256.256.256.256.256.256.256')
URL {
  href: 'https://256.256.256.256.256.256.256/',
  origin: 'https://256.256.256.256.256.256.256',
  protocol: 'https:',
  username: '',
  password: '',
  host: '256.256.256.256.256.256.256',
  hostname: '256.256.256.256.256.256.256',
  port: '',
  pathname: '/',
  search: '',
  searchParams: URLSearchParams {},
  hash: ''
}

How often does it reproduce? Is there a required condition?

URL specification states that if the parts size is greater than 4, validation error should occur and return failure. Look at https://url.spec.whatwg.org/#concept-ipv4-parser

What is the expected behavior?

It should throw error

What do you see instead?

It continues to parse the input

Additional information

No response

@Trott
Copy link
Member

Trott commented Apr 29, 2022

new URL('https://256.256.256.256.256.256.256') throws in Chrome but works in Firefox similar to Node.js. I wonder if only one or the other is spec compliant or if they are both allowable. @nodejs/url

@Trott
Copy link
Member

Trott commented Apr 29, 2022

new URL('https://256.256.256.256.256.256.256') throws in Chrome but works in Firefox similar to Node.js. I wonder if only one or the other is spec compliant or if they are both allowable. @nodejs/url

Sure looks to me like Firefox and Node.js do the wrong thing here and the issue is valid.

@anonrig
Copy link
Member Author

anonrig commented Apr 29, 2022

I've opened another issue to update the tests for the whatwg-url, since we're using a pretty old test suite. #42920

@VoltrexKeyva VoltrexKeyva added the url Issues and PRs related to the legacy built-in url module. label Apr 30, 2022
@Trott Trott closed this as completed in 87d0d7a May 1, 2022
danielleadams pushed a commit that referenced this issue Jun 11, 2022
PR-URL: #42915
Fixes: #42914
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
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
3 participants