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

Regex for URI.withinString extremely slow on certain inputs #131

Closed
jsha opened this Issue Jan 10, 2014 · 4 comments

Comments

Projects
None yet
2 participants
@jsha

jsha commented Jan 10, 2014

I believe URI's regex for extracting URIs suffers from "catastrophic backtracking" (http://www.regular-expressions.info/catastrophic.html). Here's a repro case (runs in Node). I've also been able to reproduce the slowness in Chrome.

var URI = require("URIjs");
var input = "background-image:url('http://example.com/my/long/path/to/an/image.png')";
URI.withinString(input, function(u) { console.log(u); });

@rodneyrehm

This comment has been minimized.

Show comment
Hide comment
@rodneyrehm

rodneyrehm Jan 10, 2014

Member

I haven't written that URL-finder RegExp myself, I simply used one submitted to a url-regex-golfing thing we did ages ago.

Maybe Regex 101 can help with debugging. Also see Regexpr visualization

Member

rodneyrehm commented Jan 10, 2014

I haven't written that URL-finder RegExp myself, I simply used one submitted to a url-regex-golfing thing we did ages ago.

Maybe Regex 101 can help with debugging. Also see Regexpr visualization

@rodneyrehm

This comment has been minimized.

Show comment
Hide comment
@rodneyrehm

rodneyrehm Jan 10, 2014

Member

I wonder if we shouldn't just use /\b(?:([a-z][\w-]+:\/\/)|www\.)[^\s]+/gi to find URLs, possibly containing trailing characters (closing brackets, dots, commas, …), and fix that in the replace() callback in withinString().

Member

rodneyrehm commented Jan 10, 2014

I wonder if we shouldn't just use /\b(?:([a-z][\w-]+:\/\/)|www\.)[^\s]+/gi to find URLs, possibly containing trailing characters (closing brackets, dots, commas, …), and fix that in the replace() callback in withinString().

@jsha

This comment has been minimized.

Show comment
Hide comment
@jsha

jsha Jan 13, 2014

Yeah, I think that is probably the best approach.

jsha commented Jan 13, 2014

Yeah, I think that is probably the best approach.

rodneyrehm added a commit that referenced this issue Jan 23, 2014

fixing URI.withinString to not use URI.find_uri_expression anymore an…
…d allow filtering URLs by optional RegExp as well as ignore URLs in HTML - closing #117, #131
@rodneyrehm

This comment has been minimized.

Show comment
Hide comment
@rodneyrehm

rodneyrehm Jan 23, 2014

Member

The issue is resolved in v1.12.0 see the docs for an enhanced API.

Member

rodneyrehm commented Jan 23, 2014

The issue is resolved in v1.12.0 see the docs for an enhanced API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment