httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rainer Jung <rainer.j...@kippdata.de>
Subject Re: Empty Reason Phrase (BZ 44995/45092)
Date Tue, 20 May 2008 22:25:20 GMT
Ruediger Pluem schrieb:
> On 05/20/2008 08:52 PM, Jeff Trawick wrote:
>> On Tue, May 20, 2008 at 8:56 AM, Rainer Jung <rainer.jung@kippdata.de> 
>> wrote:
>>> It seems that httpd 2.0 and 2.2 require a non empty reason phrase in the
>>> status line. RFC 2616 allows an empty reason phrase:
>>>
>>>  6.1 Status-Line
>>>
>>>  Status-Line = HTTP-Version SP Status-Code SP Reason-Phrase CRLF
>>>
>>>  6.1.1 Status Code and Reason Phrase
>>>
>>>  Reason-Phrase  = *<TEXT, excluding CR, LF>
>>>
>>> Because of the star (*) I read this as "empty reason phrase is allowed".
>>
>> right, there can be 0 repetitions
>>
>> my bad ;)
>>
>>> Do you agree, that empty reason phrases should be allowed?
>>
>> yes
>>
>>> If so, the below code fragments should be reviewed. I could provide the
>>> (trivial) patch.
>>
>> replace "<=" with "<"
>>
>>> Furthermore there is a second, related problem (for 2.x *and* 1.3): 
>>> error
>>> pages use the status line as a title. If the line has an empty reason 
>>> phrase
>>> *and* uses a custom http status code, error pages will show the title 
>>> for
>>> status code 500.
>>
>> looks like this code:
>>
>> Index: modules/http/http_protocol.c
>> ===================================================================
>> --- modules/http/http_protocol.c        (revision 658385)
>> +++ modules/http/http_protocol.c        (working copy)
>> @@ -1235,12 +1235,12 @@
>>           * with the 3 digit status code
>>           */
>>          if (r->status_line != NULL
>> -            && strlen(r->status_line) > 4       /* long enough */
>> +            && strlen(r->status_line) >= 4       /* long enough */
>>              && apr_isdigit(r->status_line[0])
>>              && apr_isdigit(r->status_line[1])
>>              && apr_isdigit(r->status_line[2])
>>              && apr_isspace(r->status_line[3])
> 
> Do we really need to require the space in the case that the reason 
> phrase is empty?

It seems the SPEC syntax for the status line requires the space. Since 
one can't be sure, what expressions the clients use to find the end of 
the status code, it would be safe to allow status lines with empty 
reason phrase and no space at the end, but to add the space at the end 
in this case.

>> -            && apr_isalnum(r->status_line[4])) {
>> +            && (r->status_line[4] == '\0' || 
>> apr_isalnum(r->status_line[4]))) {

I wonder what the reason was, to check only the first character of the 
reason phrase? I would have expected either a complete check, if it 
completely consists of "<TEXT, excluding CR, LF>", or no check at all.

>>              title = r->status_line;
>>          }

Regards,

Rainer


Mime
View raw message