slowbro/phpircd

The socket reading assuming line buffering on the writing side

Closed this issue · 3 comments

It's possible for the script to lose data since it assumes that the writing socket sends 1 or more complete lines at a time.

Basically the reading needs to be writing to a buffer, and on each read for each socket, the buffer for said socket needs to be checked.

An easy way to recreate this error is to telnet into the ircd and start typing. Instead of processing lines at a time, the server will process characters at a time. The error of losing data would require write buffering to recreate.

Basically, the idea is this (pseudo code):

socket = sockets[n]

read = socket_read(socket)

if(strlen(read) == 0) {
break because connection is closed/closing
}

buffers[n] .= str_replace(array("\r\n", "\r"), "\n", read)

if(strpos("\n", buffers[n]) !== false) {
lines = explode("\n", buffers[n])

last = array_pop(lines)
// last will be empty if the buffer ended on a line, and will contain a partial line if it did not

foreach(lines as line) {
    process line
}


buffers[n] = last

}

I'm not quite sure I get what you're saying here. I already split all input into multiple lines, should I receive more than one line in any said read:

<?php
//starting line 22 of 'ircd'
        if(false !== ($buf = $ircd->read($value))){
            if(!empty($buf)){
                $data = trim($buf);
                $user = $ircd->getUserBySocket($value);
                $data = explode("\n", str_replace("\r","\n", $data));
                foreach($data as $d){
                    $d = trim($d);
                    if(!empty($d))
                        if($user->registered === TRUE)
                            $ircd->process($d, $user);
                        else
                            $ircd->newConnection($d, $user);
                }
                unset($reads[$key]);
            } else {
                $ircd->debug("Closing Link: client ".(empty($user->prefix)?$user->address:$user->prefix).": Client Exited");
                unset($reads[$key]);
                $ircd->quit($user, 'Client Exited');
            }
        }

Right.... Your code is assuming that it is receiving at least 1 line though. For example:

telnet localhost 6600

:irc.phpircd.org NOTICE AUTH :* Looking up your hostname...
:irc.phpircd.org NOTICE AUTH :
* Found your hostname.
N:irc.phpircd.org 451 * N :You have not registered.
I:irc.phpircd.org 451 * I :You have not registered.
C:irc.phpircd.org 451 * C :You have not registered.
K:irc.phpircd.org 451 * K :You have not registered.

Most IRC clients will do line buffering. The IRC protocol is based around lines, so why not buffer lines? The problem is though, you can't assume that everything will buffer lines. For example, here you can see me typing "NICK" into telnet. Telnet does no buffering, so note that on each character, the data is flushed to the ircd. That means that $buf will be "N" and then $buf will be "I" and so on.

select() calls a socket ready for reading when it has 1 or more characters in its read buffer. So basically, if it's being buffered on the other end, due to the low bandwidth overhead of the protocol, when an IRC client sends "NICK Corbin\n" it's going be read all at once.

(It's worth noting here, that if the data is buffered, this error can still happen if the data being sent is larger than the size of 1 TCP packet's payload. Basically if the read() were called before the second packet arrived, only the content of the first packet might be read. That means you could end up with half of one line instead of a complete line and your code would treat it as two lines.)

If you don't know what I mean, I can provide a patch to fix it. Or, the easiest way to recreate an example problem is to fire up your IRCD and then telnet into it. You'll have the same thing happen to you where it processes each character as a line.

Oooohhhh... I just remembered that telnet on linux does do line buffering... Hmmm, so basically to recreate the problem you would need to make a program that writes less than a line to a socket and then flushes it. Or, the linux version of telnet may have a flag to disable buffering.

Oh- I understand. Will fix after I un-break it from SSL stuff :)