search

medialize / URI.js

Javascript URL mutation library
http://medialize.github.io/URI.js/
MIT License
6.26k stars 474 forks source link

withinString may throw an exception on malformed urls #359

Open xPaw opened 7 years ago

xPaw commented 7 years ago

Basically this function is not safe enough to be called on user input as it may crash with "URI malformed" which is thrown by nodejs' implementation of decodeURIComponent

/usr/local/lib/node_modules/thelounge/node_modules/urijs/src/URI.js:595
      parts.password = t[0] ? URI.decode(t.join(':')) : null;
                                  ^

URIError: URI malformed
    at Function.decodeURIComponent [as decode] (native)
    at Function.URI.parseUserinfo (/usr/local/lib/node_modules/thelounge/node_modules/urijs/src/URI.js:595:35)
    at Function.URI.parseAuthority (/usr/local/lib/node_modules/thelounge/node_modules/urijs/src/URI.js:581:18)
    at Function.URI.parse (/usr/local/lib/node_modules/thelounge/node_modules/urijs/src/URI.js:516:24)
    at URI.p.href (/usr/local/lib/node_modules/thelounge/node_modules/urijs/src/URI.js:1174:25)
    at new URI (/usr/local/lib/node_modules/thelounge/node_modules/urijs/src/URI.js:70:10)
    at URI (/usr/local/lib/node_modules/thelounge/node_modules/urijs/src/URI.js:46:16)
    at /usr/local/lib/node_modules/thelounge/client/js/libs/handlebars/ircmessageparser/findLinks.js:27:24
    at Function.URI.withinString (/usr/local/lib/node_modules/thelounge/node_modules/urijs/src/URI.js:1003:20)
    at findLinks (/usr/local/lib/node_modules/thelounge/client/js/libs/handlebars/ircmessageparser/findLinks.js:25:6)

Right now the only suitable solution I see is wrapping it in a try/catch, which somewhat defeats the purpose of this function.

A link that throws: http://a:%p@c

xPaw commented 7 years ago

Okay my bad on this one, it throws the exception within URI constructor, not withinString, so it's the same issue as #352.

rodneyrehm commented 7 years ago

It's not the same as #352 as that was caused by an unfortunate change in URI.js v1.18.11 (and reverted in v1.19.0).

What is the exact input you're dealing with here?

You'll probably want to make sure that the new URI() inside the withingString callback doesn't break for all the URLs in the string:

URI.withinString(text, function(uri) {
  try {
    var u = new URI(uri);
    // …
  } catch (error) {
    // ignore the uri
    return undefined;
  }
});
luisnaranjo733 commented 5 years ago

@rodneyrehm FYI the workaround described above works but the type for withinString does not permit an undefined return value

rodneyrehm commented 5 years ago

but the type for withinString does not permit an undefined return value

@luisnaranjo733 what type? if you're referring to TypeScript you're looking for https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/urijs as this project is currently not providing type definitions itself.