On Wed, 2007-03-07 at 23:57 +0200, Tsantilas Christos wrote: > When an http request adapted using ICAP then the client and server > addresses and the authentication information does not copied to adapted > request. > This is will cause problems in any following access control lists > proccessing.
Should this copying be configurable? In some environments, an adapted request is not a request "authorized" by the user. Can we always trust the ICAP server to essentially impersonate the user? I have no problems with committing a fix that always copies this information as a temporary measure, but I think we need to at least put some comments in the code that we are copying user-specific information to a request that did not originate from the user. In the ICAP 204 case, we should probably always copy (and eventually we will be able simply reuse the same request object). For ICAP 200, we should be more careful and at least put a TODO comment. If we want to distinguish these two cases, then ICAPModXact should do the copying and not client_side. If you want to move the copying code to ICAPModXact, you can call it from handle204NoContent() unconditionally and perhaps from parseHttpHead() with a "TODO: make this copying configurable" comment. I would be happy to commit this code one you think it is ready. HTH, Alex. > Looks that the following patch solves the problem. (But I am to tired > now for a good testing, tomorrow .....) > > Regards, > Christos > > diff -u -r1.34.4.24 client_side_request.cc > --- client_side_request.cc 14 Feb 2007 07:19:43 -0000 1.34.4.24 > +++ client_side_request.cc 7 Mar 2007 21:45:17 -0000 > @@ -1112,6 +1112,15 @@ > assert(msg); > > if (HttpRequest *new_req = dynamic_cast<HttpRequest*>(msg)) { > + new_req->client_addr = request->client_addr; > + new_req->client_port = request->client_port; > + new_req->my_addr = request->my_addr; > + new_req->my_port = request->my_port; > + new_req->flags = request->flags; > + if (request->auth_user_request) { > + new_req->auth_user_request = request->auth_user_request; > + new_req->auth_user_request->lock(); > + } > /* > * Replace the old request with the new request. > */