[tao-bugs] TAO_HTTP_Handler, HTTP_Handler.cpp: HTTP_Handler fails to fetch IOR due to bugs
Johnny Willemsen
jwillemsen at remedy.nl
Wed Apr 20 04:51:17 CDT 2016
Hi,
Thanks for using the PRF form. Please open a pull request at
https://github.com/DOCGroup/ATCD/ with the necessary fixes and
extensions to our unit tests to reproduce this.
Best regards,
Johnny Willemsen
Remedy IT
Postbus 81 | 6930 AB Westervoort | The Netherlands
http://www.remedy.nl
On 04/20/2016 11:27 AM, BJovke . wrote:
> :)) Well, I've just noticed that initial "buf" is also not zero terminated.
> For strstr this is critical.
> So here is the fix with this correction included:
>
> SAMPLE FIX/WORKAROUND:
>
> int
> TAO_HTTP_Reader::receive_reply (void)
> {
> size_t num_recvd = 0;
> char buf [MTU+1];
> char *buf_ptr = 0;
> size_t bytes_read = 0;
>
> // Receive the first MTU bytes and strip the header off.
> // Note that we assume that the header will fit into MTU bytes.
> if (peer ().recv_n (buf, MTU, 0, &num_recvd) >= 0)
> {
> // Zero terminate buf.
> buf[num_recvd]=0;
> //Make sure that response type is 200 OK
> // It could be "IOR:" also!!!
> if (ACE_OS::strstr (buf,"200 OK") == 0)
> {
> // If response is pure IOR string it should begin with "IOR:" without
> any white spaces before it
> // This could be modified in future to skip leading white spaces if
> there is a possibility that there are white spaces before "IOR:" string
> if (*buf == 0 || ACE_OS::strstr (buf,"IOR:") != buf)// *buf==0 is here
> because strstr returns buf when buf is empty string
> TAOLIB_ERROR_RETURN ((LM_ERROR,
> "TAO (%P|%t) - HTTP_Reader::receive_reply, Response is not 200 OK nor
> IOR:\n" ), -1);
> }
>
> // Search for the header termination string "\r\n\r\n", or "\n\n". If
> // found, move past it to get to the data portion.
> if ((buf_ptr = ACE_OS::strstr (buf,"\r\n\r\n")) != 0)
> buf_ptr += 4;
> else if ((buf_ptr = ACE_OS::strstr (buf, "\n\n")) != 0) //for
> compatibility with JAWS
> buf_ptr += 2;
> else
> buf_ptr = buf;
>
> // Determine number of data bytes read. This is equal to the
> // total bytes read minus number of header bytes.
> bytes_read = num_recvd - (buf_ptr - buf);
>
> }
> else
> {
> TAOLIB_ERROR_RETURN ((LM_ERROR,
> "TAO (%P|%t) - HTTP_Reader::receive_reply,
> error while reading header\n"), -1);
> }
>
> // ***************************************************************
> // At this point, we have stripped off the header and are ready to
> // process data. buf_ptr points to the data
>
> ACE_Message_Block* temp = 0;
> ACE_Message_Block* curr = this->mb_;
>
> ACE_NEW_RETURN (temp,
> ACE_Message_Block (bytes_read + 1),// Added "+1" to
> make room for byte 0 to make 0 terminated string.
> -1);
> curr->cont (temp);
> curr = curr->cont ();
>
> // Copy over all the data bytes into our message buffer.
> if (curr->copy (buf_ptr, bytes_read) == -1)
> {
> TAOLIB_ERROR_RETURN ((LM_ERROR, "TAO (%P|%t) -
> HTTP_Reader::receive_reply, error copying data into Message_Block\n"), -1);
> }
>
> // 0 - terminate string
> *curr->wr_ptr () = 0;
> curr->wr_ptr (1);
>
> // read the rest of the data into a number of ACE_Message_Blocks and
> // chain them together in a link list fashion
> num_recvd = 0;
>
> do
> {
> if (curr->space () == 0)
> {
> ACE_NEW_RETURN (temp,
> ACE_Message_Block (MTU + 1),// Added "+1" to make
> room for byte 0 to make 0 terminated string.
> -1);
> curr->cont (temp);
> curr = curr->cont ();
> }
>
> if (peer ().recv_n (curr->wr_ptr (), curr->space (), 0, &num_recvd) >= 0)
> {
> // Move the write pointer
> curr->wr_ptr (num_recvd);
>
> // 0 - terminate string
> *curr->wr_ptr () = 0;
> curr->wr_ptr (1);
>
> // Increment bytes_read
> bytes_read += num_recvd;
>
> }
> else
> {
> TAOLIB_ERROR_RETURN ((LM_ERROR,
> "TAO (%P|%t) - HTTP_Reader::receive_reply,
> Error while reading header\n"), -1);
> }
> } while (num_recvd != 0);
>
> // Set the byte count to number of bytes received
> this->bytecount_ = bytes_read;
>
> return 0;
> }
>
>
> 2016-04-20 11:08 GMT+02:00 BJovke . <bjovan at gmail.com
> <mailto:bjovan at gmail.com>>:
>
> TAO VERSION: 2.3.0
> ACE VERSION: 6.3.0
>
> HOST MACHINE and OPERATING SYSTEM:
> Windows 7, Winsock2
>
> COMPILER NAME AND VERSION (AND PATCHLEVEL):
> Visual studio 2012 express VC11 version 17.00.61030 for x86
>
> THE $ACE_ROOT/ace/config.h FILE [if you use a link to a platform-
> specific file, simply state which one]:
> config-win32.h
>
> THE $ACE_ROOT/include/makeinclude/platform_macros.GNU FILE [if you
> use a link to a platform-specific file, simply state which one
> (unless this isn't used in this case, e.g., with Microsoft Visual
> C++)]:
> MSVC used
>
> CONTENTS OF $ACE_ROOT/bin/MakeProjectCreator/config/default.features
> (used by MPC when you generate your own makefiles):
> -
>
> AREA/CLASS/EXAMPLE AFFECTED:
> Failed fetching of IOR via HTTP.
>
> DOES THE PROBLEM AFFECT:
> COMPILATION?
> No.
> LINKING?
> No.
> EXECUTION?
> Yes, behavior is wrong. TAO and my application are affected.
> OTHER (please specify)?
> -
>
> SYNOPSIS:
> There are two issues noticed when fetching IOR via HTTP, by
> using orb->string_to_object() function.
> Because of these issues this functions fails in fetching
> IOR, in both cases with error.
>
> DESCRIPTION:
> First issue is that TAO_HTTP_Reader::receive_reply (void)
> function in HTTP_Handler.cpp
> expects string "200 OK" as mandatory in HTTP response.
> However, for Ericsson and NSN systems
> I was connecting with my app, URL for Naming service IOR
> points to a file on web server which
> contains IOR string. HTTP response in both Ericsson and NSN
> cases contains just IOR string, there
> are no HTTP response headers. Because of that this function
> fails in reading IOR.
>
> Second issue is a bug and it's more serious. It is in the
> same function as first issue,
> TAO_HTTP_Reader::receive_reply (void) in HTTP_Handler.cpp.
> HTTP response data is read and
> placed into list of "ACE_Message_Block"s, each of MTU size.
> These blocks are after that
> concatenated into one string in function
> TAO_HTTP_Parser::parse_string in HTTP_Parser.cpp.
> The problem is that "ACE_Message_Block"s created in
> TAO_HTTP_Reader::receive_reply are NOT
> zero terminated and TAO_HTTP_Parser::parse_string uses
> ACE_OS::strlen in
> ACE_String_Base<ACE_CHAR_T>::operator+= (const ACE_CHAR_T* s) in
> String_Base.cpp.
> This operator is used for concatenation of
> "ACE_Message_Block"s as C strings.
> "strlen" expects each of these blocks to be zero terminated
> and produces unpredictable results
> with a lot of garbage bytes in resulting IOR string.
> Luckily, while testing the code I've noticed
> that eventually "strlen" encounters zero byte after at most
> 50 bytes, well after the end of
> ACE_Message_Block, so it does not cause more damage than
> putting garbage into final IOR string.
>
> REPEAT BY:
> CORBA::Object_var
> obj=orb->string_to_object("http://xxxxxxxxxxxxxxxxxxxxxxxxxx");
> URL should be a file on web server containing IOR string and
> no HTTP response headers should be present, this is to reproduce
> only first issue.
>
> Second issue (bug) is always appearing if there is at least
> one "ACE_Message_Block" containing response which is not zero
> terminated.
> Proper zero termination could happen only by chance if the
> next byte after "ACE_Message_Block" is zero or if HTTP response
> is such that contains zero bytes, which is unlikely.
>
> SAMPLE FIX/WORKAROUND:
> Here is a complete modified TAO_HTTP_Reader::receive_reply()
> function with issues corrected.
>
> int
> TAO_HTTP_Reader::receive_reply (void)
> {
> size_t num_recvd = 0;
> char buf [MTU+1];
> char *buf_ptr = 0;
> size_t bytes_read = 0;
>
> // Receive the first MTU bytes and strip the header off.
> // Note that we assume that the header will fit into MTU bytes.
> if (peer ().recv_n (buf, MTU, 0, &num_recvd) >= 0)
> {
> //Make sure that response type is 200 OK
> // It could be "IOR:" also!!!
> if (ACE_OS::strstr (buf,"200 OK") == 0)
> {
> // If response is pure IOR string it should begin with "IOR:"
> without any white spaces before it
> // This could be modified in future to skip leading white spaces if
> there is a possibility that there are white spaces before "IOR:" string
> if (*buf == 0 || ACE_OS::strstr (buf,"IOR:") != buf)// *buf==0 is
> here because strstr returns buf when buf is empty string
> TAOLIB_ERROR_RETURN ((LM_ERROR,
> "TAO (%P|%t) - HTTP_Reader::receive_reply, Response is not 200 OK
> nor IOR:\n" ), -1);
> }
>
> // Search for the header termination string "\r\n\r\n", or
> "\n\n". If
> // found, move past it to get to the data portion.
> if ((buf_ptr = ACE_OS::strstr (buf,"\r\n\r\n")) != 0)
> buf_ptr += 4;
> else if ((buf_ptr = ACE_OS::strstr (buf, "\n\n")) != 0)
> //for compatibility with JAWS
> buf_ptr += 2;
> else
> buf_ptr = buf;
>
> // Determine number of data bytes read. This is equal to the
> // total bytes read minus number of header bytes.
> bytes_read = num_recvd - (buf_ptr - buf);
>
> }
> else
> {
> TAOLIB_ERROR_RETURN ((LM_ERROR,
> "TAO (%P|%t) - HTTP_Reader::receive_reply,
> error while reading header\n"), -1);
> }
>
> // ***************************************************************
> // At this point, we have stripped off the header and are ready to
> // process data. buf_ptr points to the data
>
> ACE_Message_Block* temp = 0;
> ACE_Message_Block* curr = this->mb_;
>
> ACE_NEW_RETURN (temp,
> ACE_Message_Block (bytes_read + 1),// Added "+1"
> to make room for byte 0 to make 0 terminated string.
> -1);
> curr->cont (temp);
> curr = curr->cont ();
>
> // Copy over all the data bytes into our message buffer.
> if (curr->copy (buf_ptr, bytes_read) == -1)
> {
> TAOLIB_ERROR_RETURN ((LM_ERROR, "TAO (%P|%t) -
> HTTP_Reader::receive_reply, error copying data into
> Message_Block\n"), -1);
> }
>
> // 0 - terminate string
> *curr->wr_ptr () = 0;
> curr->wr_ptr (1);
>
> // read the rest of the data into a number of ACE_Message_Blocks and
> // chain them together in a link list fashion
> num_recvd = 0;
>
> do
> {
> if (curr->space () == 0)
> {
> ACE_NEW_RETURN (temp,
> ACE_Message_Block (MTU + 1),// Added "+1" to
> make room for byte 0 to make 0 terminated string.
> -1);
> curr->cont (temp);
> curr = curr->cont ();
> }
>
> if (peer ().recv_n (curr->wr_ptr (), curr->space (), 0,
> &num_recvd) >= 0)
> {
> // Move the write pointer
> curr->wr_ptr (num_recvd);
>
> // 0 - terminate string
> *curr->wr_ptr () = 0;
> curr->wr_ptr (1);
>
> // Increment bytes_read
> bytes_read += num_recvd;
>
> }
> else
> {
> TAOLIB_ERROR_RETURN ((LM_ERROR,
> "TAO (%P|%t) - HTTP_Reader::receive_reply,
> Error while reading header\n"), -1);
> }
> } while (num_recvd != 0);
>
> // Set the byte count to number of bytes received
> this->bytecount_ = bytes_read;
>
> return 0;
> }
>
>
> --
>
> Jovan Bunjevački.
>
>
>
>
> --
>
> Jovan Bunjevački.
>
>
> _______________________________________________
> tao-bugs mailing list
> tao-bugs at list.isis.vanderbilt.edu
> http://list.isis.vanderbilt.edu/cgi-bin/mailman/listinfo/tao-bugs
>
More information about the tao-bugs
mailing list