Re: [PATCH] Compliance: handle HTTP OPTIONS and TRACE requests with asterisk URIs

On Tue, 31 Aug 2010 15:25:56 -0600, Alex Rousskov
<rousskov_at_measurement-factory.com> wrote:
> On 08/30/2010 09:49 PM, Amos Jeffries wrote:
>> On Mon, 30 Aug 2010 19:33:39 -0600, Alex Rousskov
>> <rousskov_at_measurement-factory.com> wrote:
>>> Compliance: handle HTTP OPTIONS and TRACE requests with asterisk URIs.
>>>
>>> Handle '*' URIs in urlParse(). This allows Squid properly respond to
>>> OPTIONS and TRACE requests with '*' URIs and Max-Forwards value of
zero.
>>
>>> Forwarding similar requests with positive Max-Forwards value is out of
>>> this change scope and still does not work, because the upstream host
and
>>
>>> port are not set.
>>>
>>> Co-Advisor test cases:
>>> test_case/rfc2616/options-bodyless-asterisk
>>> test_case/rfc2616/maxForwardsZero-OPTIONS-asterisk
>>> test_case/rfc2616/maxForwardsZero-TRACE-asterisk
>>
>>
>> * The purpose of urlCheckRequest(request) is to determine if the
request
>> is servicable and should be updated to contain the condition:
>>
>> if ((method == METHOD_TRACE || method == METHOD_OPTIONS)&&
>> (request->max_forwards == 0&& request->urlpath == "*"))
>> return 1;
>>
>> this removes the need all changes to client_side.cc.
>
> Fixed. Good point!

You did see my "oops return should be 0" for that snippet though right?

> I think we can even exit urlCheckRequest() once TRACE
> or OPTIONS method is detected, but please let me know if I am wrong.

OPTIONS is covered by the generic check of protocol == PROTO_HTTP ->
return true.
I'm not sure if OPTIONS is valid for any other protocols we accept URLs
for. If its sighted reporting the wrong thing then a case can be added
specially.

>
>> url.cc:
>>
>> * Please do the parser method check before the check for "urn:" unless
>> the
>> request: "OPTIONS urn:*" is valid and to be rejected. Which I don't
>> think
>> it is.
>
> Done. I think the two checks are mutually exclusive, but they are now
> swapped anyway.

Yes, your new one is just faster and likely more common as well.

>
>> * Please copy the request setup from the end of the function and return
>> immediately from the parser after finding "*". There is no need to run
>> through (or change) any of the validation code in this special case.
>
> Done. I hate code duplication so I moved the code you asked me to copy
> into a reusable function.
>

Fair enough.

+1.

Amos
Received on Tue Aug 31 2010 - 22:45:31 MDT