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

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! I think we can even exit urlCheckRequest() once TRACE
or OPTIONS method is detected, but please let me know if I am wrong.

> 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.

> * 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.

Thank you,

Alex.