<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/102696/">http://git.reviewboard.kde.org/r/102696/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 29th, 2011, 8:08 p.m., <b>Andreas Hartmetz</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/102696/diff/5/?file=37372#file37372line2314" style="color: black; font-weight: bold; text-decoration: underline;">kioslave/http/http.cpp</a>
<span style="font-weight: normal;">
(Diff revision 5)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">bool HTTPProtocol::sendQuery()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">2263</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">httpShouldCloseConnection</span><span class="p">())</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">What if no next request is sent and the connection is supposed to get closed from our side?
Waiting for the server to do it will probably work, but it would be "nicer" to do it ourself. What does the HTTP spec say?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hmm... Confused. I only moved that piece of code from the bottom part of ::sendQuery to the very top. I did not remove it completely. Did you intend this comment for another line perhaps ?</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 29th, 2011, 8:08 p.m., <b>Andreas Hartmetz</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/102696/diff/5/?file=37372#file37372line4122" style="color: black; font-weight: bold; text-decoration: underline;">kioslave/http/http.cpp</a>
<span style="font-weight: normal;">
(Diff revision 5)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void HTTPProtocol::closeConnection()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">4033</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">m_request</span><span class="p">.</span><span class="n">isKeepAlive</span> <span class="o">=</span> <span class="kc">false</span><span class="p">;</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This looks like a hack and is, I think, surprising. It is a part of connection setup, where all the other similar variables are set. Is it really necessary to move this here?
Too distributed state changes in too many places are already a big problem in this ioslave, which is why I'm very sensitive to such things.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hmm... I am confused here too. :) I completely removed that line. I did not move it there. Resetting the "isKeepAlive" flag in httpCloseConnection was a very big mistake because it will overwrite the settings we set in resetSessionSettings, especially if the connection happens to have been closed by the server already and a new one has to be established in ::sendQuery again. IOW, isKeepAlive will be set to false in that scenario when it should have been true as already set in ::resetSessionSettings.</pre>
<br />
<p>- Dawit</p>
<br />
<p>On September 27th, 2011, 9:12 p.m., Dawit Alemayehu wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kdelibs.</div>
<div>By Dawit Alemayehu.</div>
<p style="color: grey;"><i>Updated Sept. 27, 2011, 9:12 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This 5th patch in a serious of patches meant to improve proxy support in KDE deals with support at the kioslave level. More specifically the http ioslave. The patch is necessary to provide proper support for PAC script based proxy configuration. Namely allowing aleternate proxy servers to be specified and used as necessary. Here are the change this patch makes:
* Add a new function in TCPSlaveBase to connect to the a remote server without
automatically sending error notitification to the client. [NEW API]
* Move the proxy related code from 'resetSessionSettings' to 'httpOpenConnection'.
Proxy information will now be only set from 'setHost' and reset from
'reparseConfiguration' as it should have been from the beginning. No need to
reparse proxy related information on every request.
* Added a new variable, proxyUrls, to HTTPRequest to store the multiple proxy URLs
obtained from the "ProxyUrls" meta-data.
* Modified 'httpShouldCloseConnection' to account for multiple proxy addresses.
* Modified 'sendQuery' to connect to the remote server before formatting the HTTP
headers so that the headers can properly reflect the correct proxy settings.
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">- Tested changes using a real proxy server, privoxy.
- Tested changes using a poor man's SOCKS proxy setup:
ssh -D <port#> <ssh-server-address>
and the following PAC script:
function FindProxyForURL( url, host )
{
var resolved_ip = dnsResolve(host);
if (isInNet(resolved_ip, "127.0.0.1", "255.255.255.0"))
return "DIRECT";
return "SOCKS 127.0.0.1:9999; DIRECT";
}
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kio/kio/tcpslavebase.h <span style="color: grey">(3f87ea8)</span></li>
<li>kio/kio/tcpslavebase.cpp <span style="color: grey">(ec70559)</span></li>
<li>kioslave/http/http.h <span style="color: grey">(d8c47c7)</span></li>
<li>kioslave/http/http.cpp <span style="color: grey">(6d41a13)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/102696/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>