Skip to content

Commit f508f74

Browse files
committed
Transfer-Encoding:chunked log field fix
When an origin server returns Transfer-Encoding:chunked, ATS removes this header during response processing to handle dechunking. This prevented the original header value from being logged via %<{Transfer-Encoding}ssh>. This fix stores the original Transfer-Encoding value before deletion and uses the stored value if requested via the ssh field. Fixes: #6261
1 parent a0f4cf5 commit f508f74

File tree

5 files changed

+41
-5
lines changed

5 files changed

+41
-5
lines changed

include/proxy/http/HttpTransact.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -635,6 +635,7 @@ class HttpTransact
635635
bool trust_response_cl = false;
636636
ResponseError_t response_error = ResponseError_t::NO_RESPONSE_HEADER_ERROR;
637637
bool extension_method = false;
638+
std::string server_response_transfer_encoding; ///< Storage for logging.
638639

639640
_HeaderInfo() {}
640641
};

src/proxy/http/HttpTransact.cc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5850,6 +5850,9 @@ HttpTransact::initialize_state_variables_from_response(State *s, HTTPHdr *incomi
58505850
new_enc_val = new_enc_iter.get_next(&new_enc_len);
58515851
}
58525852

5853+
// Store the original Transfer-Encoding value for logging before we delete it.
5854+
s->hdr_info.server_response_transfer_encoding.assign(field->value_get().data(), field->value_get().length());
5855+
58535856
// We're done with the old field since we copied out everything
58545857
// we needed
58555858
incoming_response->field_delete(field);

src/proxy/logging/LogAccess.cc

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3246,6 +3246,34 @@ LogAccess::marshal_http_header_field(LogField::Container container, char *field,
32463246
}
32473247
}
32483248

3249+
// The Transfer-Encoding:chunked value may have been removed from the header
3250+
// during processing, but we store it for logging purposes.
3251+
if (valid_field == false && container == LogField::SSH && strcmp(field, "Transfer-Encoding") == 0) {
3252+
const std::string &stored_te = m_http_sm->t_state.hdr_info.server_response_transfer_encoding;
3253+
if (!stored_te.empty()) {
3254+
valid_field = true;
3255+
actual_len = stored_te.length();
3256+
str = const_cast<char *>(stored_te.data());
3257+
3258+
int running_len = actual_len + 1; // +1 for null terminator
3259+
padded_len = padded_length(running_len);
3260+
3261+
if (buf) {
3262+
memcpy(buf, str, actual_len);
3263+
buf[actual_len] = '\0';
3264+
buf += running_len;
3265+
3266+
#ifdef DEBUG
3267+
int pad_len = padded_len - running_len;
3268+
for (int i = 0; i < pad_len; i++) {
3269+
*buf = '$';
3270+
buf++;
3271+
}
3272+
#endif
3273+
}
3274+
}
3275+
}
3276+
32493277
if (valid_field == false) {
32503278
padded_len = INK_MIN_ALIGN;
32513279
if (buf) {
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
application/json,%20application/json
2-
application/jason,%20application/json
3-
application/json
1+
Transfer-Encoding:chunked Content-Type:application/json,%20application/json
2+
Transfer-Encoding:- Content-Type:application/jason,%20application/json
3+
Transfer-Encoding:- Content-Type:application/json

tests/gold_tests/logging/log-field.test.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
response_header = {
3030
'timestamp': 100,
3131
"headers":
32-
"HTTP/1.1 200 OK\r\nTest: 1\r\nContent-Type: application/json\r\nConnection: close\r\nContent-Type: application/json\r\n\r\n",
32+
"HTTP/1.1 200 OK\r\nTest: 1\r\nContent-Type: application/json\r\nConnection: close\r\nContent-Type: application/json\r\nTransfer-Encoding: chunked\r\n\r\n",
3333
"body": "Test 1"
3434
}
3535
server.addResponse("sessionlog.json", request_header, response_header)
@@ -59,6 +59,8 @@
5959

6060
ts.Disk.records_config.update(
6161
{
62+
'proxy.config.diags.debug.enabled': 1,
63+
'proxy.config.diags.debug.tags': 'http|log',
6264
'proxy.config.net.connections_throttle': 100,
6365
'proxy.config.dns.nameservers': f"127.0.0.1:{nameserver.Variables.Port}",
6466
'proxy.config.dns.resolv_conf': 'NULL'
@@ -71,7 +73,7 @@
7173
logging:
7274
formats:
7375
- name: custom
74-
format: '%<{Content-Type}essh>'
76+
format: 'Transfer-Encoding:%<{Transfer-Encoding}ssh> Content-Type:%<{Content-Type}essh>'
7577
logs:
7678
- filename: field-test
7779
format: custom
@@ -81,6 +83,8 @@
8183
# at the end of the different test run a custom log file should exist
8284
# Because of this we expect the testruns to pass the real test is if the
8385
# customlog file exists and passes the format check
86+
# Note that the gold file expects Chunked, not chunked, because microserver
87+
# converts to Chunked. This is not an ATS issue, but a microserver issue.
8488
Test.Disk.File(os.path.join(ts.Variables.LOGDIR, 'field-test.log'), exists=True, content='gold/field-test.gold')
8589

8690
# first test is a miss for default

0 commit comments

Comments
 (0)