Re: svn commit: r1800111 - /httpd/httpd/trunk/server/protocol.c

[William A Rowe Jr]

On Wed, Jun 28, 2017 at 7:14 AM, Yann  wrote:
>
> Looks like the code after the patch below would be simpler and work too :

Agreed this is easier to follow, tmp_field is otherwise unused in the
unsafe code path. Proposed for backport, thanks.

Note this patch is the 2.2, non-APLOGNO flavor;

> Index: server/protocol.c
> ===
> --- server/protocol.c(revision 1800151)
> +++ server/protocol.c(working copy)
> @@ -1081,8 +1081,12 @@ AP_DECLARE(void) ap_get_mime_headers_core(request_
>  return;
>  }
>
> -/* last character of field-name */
> -tmp_field = value - (value > last_field ? 1 : 0);
> +if (value == last_field) {
> +r->status = HTTP_BAD_REQUEST;
> +ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
> +  "Request header field name was empty");
> +return;
> +}
>
>  *value++ = '\0'; /* NUL-terminate at colon */
>
> @@ -1105,13 +1109,6 @@ AP_DECLARE(void) ap_get_mime_headers_core(request_
>" bad whitespace");
>  return;
>  }
> -
> -if (tmp_field == last_field) {
> -r->status = HTTP_BAD_REQUEST;
> -ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
> -  "Request header field name was empty");
> -return;
> -}
>  }
>  else /* Using strict RFC7230 parsing */
>  {
> _

Re: svn commit: r1800111 - /httpd/httpd/trunk/server/protocol.c

[Yann]

On Wed, Jun 28, 2017 at 4:33 AM,   wrote:
> Author: wrowe
> Date: Wed Jun 28 02:33:29 2017
> New Revision: 1800111
>
> URL: http://svn.apache.org/viewvc?rev=1800111=rev
> Log:
> Appears to resolve the issue to permit single-char fieldnames; PR61220
>
> Modified:
> httpd/httpd/trunk/server/protocol.c
>
> Modified: httpd/httpd/trunk/server/protocol.c
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/protocol.c?rev=1800111=1800110=1800111=diff
> ==
> --- httpd/httpd/trunk/server/protocol.c (original)
> +++ httpd/httpd/trunk/server/protocol.c Wed Jun 28 02:33:29 2017
> @@ -,7 +,7 @@ AP_DECLARE(void) ap_get_mime_headers_cor
>  return;
>  }
>
> -if (tmp_field == last_field) {
> +if (tmp_field == last_field && !*last_field) {
>  r->status = HTTP_BAD_REQUEST;
>  ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, 
> APLOGNO(03453)
>"Request header field name was empty");
>

Looks like the code after the patch below would be simpler and work too :

Index: server/protocol.c
===
--- server/protocol.c(revision 1800151)
+++ server/protocol.c(working copy)
@@ -1081,8 +1081,12 @@ AP_DECLARE(void) ap_get_mime_headers_core(request_
 return;
 }

-/* last character of field-name */
-tmp_field = value - (value > last_field ? 1 : 0);
+if (value == last_field) {
+r->status = HTTP_BAD_REQUEST;
+ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
+  "Request header field name was empty");
+return;
+}

 *value++ = '\0'; /* NUL-terminate at colon */

@@ -1105,13 +1109,6 @@ AP_DECLARE(void) ap_get_mime_headers_core(request_
   " bad whitespace");
 return;
 }
-
-if (tmp_field == last_field) {
-r->status = HTTP_BAD_REQUEST;
-ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
-  "Request header field name was empty");
-return;
-}
 }
 else /* Using strict RFC7230 parsing */
 {
_