Mailing List Archive

Re: svn commit: r1906487 - /httpd/httpd/trunk/modules/dav/main/util.c
On 1/9/23 1:01 PM, jorton@apache.org wrote:
> Author: jorton
> Date: Mon Jan 9 12:01:56 2023
> New Revision: 1906487
>
> URL: http://svn.apache.org/viewvc?rev=1906487&view=rev
> Log:
> * modules/dav/main/util.c (dav_process_if_header): Fix error
> path for "Not" prefix parsing.
>
> Modified:
> httpd/httpd/trunk/modules/dav/main/util.c
>
> Modified: httpd/httpd/trunk/modules/dav/main/util.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/main/util.c?rev=1906487&r1=1906486&r2=1906487&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/dav/main/util.c (original)
> +++ httpd/httpd/trunk/modules/dav/main/util.c Mon Jan 9 12:01:56 2023
> @@ -801,8 +801,14 @@ static dav_error * dav_process_if_header
> "for the same state.");
> }
> condition = DAV_IF_COND_NOT;
> + list += 2;
> + }
> + else {
> + return dav_new_error(r->pool, HTTP_BAD_REQUEST,
> + DAV_ERR_IF_UNK_CHAR, 0,
> + "Invalid \"If:\" header: "
> + "Unexpected character in List");

Do we want to return here to save cycles or do we do another cycle in the while loop and reuse the error message from 'default:'
and thus also ignore single 'N' s that are followed by a ' \t<['?

Regards

Rüdiger
Re: svn commit: r1906487 - /httpd/httpd/trunk/modules/dav/main/util.c [ In reply to ]
On Mon, Jan 09, 2023 at 04:47:33PM +0100, Ruediger Pluem wrote:
> On 1/9/23 1:01 PM, jorton@apache.org wrote:
> > Author: jorton
> > Date: Mon Jan 9 12:01:56 2023
> > New Revision: 1906487
> >
> > URL: http://svn.apache.org/viewvc?rev=1906487&view=rev
> > Log:
> > * modules/dav/main/util.c (dav_process_if_header): Fix error
> > path for "Not" prefix parsing.
> >
> > Modified:
> > httpd/httpd/trunk/modules/dav/main/util.c
> >
> > Modified: httpd/httpd/trunk/modules/dav/main/util.c
> > URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/main/util.c?rev=1906487&r1=1906486&r2=1906487&view=diff
> > ==============================================================================
> > --- httpd/httpd/trunk/modules/dav/main/util.c (original)
> > +++ httpd/httpd/trunk/modules/dav/main/util.c Mon Jan 9 12:01:56 2023
> > @@ -801,8 +801,14 @@ static dav_error * dav_process_if_header
> > "for the same state.");
> > }
> > condition = DAV_IF_COND_NOT;
> > + list += 2;
> > + }
> > + else {
> > + return dav_new_error(r->pool, HTTP_BAD_REQUEST,
> > + DAV_ERR_IF_UNK_CHAR, 0,
> > + "Invalid \"If:\" header: "
> > + "Unexpected character in List");
>
> Do we want to return here to save cycles or do we do another cycle in
> the while loop and reuse the error message from 'default:' and thus
> also ignore single 'N' s that are followed by a ' \t<['?

It seems consistent with other error cases to return straight away, but
I'm not following the second part, can you explain more? An 'N'
followed by whitespace should be caught and treated as an error now (as
desired & expected).

Regards, Joe
Re: svn commit: r1906487 - /httpd/httpd/trunk/modules/dav/main/util.c [ In reply to ]
On 1/9/23 5:16 PM, Joe Orton wrote:
> On Mon, Jan 09, 2023 at 04:47:33PM +0100, Ruediger Pluem wrote:
>> On 1/9/23 1:01 PM, jorton@apache.org wrote:
>>> Author: jorton
>>> Date: Mon Jan 9 12:01:56 2023
>>> New Revision: 1906487
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1906487&view=rev
>>> Log:
>>> * modules/dav/main/util.c (dav_process_if_header): Fix error
>>> path for "Not" prefix parsing.
>>>
>>> Modified:
>>> httpd/httpd/trunk/modules/dav/main/util.c
>>>
>>> Modified: httpd/httpd/trunk/modules/dav/main/util.c
>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/main/util.c?rev=1906487&r1=1906486&r2=1906487&view=diff
>>> ==============================================================================
>>> --- httpd/httpd/trunk/modules/dav/main/util.c (original)
>>> +++ httpd/httpd/trunk/modules/dav/main/util.c Mon Jan 9 12:01:56 2023
>>> @@ -801,8 +801,14 @@ static dav_error * dav_process_if_header
>>> "for the same state.");
>>> }
>>> condition = DAV_IF_COND_NOT;
>>> + list += 2;
>>> + }
>>> + else {
>>> + return dav_new_error(r->pool, HTTP_BAD_REQUEST,
>>> + DAV_ERR_IF_UNK_CHAR, 0,
>>> + "Invalid \"If:\" header: "
>>> + "Unexpected character in List");
>>
>> Do we want to return here to save cycles or do we do another cycle in
>> the while loop and reuse the error message from 'default:' and thus
>> also ignore single 'N' s that are followed by a ' \t<['?
>
> It seems consistent with other error cases to return straight away, but
> I'm not following the second part, can you explain more? An 'N'
> followed by whitespace should be caught and treated as an error now (as
> desired & expected).

Sorry for the confusion. It is treated as an error now. I was referring to my
other approach were it would not be caught. I also found another case that would not be
caught by my proposal (an 'N' at the end of the string). Hence all good. Your approach
is the correct and better one.

Regards

Rüdiger
Re: svn commit: r1906487 - /httpd/httpd/trunk/modules/dav/main/util.c [ In reply to ]
On Tue, Jan 10, 2023 at 07:30:37AM +0100, Ruediger Pluem wrote:
> On 1/9/23 5:16 PM, Joe Orton wrote:
> > It seems consistent with other error cases to return straight away, but
> > I'm not following the second part, can you explain more? An 'N'
> > followed by whitespace should be caught and treated as an error now (as
> > desired & expected).
>
> Sorry for the confusion. It is treated as an error now. I was referring to my
> other approach were it would not be caught. I also found another case that would not be
> caught by my proposal (an 'N' at the end of the string). Hence all good. Your approach
> is the correct and better one.

Ah, great, thanks for looking at it! Any chance of a +1 for the 2.4
backport? Just need one more vote :)

Regards, Joe
Re: svn commit: r1906487 - /httpd/httpd/trunk/modules/dav/main/util.c [ In reply to ]
On 1/10/23 12:10 PM, Joe Orton wrote:
> On Tue, Jan 10, 2023 at 07:30:37AM +0100, Ruediger Pluem wrote:
>> On 1/9/23 5:16 PM, Joe Orton wrote:
>>> It seems consistent with other error cases to return straight away, but
>>> I'm not following the second part, can you explain more? An 'N'
>>> followed by whitespace should be caught and treated as an error now (as
>>> desired & expected).
>>
>> Sorry for the confusion. It is treated as an error now. I was referring to my
>> other approach were it would not be caught. I also found another case that would not be
>> caught by my proposal (an 'N' at the end of the string). Hence all good. Your approach
>> is the correct and better one.
>
> Ah, great, thanks for looking at it! Any chance of a +1 for the 2.4
> backport? Just need one more vote :)

r1906537

Regards

Rüdiger