codeminders / php-amqplib

PHP library implementing Advanced Message Queuing Protocol (AMQP).
GNU Lesser General Public License v2.1
0 stars 0 forks source link

AMQPReader::rawread goes into an infinite loop on premature EOF #12

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
From amqp_wire.inc:AMQPReader -

            while ($read < $n && (false !== ($buf = fread($this->sock, $n - $read))))

While this correctly detects errors, premature (unexpected) EOFs cause fread to 
return "", not false. 
The loop can be corrected:

            while ($read < $n && !feof($this->sock) && (false !== ($buf = fread($this->sock, $n - 
$read))))

Original issue reported on code.google.com by angrybal...@gmail.com on 6 Jan 2010 at 6:32

GoogleCodeExporter commented 9 years ago
I have attached a patch that fixes this bug.
Hope this helps.

Original comment by tom.bio...@gmail.com on 23 Feb 2010 at 12:16

Attachments:

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Lyolik please review these bugs.

Original comment by kroko...@gmail.com on 21 Mar 2010 at 9:17

GoogleCodeExporter commented 9 years ago
The issue12-amqplib.patch  worked for me when I encountered the problem. Please 
commit it.

Original comment by quasikeith on 1 Nov 2010 at 11:56

GoogleCodeExporter commented 9 years ago
committed. thanks

Original comment by kroko...@gmail.com on 3 Nov 2010 at 3:14

GoogleCodeExporter commented 9 years ago
Important:: This patch was mis-applied and evidently not tested.  Trunk is now 
broken and causing users to seek other libraries (e.g. 
http://lists.rabbitmq.com/pipermail/rabbitmq-discuss/2010-November/009957.html)

While I don't understand the specific issue here, tom.bio's patch (in comment 
2) appears valid and probably should have been applied.

But the OP's (angrybaldguy) code is incorrect and was applied to trunk.
So the code in trunk (like OP's) is now attempting to pass the BufferedInput 
directly to feof() and fread(), which is illegal and causes the following 
errors:
Warning: feof() expects parameter 1 to be resource, object given in 
amqp_wire.inc on line 303
Warning: fread() expects parameter 1 to be resource, object given in 
amqp_wire.inc on line 304

The "correct" code (as in the patch) looks like this:
            while (($read < $n && !feof($this->sock->real_sock())) && 
                   (false !== ($buf = fread($this->sock->real_sock(), $n - $read))))
and includes a definition of real_sock() in BufferedInput.

Original comment by dan.kam...@gmail.com on 10 Dec 2010 at 8:53

GoogleCodeExporter commented 9 years ago
Hi Dan,

I'm sorry to hear that my patch has caused problems for users. The patch I 
submitted is based on the state of php-amqplib several months ago; while it 
worked at the time (we're still using it, in fact), obviously the library has 
moved on. Thanks for figuring out what the problem was and correcting it, 
though.

-o

Original comment by angrybal...@gmail.com on 10 Dec 2010 at 12:29

GoogleCodeExporter commented 9 years ago
dan, could you sumbut a formal patch

Original comment by kroko...@gmail.com on 10 Dec 2010 at 5:17

GoogleCodeExporter commented 9 years ago
OK attached is a patch, BUT some caveats:
1. I don't understand what this code is doing and have not tried to figure out.
2. I've never used this library before, but was just trying to get the included 
test to run.
3. I may not even use this library at all.

With that said, this patch should at least allow the code to function at all 
and the test to run (without invalid PHP caused by angrybaldguy's older code 
being applied to the newer trunk as it was).

It seems this project isn't really under active development any more, and 
patches are being applied without testing.  This status should really be 
indicated prominently, or at least this library unlinked from the RabbitMQ site 
(http://www.rabbitmq.com/how.html#clients).

I opened a discussion here regarding different AMQP PHP clients if anybody 
wants to chime in: 
http://stackoverflow.com/questions/4405992/best-php-client-library-for-accessing
-rabbitmq-amqp/4410607

Original comment by dan.kam...@gmail.com on 10 Dec 2010 at 9:22

Attachments: