#24727 [Ana]: Net::NNTP getBody() breaks binary data with trim()

ID: 24727 Updated by: arnaud@php.net Reported By: meebey@php.net Status: Analyzed Bug Type: PEAR related Operating System: Debian PHP Version: Irrelevant -Assigned To: +Assigned To: heino New Comment: Heino: I saw you committed a patch to CVS, can you close this bug ? Previous Comments: ------------------------------------------------------------------------ [2003-07-31 05:51:01] heino at gehlsen dot dk The patch seems fine, but the code is still not binary safe in some rare cases... (feel free to commit and close - I haven't been granted access to the cvs yet) ------------------------------------------------------------------------ [2003-07-31 04:43:47] meebey@php.net nicos talk to heino, he is the new package maintainer of NNTP. ------------------------------------------------------------------------ [2003-07-31 04:31:04] nicos@php.net Okay here is a general patch, I need comments before commiting it, anyone? Index: NNTP.php =================================================================== RCS file: /repository/pear/Net_NNTP/NNTP.php,v retrieving revision 1.14 diff -u -u -r1.14 NNTP.php --- NNTP.php 4 Jan 2003 11:55:49 -0000 1.14 +++ NNTP.php 31 Jul 2003 09:30:26 -0000 @@ -215,7 +215,7 @@ } $post = null; while (!feof($this->fp)) { - $line = trim(fgets($this->fp, 256)); + $line = rtrim(fgets($this->fp, 256), "\r\n"); if ($line == ".") { break; @@ -260,8 +260,7 @@ fputs($this->fp, "POST\n"); /* The servers' response */ - $response = trim(fgets($this->fp, 128)); - + $response = rtrim(fgets($this->fp, 128), "\r\n"); fputs($this->fp, "From: $from\n"); fputs($this->fp, "Newsgroups: $newsgroup\n"); fputs($this->fp, "Subject: $subject\n"); @@ -270,7 +269,7 @@ fputs($this->fp, "\n$body\n.\n"); /* The servers' response */ - $response = trim(fgets($this->fp, 128)); + $response = rtrim(fgets($this->fp, 128), "\r\n"); return $response; } @@ -295,7 +294,7 @@ $headers = ''; while(!feof($this->fp)) { - $line = trim(fgets($this->fp, 256)); + $line = rtrim(fgets($this->fp, 128), "\r\n"); if ($line == '.') { break; @@ -379,7 +378,7 @@ $body = null; while (!feof($this->fp)) { - $line = trim(fgets($this->fp, 256)); + $line = rtrim(fgets($this->fp, 128), "\r\n"); if ($line == '.') { break; @@ -408,7 +407,7 @@ { $body = array(); while(!feof($this->fp)) { - $line = trim(fgets($this->fp, 256)); + $line = rtrim(fgets($this->fp, 128), "\r\n"); if ($line == '.') { break; } else { ------------------------------------------------------------------------ [2003-07-21 09:41:49] developer at heino dot gehlsen dot dk using str_replace() on the whole string would make some unwanted overhead! using rtrim() would do the job ------------------------------------------------- - $line = trim(fgets($this->fp, 256)); + $line = rtrim(fgets($this->fp, 256), "\r\n"); ------------------------------------------------- Tthe code will still not be binary safe, since rfc977 handles $line[0]=='.' by doubling the '.' to '..' ------------------------------------------------------------------------ [2003-07-20 10:57:28] meebey@php.net Description: ------------ getBody() of the NNTP class does a trim() for each time it reads data from the server, this breaks binary data. It should just cut the \r\n with a str_replace or something similiar, the trim() function removes chars which can be part of binary data. Here the patch: --- NNTP.php 2003-07-20 17:55:30.000000000 +0200 +++ NNTPpatched.php 2003-07-20 17:55:52.000000000 +0200 @@ -377,7 +377,7 @@ $body = null; while (!feof($this->fp)) { - $line = trim(fgets($this->fp, 256)); + $line = str_replace("\r\n", '', fgets($this->fp, 256)); if ($line == '.') { break; ------------------------------------------------------------------------ -- Edit this bug report at http://bugs.php.net/?id=24727&edit=1