[ http://issues.apache.org/jira/browse/AXISCPP-822?page=comments#action_12355605 ]
Fred Preston commented on AXISCPP-822: -------------------------------------- Hi Nadir, This is really just a 'tidy up' of what is a very scrappy bit of code. The 'NB' comment about SSL is not correct as at this point the message has not been encrypted (this happens at the point of transmission in the channel DLL). Getting rid of unnecessary intermediate steps will speed up the code and as long as it is commented, should not detract from being easy to understand. A buf size greater than 8 sounds like a good idea as that would only allow a message size of 9,999,999 bytes before it would overflow (it sounds big until you think of bitmaps and alike!). The only caveat is that I don't know if the function, 'ultoa()' is available in all compilers. I would assume that it would be a good idea to reset data buffers to an empty string when an exception occurs as if one does occur at this point, then there must be something fundamentally wrong with the socket and the recovery will not be trivial. Without looking more deeply into the whole message creation/handling process, I would not reset any pointer to null, just their content. Regards, Fred Preston. > Possible improvements to the HTTPTransport::getHTTPHeaders() and the > HTTPTransport::flushOutput() methods - HTTPTransport.cpp file > ----------------------------------------------------------------------------------------------------------------------------------- > > Key: AXISCPP-822 > URL: http://issues.apache.org/jira/browse/AXISCPP-822 > Project: Axis-C++ > Type: Improvement > Components: Transport (axis3) > Versions: current (nightly) > Reporter: Denis Linine > Assignee: nadir amra > Priority: Trivial > > Hello, > HTTPTransport::flushOutput() could be written like this (some current code > is commented out): > AXIS_TRANSPORT_STATUS HTTPTransport::flushOutput() throw (AxisException, > HTTPTransportException) > { > // In preperation for sending the message, calculate the size of the > message > // by using the string length method. > // NB: This calculation may not necessarily be correct when dealing with > SSL > // messages as the length of the encoded message is not necessarily > the > // same as the length of the uncoded message. > > // char buff[8]; // theoretically, a 8-char-long buffer can be too > small even for 32 bit systems > char buff[24]; > //sprintf( buff, "%d", m_strBytesToSend.length ()); > //this->setTransportProperty ("Content-Length", buff); > > // two lines above can be replaced like this (ultoa should work faster > than sprintf): > setTransportProperty ("Content-Length", ultoa(m_strBytesToSend.length > (), buff, 10)); > // The header is now complete. The message header and message can now be > // transmitted. > // utf8Buf will leak if an exception is thrown, catching different > types of exceptions just to rethrow them is excessive > > // try > // { > //#ifndef __OS400__ > // *m_pActiveChannel << this->getHTTPHeaders (); > // *m_pActiveChannel << this->m_strBytesToSend.c_str (); > //#else > // const char *buf = this->getHTTPHeaders (); > // char *utf8Buf = toUTF8((char *)buf, strlen(buf)+1); > // *m_pActiveChannel << utf8Buf; > // free(utf8Buf); > // buf = this->m_strBytesToSend.c_str(); > // utf8Buf = toUTF8((char *)buf, strlen(buf)+1); > // *m_pActiveChannel << utf8Buf; > // free(utf8Buf); > //#endif > // } > // catch( HTTPTransportException & e) > // { > // throw; > // } > // catch( AxisException & e) > // { > // throw; > // } > // catch(...) > // { > // throw; > // } > char *utf8Buf = NULL; > try > { > #ifndef __OS400__ > *m_pActiveChannel << getHTTPHeaders (); > *m_pActiveChannel << m_strBytesToSend.c_str (); > #else > const char *buf = this->getHTTPHeaders (); > utf8Buf = toUTF8((char *)buf, strlen(buf)+1); > *m_pActiveChannel << utf8Buf; > free(utf8Buf); > utf8Buf = NULL; > // 5 lines above could probably be rewritten like this: > // getHTTPHeaders(); > // utf8Buf = toUTF8(m_strHeaderBytesToSend.c_str(), > m_strHeaderBytesToSend.length()+1); // eliminate strlen; is > const_cast<char*>(m_strBytesToSend.c_str()) necessary (what type of the > toUTF8 first parameter)? > // free(utf8Buf); > // utf8Buf = NULL; > buf = m_strBytesToSend.c_str(); > utf8Buf = toUTF8(m_strBytesToSend.c_str(), > m_strBytesToSend.length()+1); // eliminate strlen; is > const_cast<char*>(m_strBytesToSend.c_str()) necessary (what type of the > toUTF8 first parameter)? > *m_pActiveChannel << utf8Buf; > free(utf8Buf); > utf8Buf = NULL; > #endif > } > catch(...) > { > free(utf8Buf); > // might be one should empty strings? > // m_strBytesToSend.clear(); > // m_strHeaderBytesToSend.clear(); > throw; > } > // m_strHeaderBytesToSend seem to be used only by this function. > // Is it not possible to make them local variables for flushOutput() > and pass > // Empty the bytes to send string. > //m_strBytesToSend = ""; > //m_strHeaderBytesToSend = ""; > m_strBytesToSend.clear(); // ? > m_strHeaderBytesToSend.clear(); // ? > return TRANSPORT_FINISHED; > } > The first lines of the HTTPTransport::getHTTPHeaders(): > const char * HTTPTransport::getHTTPHeaders() > { > URL & url = m_pActiveChannel->getURLObject(); > unsigned short uiPort = url.getPort(); > char buff[8]; > m_strHeaderBytesToSend = m_strHTTPMethod + " "; > if (m_bUseProxy) > m_strHeaderBytesToSend += std::string (url.getURL ()) + " "; > else > m_strHeaderBytesToSend += std::string (url.getResource ()) + " > "; > m_strHeaderBytesToSend += m_strHTTPProtocol + "\r\n"; > if (m_bUseProxy) > m_strHeaderBytesToSend += std::string ("Host: ") + m_strProxyHost; > else > m_strHeaderBytesToSend += std::string ("Host: ") + > url.getHostName (); > > if (m_bUseProxy) > uiPort = m_uiProxyPort; > > sprintf (buff, "%u", uiPort); > m_strHeaderBytesToSend += ":"; > m_strHeaderBytesToSend += buff; > m_strHeaderBytesToSend += "\r\n"; > could probably be rewritten this way (eliminate creation of temporary > strings, eliminate excessive if/else): > const char * HTTPTransport::getHTTPHeaders() > { > URL & url = m_pActiveChannel->getURLObject(); > unsigned short uiPort; > // char buff[8]; // theoretically, a 8-char-long buffer can be too > small even for 32 bit systems > char buff[32]; > m_strHeaderBytesToSend = m_strHTTPMethod + " "; > if (m_bUseProxy) > { > m_strHeaderBytesToSend += url.getURL (); > m_strHeaderBytesToSend += " "; > m_strHeaderBytesToSend += m_strHTTPProtocol; > m_strHeaderBytesToSend += "\r\nHost: "; > m_strHeaderBytesToSend += m_strProxyHost; > uiPort = m_uiProxyPort; > } > else > { > m_strHeaderBytesToSend += url.getResource (); > m_strHeaderBytesToSend += " "; > m_strHeaderBytesToSend += m_strHTTPProtocol; > m_strHeaderBytesToSend += "\r\nHost: "; > m_strHeaderBytesToSend += url.getHostName (); > uiPort = url.getPort(); > } > sprintf(buff, ":%u\r\n", uiPort); > m_strHeaderBytesToSend += buff; > Note please that this code was nether tested nor even compiled -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira
