PRIVMSGs sent from nicks ending in an underscore aren't parsed.

[unlobito]

Hi there,

I believe I've discovered a bug in this library. When throwing a message to the parser sent by a nick ending in an underscore, the parser returns NULL.

I wrote a simple tester for this bug:

<?php
require 'src/Phergie/Irc/ParserInterface.php';
require 'src/Phergie/Irc/Parser.php';

$parser = new \Phergie\Irc\Parser();
var_dump($parser->parse(":henri_!~hwatson@henriwatson.com PRIVMSG #test :test\r\n"));

var_dump($parser->parse(":henri!~hwatson@henriwatson.com PRIVMSG #test :test\r\n"));

Running this will output the following (truncated for brevity):

NULL
array(7) {
["prefix"]=>
string(31) ":henri!~hwatson@henriwatson.com"

Thanks.

[elazar]

Ah, I think I see the issue: the <nick> pattern in RFC 1459 Section 2.3.1, or more specifically the <special> subpattern used by <nick>, did not include the underscore as a valid character. However, it looks like <special> was amended in RFC 2812 Section 2.3.1 to include the underscore.

No patterns other than <nick> use <special>, so the two are consolidated in the parser implementation. As such, fixing this should be as simple as adding an underscore (escaped with a double backslash since it's a regular expression metacharacter) to the $nick pattern defined here: https://github.com/phergie/phergie-irc-parser/blob/master/src/Phergie/Irc/Parser.php#L209

I'll try to add a fix and corresponding unit test update this evening unless you care to beat me to it. :) Thanks for reporting this issue!

[unlobito]

Alright, went ahead and added a fix for this and a testcase in a1d2359.
If you'd like, I can open a pull request for it.

You're welcome. :)

[elazar]

The changes look good, though because they and unrelated changes from PR #2 are both based on your master branch, github is confusing them. PRs are based on branches; if you want to amend a PR, you simply make more commits to its branch. If you base two PRs on the same branch, github has trouble following which changes are specific to which PR.

[elazar]

Actually, one small change: per my comment from PR #1, the added ACTION test case should be commented (which you've done) and moved above the FINGER test case (which still needs to be done). At that point, I can merge both PRs #1 and #2. In the future, please base each PR on its own branch. Thanks!