This is an automated email from the ASF dual-hosted git repository.
maskit pushed a commit to branch 9.2.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/9.2.x by this push:
new 3d2f29c88f Require the use of CRLF in chunked message body (#12150)
3d2f29c88f is described below
commit 3d2f29c88f9b073cb0fd3b9c7f85430e2170acbb
Author: Masakazu Kitajo <[email protected]>
AuthorDate: Tue Apr 1 12:15:16 2025 -0600
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 +++
.../api/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 +-
.../replays/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 b81510db69..7db9e6a9f6 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 2ec2983153..b2b0e23150 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 2d0941efde..b4291b4657 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 1641565a1a..893177c88b 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 ff7fdc0e3c..e645bb6c6f 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 a25d8ab8c8..4b22ee94b5 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 d5c1c00a28..ca2edee1ee 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 6c1763f84e..53450bdbb2 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 cdc0546132..c0ba82641e 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 1508179e6b..e9c0c6eafe 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 3aac38aca6..9b7d187642 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 @@ public:
/// 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 @@ public:
* @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 @@ private:
/// 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 1a5d740794..f8c6e6e58e 100644
--- a/src/shared/overridable_txn_vars.cc
+++ b/src/shared/overridable_txn_vars.cc
@@ -31,6 +31,7 @@ const std::unordered_map<std::string_view, std::tuple<const
TSOverridableConfigK
{"proxy.config.http.normalize_ae", {TS_CONFIG_HTTP_NORMALIZE_AE,
TS_RECORDDATATYPE_INT}},
{"proxy.config.http.chunking.size", {TS_CONFIG_HTTP_CHUNKING_SIZE,
TS_RECORDDATATYPE_INT}},
{"proxy.config.http.drop_chunked_trailers",
{TS_CONFIG_HTTP_DROP_CHUNKED_TRAILERS, TS_RECORDDATATYPE_INT}},
+ {"proxy.config.http.strict_chunk_parsing",
{TS_CONFIG_HTTP_STRICT_CHUNK_PARSING, TS_RECORDDATATYPE_INT}},
{"proxy.config.ssl.client.cert.path", {TS_CONFIG_SSL_CERT_FILEPATH,
TS_RECORDDATATYPE_STRING}},
{"proxy.config.http.allow_half_open", {TS_CONFIG_HTTP_ALLOW_HALF_OPEN,
TS_RECORDDATATYPE_INT}},
{"proxy.config.http.chunking_enabled", {TS_CONFIG_HTTP_CHUNKING_ENABLED,
TS_RECORDDATATYPE_INT}},
diff --git a/src/traffic_server/FetchSM.cc b/src/traffic_server/FetchSM.cc
index 19303b7e03..ad5634845b 100644
--- a/src/traffic_server/FetchSM.cc
+++ b/src/traffic_server/FetchSM.cc
@@ -198,7 +198,8 @@ FetchSM::check_chunked()
if (resp_is_chunked && (fetch_flags & TS_FETCH_FLAGS_DECHUNK)) {
ChunkedHandler *ch = &chunked_handler;
- ch->init_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 71adb94d0c..40bc7608ff 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 a6e7217291..0e0cac586b 100644
--- a/src/traffic_server/InkAPITest.cc
+++ b/src/traffic_server/InkAPITest.cc
@@ -8774,7 +8774,8 @@ std::array<std::string_view, TS_CONFIG_LAST_ENTRY>
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<HttpSM> 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 1118036b3c..7c0ccb9a47 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
