search

williamkapke / irc-connect

Minimal IRC connection that simply emits event objects. Plugin modules can do the rest.
14 stars 5 forks source link

Incorrect parsing of packets containing colons #1

Closed ApeironTsuka closed 10 years ago

ApeironTsuka commented 10 years ago

Some IRCds issue RPL_ISUPPORT packets with colons in the parameters (such as UnrealIRCd and the CHANLIMIT parameter).

Changing the regex in connect.js::getParam to /[^\0\r\n ]([^\0\r\n ](?! :)?)*/g appears to work fine, though there may be a better way to do it.

williamkapke commented 10 years ago

Thanks for pointing this out- indeed this is out of sorts.

I revisited the code and I see there are a few things that are not needed with that regex. 1) It looks like the colon should be in the first set of brackets- preventing the first character from being a colon. However, in the context of the parsing that happens in getParam, the first character is already being testing for a colon on Line 50 so it is already covered. 2) getParam is called by parse which is fed a single line at a time. The eachline module handles splitting the lines on CRLF- so there is no need to look for them either. 3) NUL is restricted by the spec because of problems with strings in C. IF a null was actually sent from the server- the parser wouldn't parse all of the parameters... sooo then what? It is unclear what the expected behavior should be. I think I'll just allow NULs to be part of the param since I don't see it being a problem in a Javascript string. (Feedback welcome)

Most importantly here: Tests are long over due.