On 19/05/11 03:50, Alex Rousskov wrote:
On 05/18/2011 09:36 AM, Alex Rousskov wrote:
On 05/18/2011 08:52 AM, Amos Jeffries wrote:
This patch includes two bug fixes in URL handling which were uncovered
during testing of the URL logging update:
* URL re-write handling was not correctly creating its adapted request
copy. The code here is much reduced by using the clone() method. Still
not completely satisfactory (marked with XXX) since on errors there is a
wasted cycles cloning.
Wasting a few cycles on errors would be OK, I guess, but there is also
at least a possibility of a memory leak: HttpRequest::clone() always
produces a new request, but AFAICT urlParse() sometimes returns NULL
instead of the request, without deleting the request parameter.
Even worse, if the new url is a urn, parseUrl returns a brand new request:
} else if (!strncmp(url, "urn:", 4)) {
return urnParse(method, url);
which means that our attempt to use a cloned request failed _and_ we now
have two requests: the one created by urnParse(), which lacks clone
information, and the one created by clone(), which lacks the new URL.
I do not know whether anybody still cares about URNs these days, but
this code path is not going to work well.
Arg. What a catch.
The core problem here may be in trying to combine two different
interfaces into one:
1. Trying to give an old request a new method/URL. This should be done
inside HttpRequest::resetUrl() or similar. This will not create new
requests or delete old ones.
2. Trying to create a new request using a method/URL pair. This can be
done as a stand-alone function or a static HttpRequest method that
returns a new request. This function should not have a request as a
parameter.
Yes. Pretty much what I've done to detach them in the test branch. Looks
like I'll have to push that forward a bit further.
As a workaround I'll look at making urlParse and urnParse have the same
API pass-thru behaviour.
Thanks.
Amos
--
Please be using
Current Stable Squid 2.7.STABLE9 or 3.1.12
Beta testers wanted for 3.2.0.7 and 3.1.12.1
