Re: [PATCH] Add request_header_add option and [request|reply]_header_* manglers fixes

On 07/01/2012 02:25 PM, Amos Jeffries wrote:
> Thank you. In most places the problem is that AccessLogEntry is the
> closest thing we have to a XactionData object for the whole HTTP
> transaction. (I still have patches waiting around to polish up and
> submit for audit creating that master transaction object).
> AccessLogEntry is available from the ConnStateData / conn() almost
> everywhere in the code. So it *is* actually available everywhere through
> the transaction, just not filled out. We have a lot of code cleaning to
> make it consistent, (but outside this patches scope).

The AccessLogEntry is not available from ConnStateData, it is only
available from ClientHttpRequest, and this is correct because an
accessLogEntry is Http request related, not connection related.

>
> For now it is available via "request->conn()->ale". Or a new temporary
> one created in local scope for the function doing display output. It may
> need filling out with pointers to things required (is SSL content, HTTP
> request, client connection), the code before logging is very
> inconsistent/non-existent.

The only way to access AccessLogEntry is using the following path:
 request->conn()->getCurrentContext()->http->al

I am not feeling comfortable using the request->conn() although it looks
that works well in the current squid code. Moreover the
request->conn()->getCurrentContext()->http path, I think does not
guarantee that the ClientHttpRequest is the object we need.

Sometimes I am felling the initial design was that a (server side)
HttpRequest corresponds to more than one connections (more than one
connections accessing the same object) and of course to more than one
ClientHttpRequest objects.

Also I believe that the request->conn()->getCurrentContext() may not
return the ClientSocketContext corresponding to our HttpRequest.
We can go from ClientSocketContext to HttpRequest
(ClientSocketContext->http->request) but no the reverse.

Any opinions here?
Received on Fri Jul 06 2012 - 14:41:14 MDT