You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Jeff Trawick <tr...@attglobal.net> on 2003/03/27 22:21:11 UTC
Re: cvs commit: httpd-2.0/server protocol.c
gregames@apache.org wrote:
> gregames 2003/03/27 12:34:56
>
> Modified: server protocol.c
> Log:
> ap_rgetline_core: set the number of bytes read & copied into the caller's
> buffer when returning APR_ENOSPC. This prevents seg faults in
> ap_get_mime_headers_core in an error path which handles headers that are too
> long.
>
> Submitted by: Jeff Trawick
unclear; see below :)
> @@ -390,6 +391,7 @@
> last_char = *s + bytes_handled - 1;
> }
> else {
> + *read = n;
I thought this should be
*read = bytes_handled;
since bytes_handled tells how many bytes were successfully copied to the
caller's buffer, whereas n just tells how big the callers buffer is
(or am I confused???)
> return APR_ENOSPC;
> }
> }
> @@ -519,6 +521,7 @@
> return APR_SUCCESS;
> }
> else {
> + *read = n;
same comment as above
> return APR_ENOSPC;
> }
> }
Re: cvs commit: httpd-2.0/server protocol.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 04:08 PM 3/27/2003, you wrote:
>Greg Ames wrote:
>>Jeff Trawick wrote:
>>
>>>gregames@apache.org wrote:
>>>
>>>>gregames 2003/03/27 12:34:56
>>>>
>>>> Modified: server protocol.c
>>>> Log:
>>>> ap_rgetline_core: set the number of bytes read & copied into the caller's
>>>> buffer when returning APR_ENOSPC. This prevents seg faults in
>>>> ap_get_mime_headers_core in an error path which handles headers that are too
>>>> long.
>>>> Submitted by: Jeff Trawick
>>>
>>>
>>>unclear; see below :)
>>>
>>>> @@ -390,6 +391,7 @@
>>>> last_char = *s + bytes_handled - 1;
>>>> }
>>>> else {
>>>> + *read = n;
>>>
>>>
>>>
>>>I thought this should be
>>>
>>> *read = bytes_handled;
>>>
>>>since bytes_handled tells how many bytes were successfully copied to the caller's buffer, whereas n just tells how big the callers buffer is
>>>(or am I confused???)
>>
>>Back up to the "if" statements that correspond to these two else's. bytes_handled is bigger than the size of the caller's buffer in these two cases, unless I missed something. It seems safer to go with the smaller of the two.
>
>After reading it again it looks like bytes_handled and n are always exactly the same in these two error paths.
>
>I had coded it as "*read = bytes_handled" because bytes_handled is the number of bytes already coded and available for the caller to look at. But "*read = n" is just as correct from a different perspective.
>
>I remove my concern and will +1 the patch.
And +1 here too.
Bill
Re: cvs commit: httpd-2.0/server protocol.c
Posted by Jeff Trawick <tr...@attglobal.net>.
Greg Ames wrote:
> Jeff Trawick wrote:
>
>> gregames@apache.org wrote:
>>
>>> gregames 2003/03/27 12:34:56
>>>
>>> Modified: server protocol.c
>>> Log:
>>> ap_rgetline_core: set the number of bytes read & copied into the
>>> caller's
>>> buffer when returning APR_ENOSPC. This prevents seg faults in
>>> ap_get_mime_headers_core in an error path which handles headers
>>> that are too
>>> long.
>>> Submitted by: Jeff Trawick
>>
>>
>> unclear; see below :)
>>
>>> @@ -390,6 +391,7 @@
>>> last_char = *s + bytes_handled - 1;
>>> }
>>> else {
>>> + *read = n;
>>
>>
>>
>> I thought this should be
>>
>> *read = bytes_handled;
>>
>> since bytes_handled tells how many bytes were successfully copied to
>> the caller's buffer, whereas n just tells how big the callers buffer is
>> (or am I confused???)
>
>
> Back up to the "if" statements that correspond to these two else's.
> bytes_handled is bigger than the size of the caller's buffer in these
> two cases, unless I missed something. It seems safer to go with the
> smaller of the two.
After reading it again it looks like bytes_handled and n are always
exactly the same in these two error paths.
I had coded it as "*read = bytes_handled" because bytes_handled is the
number of bytes already coded and available for the caller to look at.
But "*read = n" is just as correct from a different perspective.
I remove my concern and will +1 the patch.
Re: cvs commit: httpd-2.0/server protocol.c
Posted by Greg Ames <gr...@apache.org>.
Jeff Trawick wrote:
> gregames@apache.org wrote:
>
>> gregames 2003/03/27 12:34:56
>>
>> Modified: server protocol.c
>> Log:
>> ap_rgetline_core: set the number of bytes read & copied into the
>> caller's
>> buffer when returning APR_ENOSPC. This prevents seg faults in
>> ap_get_mime_headers_core in an error path which handles headers that
>> are too
>> long.
>> Submitted by: Jeff Trawick
>
> unclear; see below :)
>
>> @@ -390,6 +391,7 @@
>> last_char = *s + bytes_handled - 1;
>> }
>> else {
>> + *read = n;
>
>
> I thought this should be
>
> *read = bytes_handled;
>
> since bytes_handled tells how many bytes were successfully copied to the
> caller's buffer, whereas n just tells how big the callers buffer is
> (or am I confused???)
Back up to the "if" statements that correspond to these two else's.
bytes_handled is bigger than the size of the caller's buffer in these two cases,
unless I missed something. It seems safer to go with the smaller of the two.
This is a garbage header anyway. The only getline caller we know of that does
anything in this error situation tries to be a nice guy and return some of the
garbage header back to the client in the request body of the error message. I
wouldn't mine nuking that code, but I assume some Smart Person thought about it
before commiting it way back when.
Greg