On Mon, 27 Jun 2011 16:25:08 +0300, Tsantilas Christos wrote:
Hi all,
When FwdServer::_peer is set, HttpStateData constructor creates a
new special HttpRequest, overwriting the request pointer set in the
parent (ServerStateData) constructor to fwd->request.
This special HttpRequest sets the proper urlpath (which maybe
different from the original HttpRequest), the host
(HttpRequest::SetHost/GetHost) to be the peer hostname and inherits
flags, protocol and method. Also sets the
HttpRequest::flags.proxying.
I believe this is originaly done to handle only the differences in
urlpath and the host. But this is has as result to have two
HttpRequests object in HttpStateData, but their difference is not
clear, and this is causes some bugs in http.cc. In patch preamble I
am
counting 4 possible bugs.
This patch removes the HttpStateData::orig_request member and uses
always the HttpStateData::request member.
If we decide that this patch is in the right direction probably we
should remove the ServerStateData::originalRequest() method too.
Regards,
Christos
I've seen one more that you don't list. Debugs() and error pages
sometimes display the cache_peer hostname as the URL requested domain
name when going to an origin. Regardless of what the virtual host name
actually is.
As to point (3), that includes caching of "private" replies as well as
no-store. Which is just as bad, if not worse. And prevents caching of
"public" replies when auth is used, not so bad, but annoying.
As to point (4), it *was* a bug, but your change fixes it. The comment
you added can be erased again.
+1.
and Thank You! that HttpStateData() illogic was blocking some of my
work :)
Amos
