Traffic Dump: fix YAML format for CONNECT requests

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();
 }

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 } ]
+

tests/gold_tests/pluginTest/traffic_dump/traffic_dump.test.py

@@ -50,7 +50,7 @@
 
 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 @@
     '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 @@
 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.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