[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:59:40 CDT 2016
Hello.
Thank you Johnny.
I will create a pull request as soon as I create the tests.
Cheers,
Joovan Bunjevacki
2016-04-25 9:45 GMT+02:00 Johnny Willemsen <jwillemsen at remedy.nl>:
> 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.
>
--
Jovan Bunjevački.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.isis.vanderbilt.edu/pipermail/tao-bugs/attachments/20160425/2fd39423/attachment-0001.html>
More information about the tao-bugs
mailing list