[tao-bugs] TAO_HTTP_Handler, HTTP_Handler.cpp: HTTP_Handler fails to fetch IOR due to bugs
Johnny Willemsen
jwillemsen at remedy.nl
Mon Apr 25 02:45:21 CDT 2016
Hi,
See TAO/tests/ for all current tests we have. Maybe start with a copy of
TAO/tests/Hello and use a small perl based http server to simulate your
webserver. The run_test.pl can be adapted for that, your unit test
should fail without the patch, success with the patch.
Best regards,
Johnny Willemsen
Remedy IT
Postbus 81 | 6930 AB Westervoort | The Netherlands
http://www.remedy.nl
On 04/25/2016 09:22 AM, BJovke . wrote:
> Hello.
>
> I'm not sure how to create test for this.
> I can create the full test with all the code and Perl script to run it
> but in my own way of coding.
> I see that there is some specific form of code in which the tests should
> be made.
> Do you have some info on how to create the test in required form?
> Sorry for the questions, I really want to create this request but I
> don't know where to find more information.
>
> Thank you and greetings,
> Jovan Bunjevački.
>
>
> 2016-04-20 11:51 GMT+02:00 Johnny Willemsen <jwillemsen at remedy.nl
> <mailto:jwillemsen at remedy.nl>>:
>
> 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>
> > <mailto: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
> <mailto:tao-bugs at list.isis.vanderbilt.edu>
> > http://list.isis.vanderbilt.edu/cgi-bin/mailman/listinfo/tao-bugs
> >
>
>
>
>
> --
>
> Jovan Bunjevački.
More information about the tao-bugs
mailing list