You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2022/10/17 22:23:00 UTC
[trafficserver] branch 9.2.x updated: Traffic Dump: fix YAML format for CONNECT requests (#9139)
This is an automated email from the ASF dual-hosted git repository.
zwoop 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 ab573761b Traffic Dump: fix YAML format for CONNECT requests (#9139)
ab573761b is described below
commit ab573761b2d93b21e7e5db718b1e78f78d70d120
Author: Brian Neradt <br...@gmail.com>
AuthorDate: Mon Oct 17 13:46:45 2022 -0500
Traffic Dump: fix YAML format for CONNECT requests (#9139)
For CONNECT requests, Traffic Dump will not have a server-side protocol
stack because we populate that on HTTP responses from the server. Since
we simply establish a connection to the origin without exchanging an
HTTP message, the stack is never populated. This resulted in malformed
YAML because we started a sequence with a ','. This patch addresses this
by not adding the ',' if the protocol description string is empty.
(cherry picked from commit 96acfe8192213c0af6a1df81c0c70417b505a015)
---
.../experimental/traffic_dump/transaction_data.cc | 9 +++++-
.../traffic_dump/replay/traffic_dump.yaml | 35 ++++++++++++++++++++++
.../pluginTest/traffic_dump/traffic_dump.test.py | 34 ++++++++++++++-------
3 files changed, 67 insertions(+), 11 deletions(-)
diff --git a/plugins/experimental/traffic_dump/transaction_data.cc b/plugins/experimental/traffic_dump/transaction_data.cc
index 37fa7089d..4c6f3482d 100644
--- a/plugins/experimental/traffic_dump/transaction_data.cc
+++ b/plugins/experimental/traffic_dump/transaction_data.cc
@@ -367,7 +367,14 @@ TransactionData::write_proxy_request_node(TSMBuffer &buffer, TSMLoc &hdr_loc)
{
std::ostringstream proxy_request_node;
proxy_request_node << R"(,"proxy-request":{)";
- proxy_request_node << _server_protocol_description + ",";
+ // For CONNECT requests, the _server_protocol_description will be empty
+ // because it gets populated on receipt of an HTTP response. For tunnels, we
+ // establish a connection to the origin for the tunnel but don't send an HTTP
+ // request nor does the server send a response. Therefore the protocol is
+ // never populated and will be empty here.
+ if (!_server_protocol_description.empty()) {
+ proxy_request_node << _server_protocol_description + ",";
+ }
proxy_request_node << write_message_node(buffer, hdr_loc, TSHttpTxnServerReqBodyBytesGet(_txnp));
_txn_json += proxy_request_node.str();
}
diff --git a/tests/gold_tests/pluginTest/traffic_dump/replay/traffic_dump.yaml b/tests/gold_tests/pluginTest/traffic_dump/replay/traffic_dump.yaml
index 2a55a4900..fc76d0f39 100644
--- a/tests/gold_tests/pluginTest/traffic_dump/replay/traffic_dump.yaml
+++ b/tests/gold_tests/pluginTest/traffic_dump/replay/traffic_dump.yaml
@@ -333,3 +333,38 @@ sessions:
proxy-response:
status: 200
+- transactions:
+ - client-request:
+ method: CONNECT
+ url: www.connect_target.com:443/tunnel/me
+ version: 1.1
+ headers:
+ fields:
+ - [ Host, www.connect_target.com ]
+ - [ X-Request-1, request_tunnel ]
+ - [ Content-Length, 0 ]
+ - [ uuid, tunnel ]
+
+ proxy-request:
+ headers:
+ fields:
+ # The field should get through to the server. The traffic dump, though,
+ # should not contain it since x-request-1 is a sensitive field.
+ - [ X-Request-1, { value: request_tunnel, as: equal } ]
+
+ server-response:
+ status: 200
+ reason: OK
+ headers:
+ fields:
+ - [ Content-Length, 16 ]
+ - [ X-Response-1, response_tunnel ]
+
+ proxy-response:
+ status: 200
+ headers:
+ field:
+ # Again, the sensitive set-cookie should get through to the client, it
+ # just shouldn't be dumped in traffic dumps.
+ - [ X-Response-1, { value: response_tunnel, as: equal } ]
+
diff --git a/tests/gold_tests/pluginTest/traffic_dump/traffic_dump.test.py b/tests/gold_tests/pluginTest/traffic_dump/traffic_dump.test.py
index 1aa57e47f..bf32cf608 100644
--- a/tests/gold_tests/pluginTest/traffic_dump/traffic_dump.test.py
+++ b/tests/gold_tests/pluginTest/traffic_dump/traffic_dump.test.py
@@ -50,7 +50,7 @@ ts.Setup.Copy("ssl/signed-foo.key")
ts.Disk.records_config.update({
'proxy.config.diags.debug.enabled': 1,
- 'proxy.config.diags.debug.tags': 'traffic_dump',
+ 'proxy.config.diags.debug.tags': 'traffic_dump|http',
'proxy.config.http.insert_age_in_response': 0,
'proxy.config.ssl.server.cert.path': ts.Variables.SSLDir,
@@ -61,21 +61,20 @@ ts.Disk.records_config.update({
'proxy.config.http.host_sni_policy': 2,
'proxy.config.ssl.TLSv1_3': 0,
'proxy.config.ssl.client.verify.server.policy': 'PERMISSIVE',
+
+ 'proxy.config.http.connect_ports': f"{server.Variables.http_port}",
})
ts.Disk.ssl_multicert_config.AddLine(
'dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key'
)
-ts.Disk.remap_config.AddLine(
- f'map https://www.client_only_tls.com/ http://127.0.0.1:{server.Variables.http_port}'
-)
-ts.Disk.remap_config.AddLine(
- f'map https://www.tls.com/ https://127.0.0.1:{server.Variables.https_port}'
-)
-ts.Disk.remap_config.AddLine(
- f'map / http://127.0.0.1:{server.Variables.http_port}'
-)
+ts.Disk.remap_config.AddLines([
+ f'map https://www.client_only_tls.com/ http://127.0.0.1:{server.Variables.http_port}',
+ f'map https://www.tls.com/ https://127.0.0.1:{server.Variables.https_port}',
+ f'map http://www.connect_target.com/ http://127.0.0.1:{server.Variables.http_port}',
+ f'map / http://127.0.0.1:{server.Variables.http_port}',
+])
# Configure traffic_dump.
ts.Disk.plugin_config.AddLine(
@@ -132,6 +131,8 @@ replay_file_session_9 = os.path.join(replay_dir, "127", "0000000000000008")
ts.Disk.File(replay_file_session_9, exists=True)
replay_file_session_10 = os.path.join(replay_dir, "127", "0000000000000009")
ts.Disk.File(replay_file_session_10, exists=True)
+replay_file_session_11 = os.path.join(replay_dir, "127", "000000000000000a")
+ts.Disk.File(replay_file_session_11, exists=True)
# Run our test traffic.
tr = Test.AddTestRun("Run the test traffic.")
@@ -303,3 +304,16 @@ tr.Processes.Default.Command = \
tr.Processes.Default.ReturnCode = 0
tr.StillRunningAfter = server
tr.StillRunningAfter = ts
+
+#
+# Test 9: Verify correct handling of a CONNECT request.
+#
+
+tr = Test.AddTestRun("Verify handling of a CONNECT request.")
+tr.Setup.CopyAs(verify_replay, Test.RunDirectory)
+
+tr.Processes.Default.Command = \
+ (f"{sys.executable} {verify_replay} {schema_path} {replay_file_session_11} {sensitive_fields_arg} ")
+tr.Processes.Default.ReturnCode = 0
+tr.StillRunningAfter = server
+tr.StillRunningAfter = ts