From 3d2f29c88f9b073cb0fd3b9c7f85430e2170acbb Mon Sep 17 00:00:00 2001 From: Masakazu Kitajo Date: Tue, 1 Apr 2025 12:15:16 -0600 Subject: [PATCH] Require the use of CRLF in chunked message body (#12150) * Require the use of CRLF in chunked message body * Fix docs --- doc/admin-guide/files/records.config.en.rst | 9 +++ .../functions/TSHttpOverridableConfig.en.rst | 1 + .../api/types/TSOverridableConfigKey.en.rst | 1 + include/ts/apidefs.h.in | 1 + mgmt/RecordsConfig.cc | 2 + plugins/lua/ts_lua_http_config.c | 2 + proxy/http/HttpConfig.cc | 2 + proxy/http/HttpConfig.h | 1 + proxy/http/HttpSM.cc | 28 +++++--- proxy/http/HttpTunnel.cc | 72 ++++++++++++------- proxy/http/HttpTunnel.h | 15 +++- src/shared/overridable_txn_vars.cc | 1 + src/traffic_server/FetchSM.cc | 3 +- src/traffic_server/InkAPI.cc | 3 + src/traffic_server/InkAPITest.cc | 3 +- .../malformed_chunked_header.replay.yaml | 44 ++++++++++++ 16 files changed, 150 insertions(+), 38 deletions(-) diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst index b81510db69d..7db9e6a9f66 100644 --- a/doc/admin-guide/files/records.config.en.rst +++ b/doc/admin-guide/files/records.config.en.rst @@ -987,6 +987,15 @@ mptcp for details about chunked trailers. By default, this option is disabled and therefore |TS| will not drop chunked trailers. +.. ts:cv:: CONFIG proxy.config.http.strict_chunk_parsing INT 1 + :reloadable: + :overridable: + + Specifies whether |TS| strictly checks errors in chunked message body. + If enabled (``1``), |TS| returns 400 Bad Request if chunked message body is + not compliant with RFC 9112. If disabled (``0``), |TS| allows using LF as + a line terminator. + .. ts:cv:: CONFIG proxy.config.http.send_http11_requests INT 1 :reloadable: :overridable: diff --git a/doc/developer-guide/api/functions/TSHttpOverridableConfig.en.rst b/doc/developer-guide/api/functions/TSHttpOverridableConfig.en.rst index 2ec29831532..b2b0e231502 100644 --- a/doc/developer-guide/api/functions/TSHttpOverridableConfig.en.rst +++ b/doc/developer-guide/api/functions/TSHttpOverridableConfig.en.rst @@ -111,6 +111,7 @@ TSOverridableConfigKey Value Config :c:enumerator:`TS_CONFIG_HTTP_CACHE_WHEN_TO_REVALIDATE` :ts:cv:`proxy.config.http.cache.when_to_revalidate` :c:enumerator:`TS_CONFIG_HTTP_CHUNKING_ENABLED` :ts:cv:`proxy.config.http.chunking_enabled` :c:enumerator:`TS_CONFIG_HTTP_CHUNKING_SIZE` :ts:cv:`proxy.config.http.chunking.size` +:c:enumerator:`TS_CONFIG_HTTP_STRICT_CHUNK_PARSING` :ts:cv:`proxy.config.http.strict_chunk_parsing` :c:enumerator:`TS_CONFIG_HTTP_CONNECT_ATTEMPTS_MAX_RETRIES_DEAD_SERVER` :ts:cv:`proxy.config.http.connect_attempts_max_retries_dead_server` :c:enumerator:`TS_CONFIG_HTTP_DROP_CHUNKED_TRAILERS` :ts:cv:`proxy.config.http.drop_chunked_trailers` :c:enumerator:`TS_CONFIG_HTTP_CONNECT_ATTEMPTS_MAX_RETRIES` :ts:cv:`proxy.config.http.connect_attempts_max_retries` diff --git a/doc/developer-guide/api/types/TSOverridableConfigKey.en.rst b/doc/developer-guide/api/types/TSOverridableConfigKey.en.rst index 2d0941efde0..b4291b46579 100644 --- a/doc/developer-guide/api/types/TSOverridableConfigKey.en.rst +++ b/doc/developer-guide/api/types/TSOverridableConfigKey.en.rst @@ -91,6 +91,7 @@ Enumeration Members .. c:enumerator:: TS_CONFIG_NET_SOCK_PACKET_TOS_OUT .. c:enumerator:: TS_CONFIG_HTTP_INSERT_AGE_IN_RESPONSE .. c:enumerator:: TS_CONFIG_HTTP_CHUNKING_SIZE +.. c:enumerator:: TS_CONFIG_HTTP_STRICT_CHUNK_PARSING .. c:enumerator:: TS_CONFIG_HTTP_DROP_CHUNKED_TRAILERS .. c:enumerator:: TS_CONFIG_HTTP_FLOW_CONTROL_ENABLED .. c:enumerator:: TS_CONFIG_HTTP_FLOW_CONTROL_LOW_WATER_MARK diff --git a/include/ts/apidefs.h.in b/include/ts/apidefs.h.in index 1641565a1a9..893177c88b9 100644 --- a/include/ts/apidefs.h.in +++ b/include/ts/apidefs.h.in @@ -875,6 +875,7 @@ typedef enum { TS_CONFIG_HTTP_ENABLE_PARENT_TIMEOUT_MARKDOWNS, TS_CONFIG_HTTP_DISABLE_PARENT_MARKDOWNS, TS_CONFIG_HTTP_DROP_CHUNKED_TRAILERS, + TS_CONFIG_HTTP_STRICT_CHUNK_PARSING, TS_CONFIG_LAST_ENTRY } TSOverridableConfigKey; diff --git a/mgmt/RecordsConfig.cc b/mgmt/RecordsConfig.cc index ff7fdc0e3c8..e645bb6c6f1 100644 --- a/mgmt/RecordsConfig.cc +++ b/mgmt/RecordsConfig.cc @@ -363,6 +363,8 @@ static const RecordElement RecordsConfig[] = , {RECT_CONFIG, "proxy.config.http.drop_chunked_trailers", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_NULL, "[0-1]", RECA_NULL} , + {RECT_CONFIG, "proxy.config.http.strict_chunk_parsing", RECD_INT, "1", RECU_DYNAMIC, RR_NULL, RECC_NULL, "[0-1]", RECA_NULL} + , {RECT_CONFIG, "proxy.config.http.flow_control.enabled", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_NULL, nullptr, RECA_NULL} , {RECT_CONFIG, "proxy.config.http.flow_control.high_water", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_NULL, nullptr, RECA_NULL} diff --git a/plugins/lua/ts_lua_http_config.c b/plugins/lua/ts_lua_http_config.c index a25d8ab8c8f..4b22ee94b50 100644 --- a/plugins/lua/ts_lua_http_config.c +++ b/plugins/lua/ts_lua_http_config.c @@ -149,6 +149,7 @@ typedef enum { TS_LUA_CONFIG_BODY_FACTORY_RESPONSE_SUPPRESSION_MODE = TS_CONFIG_BODY_FACTORY_RESPONSE_SUPPRESSION_MODE, TS_LUA_CONFIG_ENABLE_PARENT_TIMEOUT_MARKDOWNS = TS_CONFIG_HTTP_ENABLE_PARENT_TIMEOUT_MARKDOWNS, TS_LUA_CONFIG_DISABLE_PARENT_MARKDOWNS = TS_CONFIG_HTTP_DISABLE_PARENT_MARKDOWNS, + TS_LUA_CONFIG_HTTP_STRICT_CHUNK_PARSING = TS_CONFIG_HTTP_STRICT_CHUNK_PARSING, TS_LUA_CONFIG_LAST_ENTRY = TS_CONFIG_LAST_ENTRY, } TSLuaOverridableConfigKey; @@ -290,6 +291,7 @@ ts_lua_var_item ts_lua_http_config_vars[] = { TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_BODY_FACTORY_RESPONSE_SUPPRESSION_MODE), TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_ENABLE_PARENT_TIMEOUT_MARKDOWNS), TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_DISABLE_PARENT_MARKDOWNS), + TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_HTTP_STRICT_CHUNK_PARSING), TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_LAST_ENTRY), }; diff --git a/proxy/http/HttpConfig.cc b/proxy/http/HttpConfig.cc index d5c1c00a283..ca2edee1ee7 100644 --- a/proxy/http/HttpConfig.cc +++ b/proxy/http/HttpConfig.cc @@ -1190,6 +1190,7 @@ HttpConfig::startup() HttpEstablishStaticConfigByte(c.oride.chunking_enabled, "proxy.config.http.chunking_enabled"); HttpEstablishStaticConfigLongLong(c.oride.http_chunking_size, "proxy.config.http.chunking.size"); HttpEstablishStaticConfigByte(c.oride.http_drop_chunked_trailers, "proxy.config.http.drop_chunked_trailers"); + HttpEstablishStaticConfigByte(c.oride.http_strict_chunk_parsing, "proxy.config.http.strict_chunk_parsing"); HttpEstablishStaticConfigByte(c.oride.flow_control_enabled, "proxy.config.http.flow_control.enabled"); HttpEstablishStaticConfigLongLong(c.oride.flow_high_water_mark, "proxy.config.http.flow_control.high_water"); HttpEstablishStaticConfigLongLong(c.oride.flow_low_water_mark, "proxy.config.http.flow_control.low_water"); @@ -1496,6 +1497,7 @@ HttpConfig::reconfigure() params->oride.keep_alive_enabled_out = INT_TO_BOOL(m_master.oride.keep_alive_enabled_out); params->oride.chunking_enabled = INT_TO_BOOL(m_master.oride.chunking_enabled); params->oride.http_drop_chunked_trailers = m_master.oride.http_drop_chunked_trailers; + params->oride.http_strict_chunk_parsing = m_master.oride.http_strict_chunk_parsing; params->oride.auth_server_session_private = INT_TO_BOOL(m_master.oride.auth_server_session_private); params->oride.http_chunking_size = m_master.oride.http_chunking_size; diff --git a/proxy/http/HttpConfig.h b/proxy/http/HttpConfig.h index 6c1763f84e8..53450bdbb25 100644 --- a/proxy/http/HttpConfig.h +++ b/proxy/http/HttpConfig.h @@ -703,6 +703,7 @@ struct OverridableHttpConfigParams { MgmtInt http_chunking_size = 4096; // Maximum chunk size for chunked output. MgmtByte http_drop_chunked_trailers = 0; ///< Whether to drop chunked trailers. + MgmtByte http_strict_chunk_parsing = 1; ///< Whether to parse chunked body strictly. MgmtInt flow_high_water_mark = 0; ///< Flow control high water mark. MgmtInt flow_low_water_mark = 0; ///< Flow control low water mark. diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc index cdc05461320..c0ba82641e1 100644 --- a/proxy/http/HttpSM.cc +++ b/proxy/http/HttpSM.cc @@ -978,7 +978,8 @@ HttpSM::wait_for_full_body() p = tunnel.add_producer(ua_entry->vc, post_bytes, buf_start, &HttpSM::tunnel_handler_post_ua, HT_BUFFER_READ, "ua post buffer"); if (chunked) { bool const drop_chunked_trailers = t_state.http_config_param->oride.http_drop_chunked_trailers == 1; - tunnel.set_producer_chunking_action(p, 0, TCA_PASSTHRU_CHUNKED_CONTENT, drop_chunked_trailers); + bool const parse_chunk_strictly = t_state.http_config_param->oride.http_strict_chunk_parsing == 1; + tunnel.set_producer_chunking_action(p, 0, TCA_PASSTHRU_CHUNKED_CONTENT, drop_chunked_trailers, parse_chunk_strictly); } ua_entry->in_tunnel = true; ua_txn->set_inactivity_timeout(HRTIME_SECONDS(t_state.txn_conf->transaction_no_activity_timeout_in)); @@ -6197,10 +6198,11 @@ HttpSM::do_setup_post_tunnel(HttpVC_t to_vc_type) // In either case, the server will support chunked (HTTP/1.1) if (chunked) { bool const drop_chunked_trailers = t_state.http_config_param->oride.http_drop_chunked_trailers == 1; + bool const parse_chunk_strictly = t_state.http_config_param->oride.http_strict_chunk_parsing == 1; if (ua_txn->is_chunked_encoding_supported()) { - tunnel.set_producer_chunking_action(p, 0, TCA_PASSTHRU_CHUNKED_CONTENT, drop_chunked_trailers); + tunnel.set_producer_chunking_action(p, 0, TCA_PASSTHRU_CHUNKED_CONTENT, drop_chunked_trailers, parse_chunk_strictly); } else { - tunnel.set_producer_chunking_action(p, 0, TCA_CHUNK_CONTENT, drop_chunked_trailers); + tunnel.set_producer_chunking_action(p, 0, TCA_CHUNK_CONTENT, drop_chunked_trailers, parse_chunk_strictly); tunnel.set_producer_chunking_size(p, 0); } } @@ -6609,7 +6611,9 @@ HttpSM::setup_cache_read_transfer() // w/o providing a Content-Length header if (t_state.client_info.receive_chunked_response) { bool const drop_chunked_trailers = t_state.http_config_param->oride.http_drop_chunked_trailers == 1; - tunnel.set_producer_chunking_action(p, client_response_hdr_bytes, TCA_CHUNK_CONTENT, drop_chunked_trailers); + bool const parse_chunk_strictly = t_state.http_config_param->oride.http_strict_chunk_parsing == 1; + tunnel.set_producer_chunking_action(p, client_response_hdr_bytes, TCA_CHUNK_CONTENT, drop_chunked_trailers, + parse_chunk_strictly); tunnel.set_producer_chunking_size(p, t_state.txn_conf->http_chunking_size); } ua_entry->in_tunnel = true; @@ -6927,8 +6931,10 @@ HttpSM::setup_server_transfer_to_transform() transform_info.entry->in_tunnel = true; if (t_state.current.server->transfer_encoding == HttpTransact::CHUNKED_ENCODING) { - client_response_hdr_bytes = 0; // fixed by YTS Team, yamsat - tunnel.set_producer_chunking_action(p, client_response_hdr_bytes, TCA_DECHUNK_CONTENT, HttpTunnel::DROP_CHUNKED_TRAILERS); + client_response_hdr_bytes = 0; // fixed by YTS Team, yamsat + bool const parse_chunk_strictly = t_state.http_config_param->oride.http_strict_chunk_parsing == 1; + tunnel.set_producer_chunking_action(p, client_response_hdr_bytes, TCA_DECHUNK_CONTENT, HttpTunnel::DROP_CHUNKED_TRAILERS, + parse_chunk_strictly); } return p; @@ -6968,7 +6974,9 @@ HttpSM::setup_transfer_from_transform() if (t_state.client_info.receive_chunked_response) { bool const drop_chunked_trailers = t_state.http_config_param->oride.http_drop_chunked_trailers == 1; - tunnel.set_producer_chunking_action(p, client_response_hdr_bytes, TCA_CHUNK_CONTENT, drop_chunked_trailers); + bool const parse_chunk_strictly = t_state.http_config_param->oride.http_strict_chunk_parsing == 1; + tunnel.set_producer_chunking_action(p, client_response_hdr_bytes, TCA_CHUNK_CONTENT, drop_chunked_trailers, + parse_chunk_strictly); tunnel.set_producer_chunking_size(p, t_state.txn_conf->http_chunking_size); } @@ -7025,7 +7033,8 @@ HttpSM::setup_server_transfer_to_cache_only() tunnel.add_producer(server_entry->vc, nbytes, buf_start, &HttpSM::tunnel_handler_server, HT_HTTP_SERVER, "http server"); bool const drop_chunked_trailers = t_state.http_config_param->oride.http_drop_chunked_trailers == 1; - tunnel.set_producer_chunking_action(p, 0, action, drop_chunked_trailers); + bool const parse_chunk_strictly = t_state.http_config_param->oride.http_strict_chunk_parsing == 1; + tunnel.set_producer_chunking_action(p, 0, action, drop_chunked_trailers, parse_chunk_strictly); tunnel.set_producer_chunking_size(p, t_state.txn_conf->http_chunking_size); setup_cache_write_transfer(&cache_sm, server_entry->vc, &t_state.cache_info.object_store, 0, "cache write"); @@ -7114,7 +7123,8 @@ HttpSM::setup_server_transfer() } */ bool const drop_chunked_trailers = t_state.http_config_param->oride.http_drop_chunked_trailers == 1; - tunnel.set_producer_chunking_action(p, client_response_hdr_bytes, action, drop_chunked_trailers); + bool const parse_chunk_strictly = t_state.http_config_param->oride.http_strict_chunk_parsing == 1; + tunnel.set_producer_chunking_action(p, client_response_hdr_bytes, action, drop_chunked_trailers, parse_chunk_strictly); tunnel.set_producer_chunking_size(p, t_state.txn_conf->http_chunking_size); return p; } diff --git a/proxy/http/HttpTunnel.cc b/proxy/http/HttpTunnel.cc index 1508179e6b5..e9c0c6eafea 100644 --- a/proxy/http/HttpTunnel.cc +++ b/proxy/http/HttpTunnel.cc @@ -51,27 +51,28 @@ static int const CHUNK_IOBUFFER_SIZE_INDEX = MIN_IOBUFFER_SIZE; ChunkedHandler::ChunkedHandler() : max_chunk_size(DEFAULT_MAX_CHUNK_SIZE) {} void -ChunkedHandler::init(IOBufferReader *buffer_in, HttpTunnelProducer *p, bool drop_chunked_trailers) +ChunkedHandler::init(IOBufferReader *buffer_in, HttpTunnelProducer *p, bool drop_chunked_trailers, bool parse_chunk_strictly) { if (p->do_chunking) { - init_by_action(buffer_in, ACTION_DOCHUNK, drop_chunked_trailers); + init_by_action(buffer_in, ACTION_DOCHUNK, drop_chunked_trailers, parse_chunk_strictly); } else if (p->do_dechunking) { - init_by_action(buffer_in, ACTION_DECHUNK, drop_chunked_trailers); + init_by_action(buffer_in, ACTION_DECHUNK, drop_chunked_trailers, parse_chunk_strictly); } else { - init_by_action(buffer_in, ACTION_PASSTHRU, drop_chunked_trailers); + init_by_action(buffer_in, ACTION_PASSTHRU, drop_chunked_trailers, parse_chunk_strictly); } return; } void -ChunkedHandler::init_by_action(IOBufferReader *buffer_in, Action action, bool drop_chunked_trailers) +ChunkedHandler::init_by_action(IOBufferReader *buffer_in, Action action, bool drop_chunked_trailers, bool parse_chunk_strictly) { - running_sum = 0; - num_digits = 0; - cur_chunk_size = 0; - cur_chunk_bytes_left = 0; - truncation = false; - this->action = action; + running_sum = 0; + num_digits = 0; + cur_chunk_size = 0; + cur_chunk_bytes_left = 0; + truncation = false; + this->action = action; + this->strict_chunk_parsing = parse_chunk_strictly; switch (action) { case ACTION_DOCHUNK: @@ -139,7 +140,6 @@ ChunkedHandler::read_size() { int64_t bytes_consumed = 0; bool done = false; - int cr = 0; while (chunked_reader->is_read_avail_more_than(0) && !done) { const char *tmp = chunked_reader->start(); @@ -178,36 +178,59 @@ ChunkedHandler::read_size() done = true; break; } else { - if (ParseRules::is_cr(*tmp)) { - ++cr; + if ((prev_is_cr = ParseRules::is_cr(*tmp)) == true) { + ++num_cr; } state = CHUNK_READ_SIZE_CRLF; // now look for CRLF } } } else if (state == CHUNK_READ_SIZE_CRLF) { // Scan for a linefeed if (ParseRules::is_lf(*tmp)) { + if (!prev_is_cr) { + Debug("http_chunk", "Found an LF without a preceding CR (protocol violation)"); + if (strict_chunk_parsing) { + state = CHUNK_READ_ERROR; + done = true; + break; + } + } Debug("http_chunk", "read chunk size of %d bytes", running_sum); cur_chunk_bytes_left = (cur_chunk_size = running_sum); state = (running_sum == 0) ? CHUNK_READ_TRAILER_BLANK : CHUNK_READ_CHUNK; done = true; - cr = 0; + num_cr = 0; break; - } else if (ParseRules::is_cr(*tmp)) { - if (cr != 0) { + } else if ((prev_is_cr = ParseRules::is_cr(*tmp)) == true) { + if (num_cr != 0) { state = CHUNK_READ_ERROR; done = true; break; } - ++cr; + ++num_cr; } } else if (state == CHUNK_READ_SIZE_START) { - if (ParseRules::is_cr(*tmp)) { - // Skip it - } else if (ParseRules::is_lf(*tmp) && - bytes_used <= 2) { // bytes_used should be 2 if it's CRLF, but permit a single LF as well + Debug("http_chunk", "CHUNK_READ_SIZE_START 0x%02x", *tmp); + if (ParseRules::is_lf(*tmp)) { + if (!prev_is_cr) { + Debug("http_chunk", "Found an LF without a preceding CR (protocol violation) before chunk size"); + if (strict_chunk_parsing) { + state = CHUNK_READ_ERROR; + done = true; + break; + } + } running_sum = 0; num_digits = 0; + num_cr = 0; state = CHUNK_READ_SIZE; + } else if ((prev_is_cr = ParseRules::is_cr(*tmp)) == true) { + if (num_cr != 0) { + Debug("http_chunk", "Found multiple CRs before chunk size"); + state = CHUNK_READ_ERROR; + done = true; + break; + } + ++num_cr; } else { // Unexpected character state = CHUNK_READ_ERROR; done = true; @@ -651,9 +674,10 @@ HttpTunnel::deallocate_buffers() void HttpTunnel::set_producer_chunking_action(HttpTunnelProducer *p, int64_t skip_bytes, TunnelChunkingAction_t action, - bool drop_chunked_trailers) + bool drop_chunked_trailers, bool parse_chunk_strictly) { this->http_drop_chunked_trailers = drop_chunked_trailers; + this->http_strict_chunk_parsing = parse_chunk_strictly; p->chunked_handler.skip_bytes = skip_bytes; p->chunking_action = action; @@ -878,7 +902,7 @@ HttpTunnel::producer_run(HttpTunnelProducer *p) // For all the chunking cases, we must only copy bytes as we process them. body_bytes_to_copy = 0; - p->chunked_handler.init(p->buffer_start, p, this->http_drop_chunked_trailers); + p->chunked_handler.init(p->buffer_start, p, this->http_drop_chunked_trailers, this->http_strict_chunk_parsing); // Copy the header into the chunked/dechunked buffers. if (p->do_chunking) { diff --git a/proxy/http/HttpTunnel.h b/proxy/http/HttpTunnel.h index 3aac38aca68..9b7d1876425 100644 --- a/proxy/http/HttpTunnel.h +++ b/proxy/http/HttpTunnel.h @@ -112,6 +112,8 @@ struct ChunkedHandler { */ bool drop_chunked_trailers = false; + bool strict_chunk_parsing = true; + bool truncation = false; /** The number of bytes to skip from the reader because they are not body bytes. @@ -130,6 +132,8 @@ struct ChunkedHandler { // Chunked header size parsing info. int running_sum = 0; int num_digits = 0; + int num_cr = 0; + bool prev_is_cr = false; /// @name Output data. //@{ @@ -144,8 +148,8 @@ struct ChunkedHandler { //@} ChunkedHandler(); - void init(IOBufferReader *buffer_in, HttpTunnelProducer *p, bool drop_chunked_trailers); - void init_by_action(IOBufferReader *buffer_in, Action action, bool drop_chunked_trailers); + void init(IOBufferReader *buffer_in, HttpTunnelProducer *p, bool drop_chunked_trailers, bool strict_parsing); + void init_by_action(IOBufferReader *buffer_in, Action action, bool drop_chunked_trailers, bool strict_parsing); void clear(); /// Set the max chunk @a size. @@ -392,6 +396,7 @@ class HttpTunnel : public Continuation /// A named variable for the @a drop_chunked_trailers parameter to @a set_producer_chunking_action. static constexpr bool DROP_CHUNKED_TRAILERS = true; + static constexpr bool PARSE_CHUNK_STRICTLY = true; /** Designate chunking behavior to the producer. * @@ -402,9 +407,10 @@ class HttpTunnel : public Continuation * @param[in] drop_chunked_trailers If @c true, chunked trailers are filtered * out. Logically speaking, this is only applicable when proxying chunked * content, thus only when @a action is @c TCA_PASSTHRU_CHUNKED_CONTENT. + * @param[in] parse_chunk_strictly If @c true, no parse error will be allowed */ void set_producer_chunking_action(HttpTunnelProducer *p, int64_t skip_bytes, TunnelChunkingAction_t action, - bool drop_chunked_trailers); + bool drop_chunked_trailers, bool parse_chunk_strictly); /// Set the maximum (preferred) chunk @a size of chunked output for @a producer. void set_producer_chunking_size(HttpTunnelProducer *producer, int64_t size); @@ -483,6 +489,9 @@ class HttpTunnel : public Continuation /// Corresponds to proxy.config.http.drop_chunked_trailers having a value of 1. bool http_drop_chunked_trailers = false; + /// Corresponds to proxy.config.http.strict_chunk_parsing having a value of 1. + bool http_strict_chunk_parsing = false; + /** The number of body bytes processed in this last execution of the tunnel. * * This accounting is used to determine how many bytes to copy into the body diff --git a/src/shared/overridable_txn_vars.cc b/src/shared/overridable_txn_vars.cc index 1a5d740794a..f8c6e6e58ea 100644 --- a/src/shared/overridable_txn_vars.cc +++ b/src/shared/overridable_txn_vars.cc @@ -31,6 +31,7 @@ const std::unordered_mapinit_by_action(resp_reader, ChunkedHandler::ACTION_DECHUNK, HttpTunnel::DROP_CHUNKED_TRAILERS); + ch->init_by_action(resp_reader, ChunkedHandler::ACTION_DECHUNK, HttpTunnel::DROP_CHUNKED_TRAILERS, + HttpTunnel::PARSE_CHUNK_STRICTLY); ch->dechunked_reader = ch->dechunked_buffer->alloc_reader(); ch->state = ChunkedHandler::CHUNK_READ_SIZE; resp_reader->dealloc(); diff --git a/src/traffic_server/InkAPI.cc b/src/traffic_server/InkAPI.cc index 71adb94d0cc..40bc7608ffc 100644 --- a/src/traffic_server/InkAPI.cc +++ b/src/traffic_server/InkAPI.cc @@ -8928,6 +8928,9 @@ _conf_to_memberp(TSOverridableConfigKey conf, OverridableHttpConfigParams *overr case TS_CONFIG_HTTP_DROP_CHUNKED_TRAILERS: ret = _memberp_to_generic(&overridableHttpConfig->http_drop_chunked_trailers, conv); break; + case TS_CONFIG_HTTP_STRICT_CHUNK_PARSING: + ret = _memberp_to_generic(&overridableHttpConfig->http_strict_chunk_parsing, conv); + break; case TS_CONFIG_HTTP_FLOW_CONTROL_ENABLED: ret = _memberp_to_generic(&overridableHttpConfig->flow_control_enabled, conv); break; diff --git a/src/traffic_server/InkAPITest.cc b/src/traffic_server/InkAPITest.cc index a6e7217291a..0e0cac586be 100644 --- a/src/traffic_server/InkAPITest.cc +++ b/src/traffic_server/InkAPITest.cc @@ -8774,7 +8774,8 @@ std::array SDK_Overridable_Configs = { "proxy.config.body_factory.response_suppression_mode", "proxy.config.http.parent_proxy.enable_parent_timeout_markdowns", "proxy.config.http.parent_proxy.disable_parent_markdowns", - "proxy.config.http.drop_chunked_trailers"}}; + "proxy.config.http.drop_chunked_trailers", + "proxy.config.http.strict_chunk_parsing"}}; extern ClassAllocator httpSMAllocator; diff --git a/tests/gold_tests/chunked_encoding/replays/malformed_chunked_header.replay.yaml b/tests/gold_tests/chunked_encoding/replays/malformed_chunked_header.replay.yaml index 1118036b3c8..7c0ccb9a47f 100644 --- a/tests/gold_tests/chunked_encoding/replays/malformed_chunked_header.replay.yaml +++ b/tests/gold_tests/chunked_encoding/replays/malformed_chunked_header.replay.yaml @@ -98,6 +98,27 @@ sessions: server-response: status: 200 +- transactions: + - client-request: + method: "POST" + version: "1.1" + url: /malformed/chunk/header3 + headers: + fields: + - [ Host, example.com ] + - [ Transfer-Encoding, chunked ] + - [ uuid, 5 ] + content: + transfer: plain + encoding: uri + # Chunk header must end with a sequence of CRLF. + data: 7;x%0Aabcwxyz%0D%0A0%0D%0A%0D%0A + + # The connection will be dropped and this response will not go out. + server-response: + status: 200 + + # # Now repeat the above two malformed chunk header tests, but on the server # side. @@ -193,3 +214,26 @@ sessions: encoding: uri # BWS cannot have CR data: 3%0D%0D%0Adef%0D%0A0%0D%0A%0D%0A + +- transactions: + - client-request: + method: "GET" + version: "1.1" + url: /response/malformed/chunk/size2 + headers: + fields: + - [ Host, example.com ] + - [ uuid, 105 ] + + # The connection will be dropped and this response will not go out. + server-response: + status: 200 + reason: OK + headers: + fields: + - [ Transfer-Encoding, chunked ] + content: + transfer: plain + encoding: uri + # Chunk header must end with a sequence of CRLF. + data: 3;x%0Adef%0D%0A0%0D%0A%0D%0A