php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #64670 Reading message body is broken, when content-length is declared
Submitted: 2013-04-18 18:41 UTC Modified: 2014-12-13 18:00 UTC
Votes:1
Avg. Score:5.0 ± 0.0
Reproduced:0 of 0 (0.0%)
From: mi+php at aldan dot algebra dot com Assigned: pierrick (profile)
Status: Closed Package: stomp (PECL)
PHP Version: Irrelevant OS: All
Private report: No CVE-ID: None
 [2013-04-18 18:41 UTC] mi+php at aldan dot algebra dot com
Description:
------------
When an incoming message has the content-length header, the current (1.0.5) version of the stomp.c calls stomp_recv(stomp, f->body, f->body_length) -- once.

That function calls either recv(2) or SSL_read(3) -- once. Unfortunately, recv(2) (not sure about SSL_read) may not read the whole message at once -- it depends on the OS and the network.

In our testing here, for example, the first recv call would read 5472 of the body, even thought the full message was 27088 (as was correctly specified in the content-length).

As long as stomp-1.0.5 is used on both sending and receiving sides, there is no work-around -- the sending part will always set the header (correctly).

The included patch fixes this problem by calling stomp_recv repeatedly until it either reads the full body or returns an error.

The patch also refactors stomp.c a little to:

1. Declare functions not referenced in other parts of the package static.
2. Ensure, stomp_error() has something to say in ALL cases, when a FALSE/NULL
   is returned
3. Change stomp_set_error() to be printf-like -- and modify the callers to
   take advantage of the functionality instead of cooking up their own.
4. In stomp_read_buffer and stomp_read_line use realloc() instead of allocating
   a whole new piece of memory for the result and memcpy-ing data. Also remove
   checking for conditions that are always true.
5. Change the style to have function-names begin on the new line -- so that
   searching for ^functionname finds the function's definition.
6. Remove white-space (tabs and spaces) at ends of lines.


Patches

stomp-patch-content-length (last revision 2013-05-06 21:20 UTC by mi+php at aldan dot algebra dot com)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2013-04-25 18:48 UTC] pierrick@php.net
-Assigned To: +Assigned To: pierrick
 [2013-05-06 21:19 UTC] mi+php at aldan dot algebra dot com
More careful reading of the STOMP spec (both 1.0 and 1.1) tells me, that:

a) there is nothing in 1.0 spec about newlines ("\n") following the frame-ending null-byte ("\0");
b) the 1.1 spec allows for none or multiple such newlines -- which are to be ignored.

The patch I'm about upload changes the code to not insist on any particular number of newlines at the end of a frame -- it will simply drain ALL of the newlines that have arrived with the frame (without waiting for any).

The stomp_select_ex() is also modified to return not just when the socket is readable, but also when something other than a newline is there to be read.

Finally, stomp_read_line can be asked to "drain" any newlines, which may have arrived between the previous frame was read and the current one arrived.
 [2014-12-13 18:00 UTC] pierrick@php.net
-Status: Assigned +Status: Closed
 [2014-12-13 18:00 UTC] pierrick@php.net
Thank you for your bug report. This issue has already been fixed
in the latest released version of Stomp, which you can download at 
http://pecl.php.net/stomp
 
PHP Copyright © 2001-2020 The PHP Group
All rights reserved.
Last updated: Wed Oct 28 09:01:28 2020 UTC