[tao-bugs] TAO_HTTP_Handler, HTTP_Handler.cpp: HTTP_Handler fails to fetch IOR due to bugs

BJovke . bjovan at gmail.com
Mon Apr 25 02:22:01 CDT 2016


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>:

> 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
> >
>



-- 

Jovan Bunjevački.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.isis.vanderbilt.edu/pipermail/tao-bugs/attachments/20160425/5798f5d3/attachment-0001.html>


More information about the tao-bugs mailing list