You are viewing a plain text version of this content. The canonical link for it is here.
Posted to c-dev@axis.apache.org by "Denis Linine (JIRA)" <ax...@ws.apache.org> on 2005/09/06 08:57:31 UTC
[jira] Created: (AXISCPP-822) Possible improvements to the HTTPTransport::getHTTPHeaders() and the HTTPTransport::flushOutput() methods - HTTPTransport.cpp file
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
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
[jira] Commented: (AXISCPP-822) Possible improvements to the HTTPTransport::getHTTPHeaders() and the HTTPTransport::flushOutput() methods - HTTPTransport.cpp file
Posted by "nadir amra (JIRA)" <ax...@ws.apache.org>.
[ http://issues.apache.org/jira/browse/AXISCPP-822?page=comments#action_12355625 ]
nadir amra commented on AXISCPP-822:
------------------------------------
Fred, sounds good. I will reset the contents to null string on an exception - and I do not use ultoa() :-) I left that as-is.
> 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
[jira] Commented: (AXISCPP-822) Possible improvements to the HTTPTransport::getHTTPHeaders() and the HTTPTransport::flushOutput() methods - HTTPTransport.cpp file
Posted by "Fred Preston (JIRA)" <ax...@ws.apache.org>.
[ 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
[jira] Assigned: (AXISCPP-822) Possible improvements to the HTTPTransport::getHTTPHeaders() and the HTTPTransport::flushOutput() methods - HTTPTransport.cpp file
Posted by "nadir amra (JIRA)" <ax...@ws.apache.org>.
[ http://issues.apache.org/jira/browse/AXISCPP-822?page=all ]
nadir amra reassigned AXISCPP-822:
----------------------------------
Assign To: nadir amra
> 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
[jira] Closed: (AXISCPP-822) Possible improvements to the HTTPTransport::getHTTPHeaders() and the HTTPTransport::flushOutput() methods - HTTPTransport.cpp file
Posted by "nadir amra (JIRA)" <ax...@ws.apache.org>.
[ http://issues.apache.org/jira/browse/AXISCPP-822?page=all ]
nadir amra closed AXISCPP-822:
------------------------------
Fix Version: 1.6 Alpha
Resolution: Fixed
> 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
> Fix For: 1.6 Alpha
>
> 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
[jira] Commented: (AXISCPP-822) Possible improvements to the HTTPTransport::getHTTPHeaders() and the HTTPTransport::flushOutput() methods - HTTPTransport.cpp file
Posted by "nadir amra (JIRA)" <ax...@ws.apache.org>.
[ http://issues.apache.org/jira/browse/AXISCPP-822?page=comments#action_12332922 ]
nadir amra commented on AXISCPP-822:
------------------------------------
I closed the potential memory leak for OS/400 and implemented the nice to do portion of this issue, but before I close it there is a question whether on HTTPTransport::flushOutput() , if an exception happens should the the header and user data buffers be reset to null string?
> 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