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

[READY] tldjs chokes on weird domains #33

Merged
merged 2 commits into from
Jan 15, 2014
Merged

[READY] tldjs chokes on weird domains #33

merged 2 commits into from
Jan 15, 2014

Conversation

thom4parisot
Copy link
Owner

Hi

Thanks for making tldjs, it is immensely useful :-)

I've incorporated it in a project and tested it against a database of 25.000 domains.

It seems that tldjs can crash the whole server (node process) when given bad input. Example:

$ node
> var tld = require('tldjs');
> tld.getSubdomain('www.weir)domain.com');

SyntaxError: Invalid regular expression: /\.?weir)domain\.com$/: Unmatched ')'
    at new RegExp (<anonymous>)
    at tld.getSubdomain (./node_modules/tldjs/lib/tld.js:189:28)
    at repl:1:5

Looks like the problem is that the input strings are not properly escaped, before they are put to use inside a regular expression.

If I may suggest a solution also, this is from Mozilla MDN documentation:

if (! RegExp.quote) {
    RegExp.quote = function(s) {
        return String(s).replace(/([.*+?^=!:${}()|\[\]\/\\])/g, "\\$1");
    }
}

The function from MDN can be called with an arbitrary string, and will properly escape it for use inside a regular expression.

Thanks for reading :-)

@thom4parisot
Copy link
Owner

Ouh, nicely spotted. Thanks for the suggestion; I'll take that in account :-)

@ghost ghost assigned thom4parisot Jan 14, 2014
thom4parisot pushed a commit that referenced this pull request Jan 15, 2014
tldjs chokes on weird domains
@thom4parisot thom4parisot merged commit cd18296 into master Jan 15, 2014
@thom4parisot thom4parisot deleted the fix-33 branch January 15, 2014 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant