You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Jacob Champion <ch...@gmail.com> on 2016/08/03 20:21:19 UTC
Re: svn commit: r1755098 - in /httpd/httpd/trunk:
docs/log-message-tags/next-number server/protocol.c
On 08/03/2016 09:46 AM, wrowe@apache.org wrote:
> Modified: httpd/httpd/trunk/server/protocol.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/protocol.c?rev=1755098&r1=1755097&r2=1755098&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/protocol.c (original)
> +++ httpd/httpd/trunk/server/protocol.c Wed Aug 3 16:46:20 2016
> @@ -835,8 +835,15 @@ AP_DECLARE(void) ap_get_mime_headers_cor
> return;
> }
>
> - if (last_field != NULL) {
> - if ((len > 0) && ((*field == '\t') || *field == ' ')) {
> + if ((len > 0) && ((*field == '\t') || *field == ' ')) {
> + if (last_field == NULL) {
> + r->status = HTTP_BAD_REQUEST;
> + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(03442)
> + "Line folding encounterd before first"
> + " header line");
> + return;
> + }
> +
I don't think this is an equivalent transformation. More logic below
this case relies on the last_field NULL check, and I'm currently getting
segfaults on trunk due to the strchr on line 907.
The addition of the `== NULL` check also triggers a C90 compiler warning
for the combo declaration/assignment of fold_len.
--Jacob
Re: svn commit: r1755098 - in /httpd/httpd/trunk: docs/log-message-tags/next-number
server/protocol.c
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Wed, Aug 3, 2016 at 4:58 PM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:
> On Wed, Aug 3, 2016 at 3:21 PM, Jacob Champion <ch...@gmail.com>
>> wrote:
>>
>>>
>>> I don't think this is an equivalent transformation. More logic below
>>> this case relies on the last_field NULL check, and I'm currently getting
>>> segfaults on trunk due to the strchr on line 907.
>>>
>>
>> You were correct, that was my oversight. It was easier to simply revert
> the
> work so that I'm not overlaying one whitespace change after another, since
> the fix causes everything to be re-indented anyways, so we are back to
> trunk of last evening before my patches this morning.
>
And I've reapplied the now-corrected logic. Thanks for your detailed and
careful review of these changes, please feel free to review this code again
offer any feedback.
We'll fix the obs-fold whitespace question next.
Cheers,
Bill
Re: svn commit: r1755098 - in /httpd/httpd/trunk: docs/log-message-tags/next-number
server/protocol.c
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Wed, Aug 3, 2016 at 4:29 PM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:
> On Wed, Aug 3, 2016 at 3:21 PM, Jacob Champion <ch...@gmail.com>
> wrote:
>
>> On 08/03/2016 09:46 AM, wrowe@apache.org wrote:
>>
>>> Modified: httpd/httpd/trunk/server/protocol.c
>>> URL:
>>> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/protocol.c?rev=1755098&r1=1755097&r2=1755098&view=diff
>>>
>>> ==============================================================================
>>> --- httpd/httpd/trunk/server/protocol.c (original)
>>> +++ httpd/httpd/trunk/server/protocol.c Wed Aug 3 16:46:20 2016
>>> @@ -835,8 +835,15 @@ AP_DECLARE(void) ap_get_mime_headers_cor
>>> return;
>>> }
>>>
>>> - if (last_field != NULL) {
>>> - if ((len > 0) && ((*field == '\t') || *field == ' ')) {
>>> + if ((len > 0) && ((*field == '\t') || *field == ' ')) {
>>> + if (last_field == NULL) {
>>> + r->status = HTTP_BAD_REQUEST;
>>> + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
>>> APLOGNO(03442)
>>> + "Line folding encounterd before first"
>>> + " header line");
>>> + return;
>>> + }
>>> +
>>>
>>
>> I don't think this is an equivalent transformation. More logic below this
>> case relies on the last_field NULL check, and I'm currently getting
>> segfaults on trunk due to the strchr on line 907.
>>
>> The addition of the `== NULL` check also triggers a C90 compiler warning
>> for the combo declaration/assignment of fold_len.
>
>
> Thanks for the heads-up. Investigating.
>
You were correct, that was my oversight. It was easier to simply revert the
work so that I'm not overlaying one whitespace change after another, since
the fix causes everything to be re-indented anyways, so we are back to
trunk of last evening before my patches this morning.
Again, thank you for the feedback!
Cheers,
Bill
Re: svn commit: r1755098 - in /httpd/httpd/trunk: docs/log-message-tags/next-number
server/protocol.c
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Wed, Aug 3, 2016 at 3:21 PM, Jacob Champion <ch...@gmail.com> wrote:
> On 08/03/2016 09:46 AM, wrowe@apache.org wrote:
>
>> Modified: httpd/httpd/trunk/server/protocol.c
>> URL:
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/protocol.c?rev=1755098&r1=1755097&r2=1755098&view=diff
>>
>> ==============================================================================
>> --- httpd/httpd/trunk/server/protocol.c (original)
>> +++ httpd/httpd/trunk/server/protocol.c Wed Aug 3 16:46:20 2016
>> @@ -835,8 +835,15 @@ AP_DECLARE(void) ap_get_mime_headers_cor
>> return;
>> }
>>
>> - if (last_field != NULL) {
>> - if ((len > 0) && ((*field == '\t') || *field == ' ')) {
>> + if ((len > 0) && ((*field == '\t') || *field == ' ')) {
>> + if (last_field == NULL) {
>> + r->status = HTTP_BAD_REQUEST;
>> + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
>> APLOGNO(03442)
>> + "Line folding encounterd before first"
>> + " header line");
>> + return;
>> + }
>> +
>>
>
> I don't think this is an equivalent transformation. More logic below this
> case relies on the last_field NULL check, and I'm currently getting
> segfaults on trunk due to the strchr on line 907.
>
> The addition of the `== NULL` check also triggers a C90 compiler warning
> for the combo declaration/assignment of fold_len.
Thanks for the heads-up. Investigating.