Proxy Cookie Support (Bug #10722)

[Nick Kew <ni...@webthing.com>]

I recently patched bug 10722.  My patch was against 2.0.49, for a Client
who needed it in a hurry.

I'm just porting it to 2.1-HEAD.  In doing so, I find there's an existing
patch that saves and restores cookies without rewriting the Domain and
Path components.  AFAICS my patch would/should supersede that one.
But I'm reluctant to do so without understanding the purpose of the
other patch.

The other patch is at
http://cvs.apache.org/viewcvs.cgi/httpd-2.0/modules/proxy/proxy_http.c?r1=1.184&r2=1.185

It appears just to merge Set-Cookie headers in r->err_headers_out with
those in r->headers_out.  The latter have just come from the backend
(the server proxied).  But how/why should there be (any) cookies in
r->err_headers_out at this point?  Presumably they'd be from the proxy
rather than the backend?  And why merge them into a normal 2xx response?


-- 
Nick Kew

Re: Proxy Cookie Support (Bug #10722)

[Joe Schaefer <jo...@sunstarsys.com>]

Jim Jagielski <ji...@jaguNET.com> writes:

> Joe Schaefer wrote:
> > 
> > But is the err_headers_out logic in proxy_http.c HEAD really ok?
> > It appears to put a copy of r->err_headers_out into r->headers_out,
> > so I'm wondering why this doesn't produce duplicates in the final
> > response (ie. doesn't httpd add r->err_headers_out itself, in
> > ap_http_header_filter)?
> > 
> 
> It overlays the 2, so yes.

Just to be sure we're on the same page- you agree here that HEAD
will erroneously produce duplicates of any "Set-Cookie" headers
in the proxy server's r->err_headers_out table.

-- 
Joe Schaefer

Re: Proxy Cookie Support (Bug #10722)

[Jim Jagielski <ji...@jaguNET.com>]

On Jun 25, 2004, at 9:36 AM, Nick Kew wrote:
>
> OK, on further consideration after posting, I've reached the view that
> the 2.0.49 patch should be modified.  I'd propose to run the same
> transformations on Set-Cookie headers from the backend, but move
> the rewriting - along with the rewriting of Date and Location headers
> (ap_proxy_date_canon and ap_proxy_location_reverse_map) into
> ap_proxy_read_headers.  That actually both cleans up the code (reduces
> adhockery) and makes it more efficient (reduces table operations),
> as well as making my patch orthogonal to yours.
>

The patch was never intended to be the full and complete
solution, rather a specific fix to a specific problem. The
emails noted that ideally we could select which headers to
honor/keep in a proxied environment instead of hardcoding
the assumption of "just" cookies.

See #27023

Re: Proxy Cookie Support (Bug #10722)

[Nick Kew <ni...@webthing.com>]

On 25 Jun 2004, Joe Schaefer wrote:

> Jim Jagielski <ji...@jaguNET.com> writes:
>
> > Nick Kew wrote:
>
> [...]
>
> > > It appears just to merge Set-Cookie headers in r->err_headers_out with
> > > those in r->headers_out.  The latter have just come from the backend
> > > (the server proxied).  But how/why should there be (any) cookies in
> > > r->err_headers_out at this point?  Presumably they'd be from the proxy
> > > rather than the backend?  And why merge them into a normal 2xx response?
> > >
> >
> > This is so Cookies added my the local proxy server (Apache) via
> > internal custom modules do not loose those cookies when used also
> > as a proxy. If a module has added Cookie information we should
> > honor that and maintain it as is. Roy and I talked about
> > this and both agreed that it made sense hence the patch.

OK, on further consideration after posting, I've reached the view that
the 2.0.49 patch should be modified.  I'd propose to run the same
transformations on Set-Cookie headers from the backend, but move
the rewriting - along with the rewriting of Date and Location headers
(ap_proxy_date_canon and ap_proxy_location_reverse_map) into
ap_proxy_read_headers.  That actually both cleans up the code (reduces
adhockery) and makes it more efficient (reduces table operations),
as well as making my patch orthogonal to yours.

> But is the err_headers_out logic in proxy_http.c HEAD really ok?

That I still find puzzling.  Perhaps there's a URL into the mail
archives from your previous discussion that would explain it?

-- 
Nick Kew

Re: Proxy Cookie Support (Bug #10722)

[Joe Schaefer <jo...@sunstarsys.com>]

Jim Jagielski <ji...@jaguNET.com> writes:

> Nick Kew wrote:

[...]

> > It appears just to merge Set-Cookie headers in r->err_headers_out with
> > those in r->headers_out.  The latter have just come from the backend
> > (the server proxied).  But how/why should there be (any) cookies in
> > r->err_headers_out at this point?  Presumably they'd be from the proxy
> > rather than the backend?  And why merge them into a normal 2xx response?
> > 
> 
> This is so Cookies added my the local proxy server (Apache) via
> internal custom modules do not loose those cookies when used also
> as a proxy. If a module has added Cookie information we should
> honor that and maintain it as is. Roy and I talked about
> this and both agreed that it made sense hence the patch.

But is the err_headers_out logic in proxy_http.c HEAD really ok?
It appears to put a copy of r->err_headers_out into r->headers_out,
so I'm wondering why this doesn't produce duplicates in the final
response (ie. doesn't httpd add r->err_headers_out itself, in
ap_http_header_filter)?


-- 
Joe Schaefer