You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by wr...@apache.org on 2016/08/03 22:58:11 UTC
svn commit: r1755124 - /httpd/httpd/trunk/server/protocol.c
Author: wrowe
Date: Wed Aug 3 22:58:10 2016
New Revision: 1755124
URL: http://svn.apache.org/viewvc?rev=1755124&view=rev
Log:
Reformat for indentation following r1755123, Whitespace Only
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=1755124&r1=1755123&r2=1755124&view=diff
==============================================================================
--- httpd/httpd/trunk/server/protocol.c (original)
+++ httpd/httpd/trunk/server/protocol.c Wed Aug 3 22:58:10 2016
@@ -847,181 +847,180 @@ AP_DECLARE(void) ap_get_mime_headers_cor
return;
}
- /* This line is a continuation of the preceding line(s),
- * so append it to the line that we've set aside.
- * Note: this uses a power-of-two allocator to avoid
- * doing O(n) allocs and using O(n^2) space for
- * continuations that span many many lines.
+ /* This line is a continuation of the preceding line(s),
+ * so append it to the line that we've set aside.
+ * Note: this uses a power-of-two allocator to avoid
+ * doing O(n) allocs and using O(n^2) space for
+ * continuations that span many many lines.
+ */
+ fold_len = last_len + len + 1; /* trailing null */
+
+ if (fold_len >= (apr_size_t)(r->server->limit_req_fieldsize)) {
+ const char *field_escaped;
+
+ r->status = HTTP_BAD_REQUEST;
+ /* report what we have accumulated so far before the
+ * overflow (last_field) as the field with the problem
*/
- fold_len = last_len + len + 1; /* trailing null */
-
- if (fold_len >= (apr_size_t)(r->server->limit_req_fieldsize)) {
- const char *field_escaped;
-
- r->status = HTTP_BAD_REQUEST;
- /* report what we have accumulated so far before the
- * overflow (last_field) as the field with the problem
- */
- field_escaped = ap_escape_html(r->pool, last_field);
- apr_table_setn(r->notes, "error-notes",
- apr_psprintf(r->pool,
- "Size of a request header field "
- "after folding "
- "exceeds server limit.<br />\n"
- "<pre>\n%.*s\n</pre>\n",
- field_name_len(field_escaped),
- field_escaped));
- ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00562)
- "Request header exceeds LimitRequestFieldSize "
- "after folding: %.*s",
- field_name_len(last_field), last_field);
- return;
- }
+ field_escaped = ap_escape_html(r->pool, last_field);
+ apr_table_setn(r->notes, "error-notes",
+ apr_psprintf(r->pool,
+ "Size of a request header field "
+ "after folding "
+ "exceeds server limit.<br />\n"
+ "<pre>\n%.*s\n</pre>\n",
+ field_name_len(field_escaped),
+ field_escaped));
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00562)
+ "Request header exceeds LimitRequestFieldSize "
+ "after folding: %.*s",
+ field_name_len(last_field), last_field);
+ return;
+ }
+ if (fold_len > alloc_len) {
+ char *fold_buf;
+ alloc_len += alloc_len;
if (fold_len > alloc_len) {
- char *fold_buf;
- alloc_len += alloc_len;
- if (fold_len > alloc_len) {
- alloc_len = fold_len;
- }
- fold_buf = (char *)apr_palloc(r->pool, alloc_len);
- memcpy(fold_buf, last_field, last_len);
- last_field = fold_buf;
- }
- memcpy(last_field + last_len, field, len +1); /* +1 for nul */
- /* Replace obs-fold w/ SP per RFC 7230 3.2.4 */
- if (conf->http_conformance & AP_HTTP_CONFORMANCE_STRICT) {
- last_field[last_len] = ' ';
- }
- last_len += len;
- folded = 1;
- continue;
+ alloc_len = fold_len;
+ }
+ fold_buf = (char *)apr_palloc(r->pool, alloc_len);
+ memcpy(fold_buf, last_field, last_len);
+ last_field = fold_buf;
+ }
+ memcpy(last_field + last_len, field, len +1); /* +1 for nul */
+ /* Replace obs-fold w/ SP per RFC 7230 3.2.4 */
+ if (conf->http_conformance & AP_HTTP_CONFORMANCE_STRICT) {
+ last_field[last_len] = ' ';
+ }
+ last_len += len;
+ folded = 1;
+ continue;
}
else if (last_field != NULL) {
- /* not a continuation line */
+ /* not a continuation line */
- if (r->server->limit_req_fields
+ if (r->server->limit_req_fields
&& (++fields_read > r->server->limit_req_fields)) {
- r->status = HTTP_BAD_REQUEST;
+ r->status = HTTP_BAD_REQUEST;
+ apr_table_setn(r->notes, "error-notes",
+ "The number of request header fields "
+ "exceeds this server's limit.");
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00563)
+ "Number of request headers exceeds "
+ "LimitRequestFields");
+ return;
+ }
+
+ if (!(conf->http_conformance & AP_HTTP_CONFORMANCE_STRICT))
+ {
+ /* Not Strict, using the legacy parser */
+
+ if (!(value = strchr(last_field, ':'))) { /* Find ':' or */
+ r->status = HTTP_BAD_REQUEST; /* abort bad request */
apr_table_setn(r->notes, "error-notes",
- "The number of request header fields "
- "exceeds this server's limit.");
- ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00563)
- "Number of request headers exceeds "
- "LimitRequestFields");
+ apr_psprintf(r->pool,
+ "Request header field is "
+ "missing ':' separator.<br />\n"
+ "<pre>\n%.*s</pre>\n",
+ (int)LOG_NAME_MAX_LEN,
+ ap_escape_html(r->pool, last_field)));
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00564)
+ "Request header field is missing ':' "
+ "separator: %.*s", (int)LOG_NAME_MAX_LEN,
+ last_field);
return;
}
- if (!(conf->http_conformance & AP_HTTP_CONFORMANCE_STRICT))
- {
- /* Not Strict, using the legacy parser */
-
- if (!(value = strchr(last_field, ':'))) { /* Find ':' or */
- r->status = HTTP_BAD_REQUEST; /* abort bad request */
- apr_table_setn(r->notes, "error-notes",
- apr_psprintf(r->pool,
- "Request header field is "
- "missing ':' separator.<br />\n"
- "<pre>\n%.*s</pre>\n",
- (int)LOG_NAME_MAX_LEN,
- ap_escape_html(r->pool,
- last_field)));
- ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00564)
- "Request header field is missing ':' "
- "separator: %.*s", (int)LOG_NAME_MAX_LEN,
- last_field);
- return;
- }
-
- tmp_field = value - 1; /* last character of field-name */
+ tmp_field = value - 1; /* last character of field-name */
- *value++ = '\0'; /* NUL-terminate at colon */
+ *value++ = '\0'; /* NUL-terminate at colon */
- while (*value == ' ' || *value == '\t') {
- ++value; /* Skip to start of value */
- }
+ while (*value == ' ' || *value == '\t') {
+ ++value; /* Skip to start of value */
+ }
- /* Strip LWS after field-name: */
- while (tmp_field > last_field
+ /* Strip LWS after field-name: */
+ while (tmp_field > last_field
&& (*tmp_field == ' ' || *tmp_field == '\t')) {
- *tmp_field-- = '\0';
- }
+ *tmp_field-- = '\0';
+ }
- /* Strip LWS after field-value: */
- tmp_field = last_field + last_len - 1;
- while (tmp_field > value
+ /* Strip LWS after field-value: */
+ tmp_field = last_field + last_len - 1;
+ while (tmp_field > value
&& (*tmp_field == ' ' || *tmp_field == '\t')) {
- *tmp_field-- = '\0';
- }
+ *tmp_field-- = '\0';
+ }
+ }
+ else /* Using strict RFC7230 parsing */
+ {
+ /* Ensure valid token chars before ':' per RFC 7230 3.2.4 */
+ value = (char *)ap_scan_http_token(last_field);
+ if ((value == last_field) || *value != ':') {
+ r->status = HTTP_BAD_REQUEST;
+ apr_table_setn(r->notes, "error-notes",
+ apr_psprintf(r->pool,
+ "Request header field name "
+ "is malformed.<br />\n"
+ "<pre>\n%.*s</pre>\n",
+ (int)LOG_NAME_MAX_LEN,
+ ap_escape_html(r->pool, last_field)));
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(02426)
+ "Request header field name is malformed: "
+ "%.*s", (int)LOG_NAME_MAX_LEN, last_field);
+ return;
}
- else /* Using strict RFC7230 parsing */
- {
- /* Ensure valid token chars before ':' per RFC 7230 3.2.4 */
- value = (char *)ap_scan_http_token(last_field);
- if ((value == last_field) || *value != ':') {
- r->status = HTTP_BAD_REQUEST;
- apr_table_setn(r->notes, "error-notes",
- apr_psprintf(r->pool,
- "Request header field name "
- "is malformed.<br />\n"
- "<pre>\n%.*s</pre>\n",
- (int)LOG_NAME_MAX_LEN,
- ap_escape_html(r->pool, last_field)));
- ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(02426)
- "Request header field name is malformed: "
- "%.*s", (int)LOG_NAME_MAX_LEN, last_field);
- return;
- }
- *value++ = '\0'; /* NUL-terminate last_field name at ':' */
+ *value++ = '\0'; /* NUL-terminate last_field name at ':' */
- while (*value == ' ' || *value == '\t') {
- ++value; /* Skip LWS of value */
- }
+ while (*value == ' ' || *value == '\t') {
+ ++value; /* Skip LWS of value */
+ }
- /* Find invalid, non-HT ctrl char, or the trailing NULL */
- tmp_field = (char *)ap_scan_http_field_content(value);
-
- /* Strip LWS after field-value, if string not empty */
- if (*value && (*tmp_field == '\0')) {
- tmp_field--;
- while (*tmp_field == ' ' || *tmp_field == '\t') {
- *tmp_field-- = '\0';
- }
- ++tmp_field;
- }
+ /* Find invalid, non-HT ctrl char, or the trailing NULL */
+ tmp_field = (char *)ap_scan_http_field_content(value);
- /* Reject value for all garbage input (CTRLs excluding HT)
- * e.g. only VCHAR / SP / HT / obs-text are allowed per
- * RFC7230 3.2.6 - leave all more explicit rule enforcement
- * for specific header handler logic later in the cycle
- */
- if (*tmp_field != '\0') {
- r->status = HTTP_BAD_REQUEST;
- apr_table_setn(r->notes, "error-notes",
- apr_psprintf(r->pool,
- "Request header value "
- "is malformed.<br />\n"
- "<pre>\n%.*s</pre>\n",
- (int)LOG_NAME_MAX_LEN,
- ap_escape_html(r->pool, value)));
- ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(02427)
- "Request header value is malformed: "
- "%.*s", (int)LOG_NAME_MAX_LEN, value);
- return;
+ /* Strip LWS after field-value, if string not empty */
+ if (*value && (*tmp_field == '\0')) {
+ tmp_field--;
+ while (*tmp_field == ' ' || *tmp_field == '\t') {
+ *tmp_field-- = '\0';
}
+ ++tmp_field;
}
- apr_table_addn(r->headers_in, last_field, value);
-
- /* reset the alloc_len so that we'll allocate a new
- * buffer if we have to do any more folding: we can't
- * use the previous buffer because its contents are
- * now part of r->headers_in
+ /* Reject value for all garbage input (CTRLs excluding HT)
+ * e.g. only VCHAR / SP / HT / obs-text are allowed per
+ * RFC7230 3.2.6 - leave all more explicit rule enforcement
+ * for specific header handler logic later in the cycle
*/
- alloc_len = 0;
- /* end of logic where current line was not a continuation line */
+ if (*tmp_field != '\0') {
+ r->status = HTTP_BAD_REQUEST;
+ apr_table_setn(r->notes, "error-notes",
+ apr_psprintf(r->pool,
+ "Request header value "
+ "is malformed.<br />\n"
+ "<pre>\n%.*s</pre>\n",
+ (int)LOG_NAME_MAX_LEN,
+ ap_escape_html(r->pool, value)));
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(02427)
+ "Request header value is malformed: "
+ "%.*s", (int)LOG_NAME_MAX_LEN, value);
+ return;
+ }
+ }
+
+ apr_table_addn(r->headers_in, last_field, value);
+
+ /* reset the alloc_len so that we'll allocate a new
+ * buffer if we have to do any more folding: we can't
+ * use the previous buffer because its contents are
+ * now part of r->headers_in
+ */
+ alloc_len = 0;
+ /* end of logic where current line was not a continuation line */
}
/* Found a blank line, stop. */
Re: svn commit: r1755124 - /httpd/httpd/trunk/server/protocol.c
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Fri, Aug 5, 2016 at 12:50 PM, Yann Ylavic <yl...@gmail.com> wrote:
> > while adding
> > the unnecessary verification of last_len != 0 from line 904, so I'd say
> it's
> > a
> > net loss of legibility in spite of gaining us 4 characters. Just my 2c.
>
> The 'len' verification is necessary because it can be zero when
> last_field is NULL (legitimately, no headers at all), and we must
> leave still.
>
I thought that was a bug, but you caught it in the no-lines case by hitting
the trailing while() test. What I referred to was the continue from line
904
where we had merged an obs-fold, where can't have an empty last_len,
so the test from that continue is a wasted cycle.
Re: svn commit: r1755124 - /httpd/httpd/trunk/server/protocol.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Aug 5, 2016 at 7:33 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> On Fri, Aug 5, 2016 at 10:43 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>
>> @@ -903,8 +903,16 @@ AP_DECLARE(void) ap_get_mime_headers_core(request_
>> */
>> continue;
>> }
>> - else if (last_field != NULL) {
>>
>> + if (last_field == NULL) {
>> + /* Keep track of this first header line so that we can extend
>> it
>> + * across any obs-fold or parse it on the next loop
>> iteration.
>> + */
>> + last_field = field;
>> + last_len = len;
>> + continue;
>> + }
>> +
>> /* Process the previous last_field header line with all
>> obs-folded
>> * segments already concatinated (this is not operating on
>> the
>> * most recently read input line).
>
>
> This patch makes it less clear that the continue; case above also avoided
> the empty-line case (that was clearer in the main loop imo),
Personnaly an if-continued followed by an else make me think that
there is (at least) one top if-not-continued in the series (often hard
to follow construction), but this isn't the case here...
Anyway, I don't like too much my patch finally because it duplicates
logic code (few but still).
> while adding
> the unnecessary verification of last_len != 0 from line 904, so I'd say it's
> a
> net loss of legibility in spite of gaining us 4 characters. Just my 2c.
The 'len' verification is necessary because it can be zero when
last_field is NULL (legitimately, no headers at all), and we must
leave still.
Re: svn commit: r1755124 - /httpd/httpd/trunk/server/protocol.c
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Fri, Aug 5, 2016 at 10:43 AM, Yann Ylavic <yl...@gmail.com> wrote:
> @@ -903,8 +903,16 @@ AP_DECLARE(void) ap_get_mime_headers_core(request_
> */
> continue;
> }
> - else if (last_field != NULL) {
>
> + if (last_field == NULL) {
> + /* Keep track of this first header line so that we can extend
> it
> + * across any obs-fold or parse it on the next loop iteration.
> + */
> + last_field = field;
> + last_len = len;
> + continue;
> + }
> +
> /* Process the previous last_field header line with all
> obs-folded
> * segments already concatinated (this is not operating on the
> * most recently read input line).
>
This patch makes it less clear that the continue; case above also avoided
the empty-line case (that was clearer in the main loop imo), while adding
the unnecessary verification of last_len != 0 from line 904, so I'd say
it's a
net loss of legibility in spite of gaining us 4 characters. Just my 2c.
Re: svn commit: r1755124 - /httpd/httpd/trunk/server/protocol.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, Aug 4, 2016 at 12:58 AM, <wr...@apache.org> wrote:
> Author: wrowe
> Date: Wed Aug 3 22:58:10 2016
> New Revision: 1755124
>
> URL: http://svn.apache.org/viewvc?rev=1755124&view=rev
> Log:
> Reformat for indentation following r1755123, Whitespace Only
>
> Modified:
> httpd/httpd/trunk/server/protocol.c
We could save yet another level of indentation with:
Index: server/protocol.c
===================================================================
--- server/protocol.c (revision 1755347)
+++ server/protocol.c (working copy)
@@ -788,7 +788,7 @@ AP_DECLARE(void) ap_get_mime_headers_core(request_
* Read header lines until we get the empty separator line, a read error,
* the connection closes (EOF), reach the server limit, or we timeout.
*/
- while(1) {
+ do {
apr_status_t rv;
field = NULL;
@@ -903,8 +903,16 @@ AP_DECLARE(void) ap_get_mime_headers_core(request_
*/
continue;
}
- else if (last_field != NULL) {
+ if (last_field == NULL) {
+ /* Keep track of this first header line so that we can extend it
+ * across any obs-fold or parse it on the next loop iteration.
+ */
+ last_field = field;
+ last_len = len;
+ continue;
+ }
+
/* Process the previous last_field header line with all obs-folded
* segments already concatinated (this is not operating on the
* most recently read input line).
@@ -986,25 +994,14 @@ AP_DECLARE(void) ap_get_mime_headers_core(request_
apr_table_addn(r->headers_in, last_field, value);
- /* This last_field header is now stored in headers_in,
- * resume processing of the current input line.
- */
- }
-
- /* Found the terminating empty end-of-headers line, stop. */
- if (len == 0) {
- break;
- }
-
- /* Keep track of this new header line so that we can extend it across
- * any obs-fold or parse it on the next loop iteration. We referenced
- * our previously allocated buffer in r->headers_in,
- * so allocate a fresh buffer if required.
+ /* This last_field header is now stored in headers_in,
+ * resume processing of the current input line, so
+ * allocate a fresh buffer if required.
*/
alloc_len = 0;
last_field = field;
last_len = len;
- }
+ } while (last_len);
/* Combine multiple message-header fields with the same
* field-name, following RFC 2616, 4.2.
_
and then reindent the block left shifted above (in a second whitespace
only commit).
Not sure this old shared/participatory code deserves so much mangling,
that's no functional change, just readability improvement (maybe)...
WDYT?