Re: [PATCH] Squid host rewrite for intercepted https requests

Hello Alex!

On Thu, 2012-06-07 at 17:22 -0600, Alex Rousskov wrote:
> On 06/07/2012 10:02 AM, Alexander Komyagin wrote:
> > Hi! I've found that Squid (as of 3.2.0.16) is still overwriting clients
> > requests to HTTP if they are intercepted:
> >
> > src/client_side.cc:prepareTransparentURL():
> >
> > snprintf(http->uri, url_sz, "http://%s%s", /*conn->port->protocol,*/
> > host, url);
> >
> > When I want to intercept https traffic this one really breaks the things
> > down. I have not deepened into the details, but it seems that everything
> > works fine (simultaneous http/https) when I use conn->port->protocol and
> > force protocol to http if it is NULL.
> >
> > Patch is attached.
>
> Hello Alexander,
>
> It looks like your patch is changing more than just the lines that
> format the URL. We made a similar but more compact change for the
> bump-server-first project:
>
> > static void
> > prepareTransparentURL(ConnStateData * conn, ClientHttpRequest *http, char *url,
> > const char *req_hdr)
> > {
> > char *host;
> > char ipbuf[MAX_IPSTRLEN];
> >
> > if (*url != '/')
> > return; /* already in good shape */
> >
> > /* BUG: Squid cannot deal with '*' URLs (RFC2616 5.1.2) */
> > // BUG 2976: Squid only accepts intercepted HTTP.
> >
> > if ((host = mime_get_header(req_hdr, "Host")) != NULL) {
> > int url_sz = strlen(url) + 32 + Config.appendDomainLen +
> > strlen(host);
> > http->uri = (char *)xcalloc(url_sz, 1);
> > - snprintf(http->uri, url_sz, "http://%s%s", /*conn->port->protocol,*/ host, url);
> > + snprintf(http->uri, url_sz, "%s://%s%s", conn->port->protocol, host, url);
> > debugs(33, 5, "TRANSPARENT HOST REWRITE: '" << http->uri <<"'");
> > } else {
> > /* Put the local socket IP address as the hostname. */
> > int url_sz = strlen(url) + 32 + Config.appendDomainLen;
> > http->uri = (char *)xcalloc(url_sz, 1);
> > http->getConn()->clientConnection->local.ToHostname(ipbuf,MAX_IPSTRLEN),
> > - snprintf(http->uri, url_sz, "http://%s:%d%s",
> > - // http->getConn()->port->protocol,
> > + snprintf(http->uri, url_sz, "%s://%s:%d%s",
> > + http->getConn()->port->protocol,
> > ipbuf, http->getConn()->clientConnection->local.GetPort(), url);
> > debugs(33, 5, "TRANSPARENT REWRITE: '" << http->uri << "'");
> > }
> > }
>
>
> Have you seen the port protocol being NULL? It feels like that should
> not be possible. If it is never NULL, I think the above changes are
> preferred.

Nope. Never seen. But the guy who reported *BUG 2976* certainly did. See
http://www.squid-cache.org/mail-archive/squid-dev/201103/0020.html

>
> The "," typo should also be fixed, of course, but that fix is not
> related to the protocol changes discussed here.
>

Indeed. But anyway I spent some time figuring out why compiler says
nothing about it ;)

>
> Thank you,
>
> Alex.
>

-- 
Best wishes,
Alexander Komyagin