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

non-breaking space causes parse-domain to fail #61

Closed
AurielleP opened this issue Jan 17, 2019 · 5 comments
Closed

non-breaking space causes parse-domain to fail #61

AurielleP opened this issue Jan 17, 2019 · 5 comments

Comments

@AurielleP
Copy link

AurielleP commented Jan 17, 2019

ParseDomain('\xa0')
TypeError: Cannot read property '3' of null
at parseDomain (/node_modules/parse-domain/lib/parseDomain.js:58:22)

related to #32

@AurielleP
Copy link
Author

AurielleP commented Jan 23, 2019

new lines also throw this error

ParseDomain('\n\nhttp://somewebsite.com/privacy-statement/')
TypeError: Cannot read property '3' of null
at parseDomain (/node_modules/parse-domain/lib/parseDomain.js:48:22)

@kulinsj
Copy link

kulinsj commented May 9, 2019

In the case of line breaks in the url, the normalizedUrl is not falsy, so this referenced PR is not sufficient to prevent the error. It would probably be more prudent to sanitize the string in the normalize.js file. I'm now pre-sanitizing the urls i want to parse before handing them to the parse-domain function
cleanedUrl = url.replace(/(\r\n|\n|\r)/gm, '');
With all that said, it's still good practice to do a null check on the urlSplit in case there are other unforseen cases where the regex won't match

@jhnns
Copy link
Member

jhnns commented May 10, 2019

We delegate all the URL parsing logic to the WHATWG URL constructor in a major version update. URL parsing is too complicated. See #14

@jhnns
Copy link
Member

jhnns commented May 27, 2019

In the case of line breaks in the url, the normalizedUrl is not falsy, so this referenced PR is not sufficient to prevent the error

normalize.js uses .trim() which removes line breaks. A falsy check should be sufficient after that. So I'm going to merge and publish this PR as a fix.

@jhnns
Copy link
Member

jhnns commented May 27, 2019

Fixed with v2.1.8

@jhnns jhnns closed this as completed May 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants