diff --git a/include/httpd.h b/include/httpd.h index 5db96fb4d17..38f796a6e7d 100644 --- a/include/httpd.h +++ b/include/httpd.h @@ -2780,6 +2780,12 @@ AP_DECLARE(int) ap_array_str_index(const apr_array_header_t *array, AP_DECLARE(int) ap_array_str_contains(const apr_array_header_t *array, const char *s); +/** + * Strip trailing whitespace from a NUL-terminated string in place. + * @param str String to modify + */ +AP_DECLARE(void) ap_cstr_stripws(char *str); + /** * Perform a case-insensitive comparison of two strings @a str1 and @a str2, * treating upper and lower case values of the 26 standard C/POSIX alphabetic diff --git a/modules/http/byterange_filter.c b/modules/http/byterange_filter.c index e1c69b0ba71..bd3a089f0e6 100644 --- a/modules/http/byterange_filter.c +++ b/modules/http/byterange_filter.c @@ -136,16 +136,14 @@ static int ap_set_byterange(request_rec *r, apr_off_t clength, } *indexes = apr_array_make(r->pool, ranges, sizeof(indexes_t)); while ((cur = ap_getword(r->pool, &range, ','))) { - char *dash, *end_cur; + char *dash; apr_off_t number, start, end; /* Remove leading and trailing white spaces */ while (apr_isspace(*cur)) ++cur; /* blast trailing whitespace */ - end_cur = &cur[strlen(cur)]; - while (--end_cur >= cur && apr_isspace(*end_cur)) - *end_cur = '\0'; + ap_cstr_stripws(cur); if (!*cur) break; diff --git a/modules/mappers/mod_negotiation.c b/modules/mappers/mod_negotiation.c index a528f814397..af96b263867 100644 --- a/modules/mappers/mod_negotiation.c +++ b/modules/mappers/mod_negotiation.c @@ -1022,10 +1022,8 @@ static int read_type_map(apr_file_t **map, negotiation_state *neg, } else if (!strncmp(buffer, "body:", 5)) { char *tag = apr_pstrdup(neg->pool, body); - char *eol = strchr(tag, '\0'); apr_size_t len = MAX_STRING_LEN; - while (--eol >= tag && apr_isspace(*eol)) - *eol = '\0'; + ap_cstr_stripws(tag); if ((mime_info.body = get_body(buffer, &len, tag, *map)) < 0) { ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(00685) "Syntax error in type map, no end tag '%s' " diff --git a/modules/metadata/mod_cern_meta.c b/modules/metadata/mod_cern_meta.c index a150b3c9fa1..ec843a66412 100644 --- a/modules/metadata/mod_cern_meta.c +++ b/modules/metadata/mod_cern_meta.c @@ -244,9 +244,7 @@ static int scan_meta_file(request_rec *r, apr_file_t *f) char *tmp; /* Nuke trailing whitespace */ - char *endp = l + strlen(l) - 1; - while (endp > l && apr_isspace(*endp)) - *endp-- = '\0'; + ap_cstr_stripws(l); tmp = apr_pstrdup(r->pool, l); ap_content_type_tolower(tmp); diff --git a/modules/proxy/mod_proxy_hcheck.c b/modules/proxy/mod_proxy_hcheck.c index 38c5c2b5609..2fe1152fe1c 100644 --- a/modules/proxy/mod_proxy_hcheck.c +++ b/modules/proxy/mod_proxy_hcheck.c @@ -766,7 +766,7 @@ static int hc_read_headers(request_rec *r) /* OK, 1st line is OK... scarf in the headers */ while ((len = ap_getline(buffer, sizeof(buffer), r, 1)) > 0) { - char *value, *end; + char *value; ap_log_error(APLOG_MARK, APLOG_TRACE7, 0, r->server, "%.*s", len, buffer); if (!(value = strchr(buffer, ':'))) { @@ -776,8 +776,7 @@ static int hc_read_headers(request_rec *r) ++value; while (apr_isspace(*value)) ++value; /* Skip to start of value */ - for (end = &value[strlen(value)-1]; end > value && apr_isspace(*end); --end) - *end = '\0'; + ap_cstr_stripws(value); apr_table_add(r->headers_out, buffer, value); } diff --git a/modules/proxy/mod_proxy_http.c b/modules/proxy/mod_proxy_http.c index 970487be6ea..0b33f095630 100644 --- a/modules/proxy/mod_proxy_http.c +++ b/modules/proxy/mod_proxy_http.c @@ -864,7 +864,7 @@ static apr_status_t ap_proxy_read_headers(request_rec *r, request_rec *rr, conn_rec *c, int *pread_len) { int len; - char *value, *end; + char *value; int saw_headers = 0; void *sconf = r->server->module_config; proxy_server_conf *psc; @@ -973,8 +973,7 @@ static apr_status_t ap_proxy_read_headers(request_rec *r, request_rec *rr, ++value; /* Skip to start of value */ /* should strip trailing whitespace as well */ - for (end = &value[strlen(value)-1]; end > value && apr_isspace(*end); --end) - *end = '\0'; + ap_cstr_stripws(value); /* make sure we add so as not to destroy duplicated headers * Modify headers requiring canonicalisation and/or affected diff --git a/modules/proxy/mod_proxy_uwsgi.c b/modules/proxy/mod_proxy_uwsgi.c index 26091087263..6b602c247b9 100644 --- a/modules/proxy/mod_proxy_uwsgi.c +++ b/modules/proxy/mod_proxy_uwsgi.c @@ -303,7 +303,7 @@ static int uwsgi_response(request_rec *r, proxy_conn_rec * backend, char buffer[HUGE_STRING_LEN]; const char *buf; - char *value, *end; + char *value; char keepchar; int len; int backend_broke = 0; @@ -386,9 +386,7 @@ static int uwsgi_response(request_rec *r, proxy_conn_rec * backend, } while (apr_isspace(*value)) ++value; - for (end = &value[strlen(value) - 1]; - end > value && apr_isspace(*end); --end) - *end = '\0'; + ap_cstr_stripws(value); if (*ap_scan_http_field_content(value)) { /* invalid value */ len = -1; diff --git a/server/util.c b/server/util.c index d1d06fc15b4..eeddf63bc3d 100644 --- a/server/util.c +++ b/server/util.c @@ -1183,16 +1183,15 @@ static int cfg_trim_line(char *buf) while (apr_isspace(*start)) ++start; /* blast trailing whitespace */ - end = &start[strlen(start)]; - while (--end >= start && apr_isspace(*end)) - *end = '\0'; + ap_cstr_stripws(start); + end = start + strlen(start); /* Zap leading whitespace by shifting */ if (start != buf) - memmove(buf, start, end - start + 2); + memmove(buf, start, end - start + 1); #ifdef DEBUG_CFG_LINES ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, NULL, APLOGNO(00555) "Read config: '%s'", buf); #endif - return end - start + 1; + return end - start; } /* Read one line from open ap_configfile_t, strip LF, increase line number */ @@ -3704,6 +3703,16 @@ static const unsigned char ucharmap[256] = { }; #endif +AP_DECLARE(void) ap_cstr_stripws(char *str) +{ + char *end = str + strlen(str); + + while (end > str && apr_isspace(end[-1])) { + --end; + } + *end = '\0'; +} + AP_DECLARE(int) ap_cstr_casecmp(const char *s1, const char *s2) { const unsigned char *u1 = (const unsigned char *)s1; diff --git a/server/util_script.c b/server/util_script.c index 6a18aec8c90..d620f42ce82 100644 --- a/server/util_script.c +++ b/server/util_script.c @@ -649,10 +649,7 @@ AP_DECLARE(int) ap_scan_script_header_err_core_ex(request_rec *r, char *buffer, /* Nuke trailing whitespace */ - char *endp = l + strlen(l) - 1; - while (endp > l && apr_isspace(*endp)) { - *endp-- = '\0'; - } + ap_cstr_stripws(l); tmp = apr_pstrdup(r->pool, l); ap_content_type_tolower(tmp); diff --git a/test/modules/proxy/test_03_response.py b/test/modules/proxy/test_03_response.py index 60a4afa8af9..5c22528debd 100644 --- a/test/modules/proxy/test_03_response.py +++ b/test/modules/proxy/test_03_response.py @@ -16,6 +16,16 @@ def _make_response(self, data): return """HTTP/1.1 200 OK\r\nContent-Type: text/html\x00extra\r\n\r\n""".encode() + if "/empty-header" in path: + body = b"Hello" + return ( + b"HTTP/1.1 200 OK\r\n" + b"X-Empty:\r\n" + b"Content-Length: 5\r\n" + b"\r\n" + + body + ) + if "/forwarded" in path: headers = data.split(b"\r\n\r\n")[0].decode("latin-1") forwarded = [] @@ -89,6 +99,13 @@ def test_proxy_03_001(self, env): lognos=["AH01106", "AH10404"] ) + # empty backend response header values are valid + def test_proxy_03_004(self, env): + r = env.curl_get(env.mkurl("http", "test1", "/empty-header")) + assert r.response["status"] == 200 + assert "x-empty" in r.response["header"] + assert r.response["body"] == b"Hello" + # checks X-Forwarded headers def test_proxy_03_002(self, env): if not env.httpd_is_at_least("2.4.54"): diff --git a/test/modules/proxy/test_05_uwsgi.py b/test/modules/proxy/test_05_uwsgi.py index b0733aba7dd..0b360b89ff6 100644 --- a/test/modules/proxy/test_05_uwsgi.py +++ b/test/modules/proxy/test_05_uwsgi.py @@ -17,6 +17,17 @@ def hello(data): + body ) + @staticmethod + def empty_header(data): + body = b"Hello" + return ( + b"HTTP/1.1 200 OK\r\n" + b"X-Empty:\r\n" + b"Content-Length: 5\r\n" + b"\r\n" + + body + ) + class TestProxyUwsgi: @@ -51,3 +62,11 @@ def test_proxy_005_01(self, env, _class_scope): assert data[3] == 0x00 # standard WSGI request assert len(data) == 4 + datasize + # empty backend response header values are valid + def test_proxy_005_02(self, env, _class_scope): + _class_scope._make_response = _UWSGIFaker.empty_header + r = env.curl_get(env.mkurl("http", "test1", "/")) + assert r.response["status"] == 200 + assert "x-empty" in r.response["header"] + assert r.response["body"] == b"Hello" + diff --git a/test/unit/util.c b/test/unit/util.c index c9dec41a18e..ad0a59a375f 100644 --- a/test/unit/util.c +++ b/test/unit/util.c @@ -17,6 +17,7 @@ #include "../httpdunit.h" #include "httpd.h" +#include "apr_strings.h" /* * Test Fixture -- runs once per test @@ -80,6 +81,39 @@ HTTPD_START_LOOP_TEST(find_token_correctly_parses_token_list, ap_test_token_case END_TEST +/* + * ap_cstr_stripws() + */ + +struct ap_cstr_stripws_case { + const char *input; + const char *expected; +}; + +const struct ap_cstr_stripws_case ap_cstr_stripws_cases[] = { + { "", "" }, + { " ", "" }, + { "\t\r\n", "" }, + { "value", "value" }, + { "value ", "value" }, + { "value\t ", "value" }, + { " value ", " value" }, +}; + +const size_t ap_cstr_stripws_cases_len = sizeof(ap_cstr_stripws_cases) / + sizeof(ap_cstr_stripws_cases[0]); + +HTTPD_START_LOOP_TEST(cstr_stripws_strips_trailing_whitespace, ap_cstr_stripws_cases_len) +{ + const struct ap_cstr_stripws_case *c = &ap_cstr_stripws_cases[_i]; + char *str = apr_pstrdup(g_pool, c->input); + + ap_cstr_stripws(str); + ck_assert_str_eq(str, c->expected); +} +END_TEST + + /* * ap_escape_quotes() */