From 4cd871656070982bafa4f0f52c7ded8e7d4233ab Mon Sep 17 00:00:00 2001 From: bneradt Date: Tue, 3 Dec 2019 16:23:14 +0000 Subject: [PATCH] Fixing TS_HTTP_REQUEST_BUFFER_READ_COMPLETE_HOOK enum value. The value for TS_HTTP_REQUEST_BUFFER_READ_COMPLETE_HOOK was out of sync with the corresponding event, TS_EVENT_HTTP_REQUEST_BUFFER_READ_COMPLETE. This caused the handler for TS_HTTP_REQUEST_BUFFER_READ_COMPLETE_HOOK to never be invoked with the TS_EVENT_HTTP_REQUEST_BUFFER_READ_COMPLETE event. This patch fixes this problem and adds comments explaining the otherwise implicit but very important requirement that these values correspond as they do. The naming of the event and hook were off too. A hook with name TS_HTTP_X_HOOK should have an event of name TS_EVENT_HTTP_X, but these got out of sync somehow for request buffer read complete. This adjusts the event name appropriately. --- .../c-api/request_buffer/request_buffer.c | 2 +- include/ts/apidefs.h.in | 293 ++++++++++-------- proxy/ProxySession.cc | 2 +- proxy/http/HttpDebugNames.cc | 4 +- proxy/http/HttpSM.cc | 22 +- src/traffic_server/InkAPITest.cc | 4 +- .../gold_tests/pluginTest/test_hooks/200.gold | 28 ++ .../pluginTest/test_hooks/body_buffer.test.py | 121 ++++++++ 8 files changed, 331 insertions(+), 145 deletions(-) create mode 100644 tests/gold_tests/pluginTest/test_hooks/200.gold create mode 100644 tests/gold_tests/pluginTest/test_hooks/body_buffer.test.py diff --git a/example/plugins/c-api/request_buffer/request_buffer.c b/example/plugins/c-api/request_buffer/request_buffer.c index 69ab0c6269d..2214cda5243 100644 --- a/example/plugins/c-api/request_buffer/request_buffer.c +++ b/example/plugins/c-api/request_buffer/request_buffer.c @@ -67,7 +67,7 @@ request_buffer_plugin(TSCont contp, TSEvent event, void *edata) { TSDebug(PLUGIN_NAME, "request_buffer_plugin starting, event[%d]", event); TSHttpTxn txnp = (TSHttpTxn)(edata); - if (event == TS_EVENT_HTTP_REQUEST_BUFFER_COMPLETE) { + if (event == TS_EVENT_HTTP_REQUEST_BUFFER_READ_COMPLETE) { int len = 0; char *body = request_body_get(txnp, &len); TSDebug(PLUGIN_NAME, "request_buffer_plugin gets the request body with length[%d]", len); diff --git a/include/ts/apidefs.h.in b/include/ts/apidefs.h.in index fd745795083..7912935d844 100644 --- a/include/ts/apidefs.h.in +++ b/include/ts/apidefs.h.in @@ -264,6 +264,122 @@ typedef enum { TS_HTTP_STATUS_NETWORK_AUTHENTICATION_REQUIRED = 511 } TSHttpStatus; +/** + TSEvents are sent to continuations when they are called back. + The TSEvent provides the continuation's handler function with + information about the callback. Based on the event it receives, + the handler function can decide what to do. + */ +typedef enum { + TS_EVENT_NONE = 0, + TS_EVENT_IMMEDIATE = 1, + TS_EVENT_TIMEOUT = 2, + TS_EVENT_ERROR = 3, + TS_EVENT_CONTINUE = 4, + + TS_EVENT_VCONN_READ_READY = 100, + TS_EVENT_VCONN_WRITE_READY = 101, + TS_EVENT_VCONN_READ_COMPLETE = 102, + TS_EVENT_VCONN_WRITE_COMPLETE = 103, + TS_EVENT_VCONN_EOS = 104, + TS_EVENT_VCONN_INACTIVITY_TIMEOUT = 105, + TS_EVENT_VCONN_ACTIVE_TIMEOUT = 106, + TS_EVENT_VCONN_START = 107, + TS_EVENT_VCONN_CLOSE = 108, + TS_EVENT_VCONN_OUTBOUND_START = 109, + TS_EVENT_VCONN_OUTBOUND_CLOSE = 110, + TS_EVENT_VCONN_PRE_ACCEPT = TS_EVENT_VCONN_START, // Deprecated but still compatible + + TS_EVENT_NET_CONNECT = 200, + TS_EVENT_NET_CONNECT_FAILED = 201, + TS_EVENT_NET_ACCEPT = 202, + TS_EVENT_NET_ACCEPT_FAILED = 204, + + TS_EVENT_INTERNAL_206 = 206, + TS_EVENT_INTERNAL_207 = 207, + TS_EVENT_INTERNAL_208 = 208, + TS_EVENT_INTERNAL_209 = 209, + TS_EVENT_INTERNAL_210 = 210, + TS_EVENT_INTERNAL_211 = 211, + TS_EVENT_INTERNAL_212 = 212, + + TS_EVENT_HOST_LOOKUP = 500, + + TS_EVENT_CACHE_OPEN_READ = 1102, + TS_EVENT_CACHE_OPEN_READ_FAILED = 1103, + TS_EVENT_CACHE_OPEN_WRITE = 1108, + TS_EVENT_CACHE_OPEN_WRITE_FAILED = 1109, + TS_EVENT_CACHE_REMOVE = 1112, + TS_EVENT_CACHE_REMOVE_FAILED = 1113, + TS_EVENT_CACHE_SCAN = 1120, + TS_EVENT_CACHE_SCAN_FAILED = 1121, + TS_EVENT_CACHE_SCAN_OBJECT = 1122, + TS_EVENT_CACHE_SCAN_OPERATION_BLOCKED = 1123, + TS_EVENT_CACHE_SCAN_OPERATION_FAILED = 1124, + TS_EVENT_CACHE_SCAN_DONE = 1125, + TS_EVENT_CACHE_LOOKUP = 1126, + TS_EVENT_CACHE_READ = 1127, + TS_EVENT_CACHE_DELETE = 1128, + TS_EVENT_CACHE_WRITE = 1129, + TS_EVENT_CACHE_WRITE_HEADER = 1130, + TS_EVENT_CACHE_CLOSE = 1131, + TS_EVENT_CACHE_LOOKUP_READY = 1132, + TS_EVENT_CACHE_LOOKUP_COMPLETE = 1133, + TS_EVENT_CACHE_READ_READY = 1134, + TS_EVENT_CACHE_READ_COMPLETE = 1135, + + TS_EVENT_INTERNAL_1200 = 1200, + + TS_EVENT_SSL_SESSION_GET = 2000, + TS_EVENT_SSL_SESSION_NEW = 2001, + TS_EVENT_SSL_SESSION_REMOVE = 2002, + + TS_EVENT_AIO_DONE = 3900, + + TS_EVENT_HTTP_CONTINUE = 60000, + TS_EVENT_HTTP_ERROR = 60001, + TS_EVENT_HTTP_READ_REQUEST_HDR = 60002, + TS_EVENT_HTTP_OS_DNS = 60003, + TS_EVENT_HTTP_SEND_REQUEST_HDR = 60004, + TS_EVENT_HTTP_READ_CACHE_HDR = 60005, + TS_EVENT_HTTP_READ_RESPONSE_HDR = 60006, + TS_EVENT_HTTP_SEND_RESPONSE_HDR = 60007, + TS_EVENT_HTTP_REQUEST_TRANSFORM = 60008, + TS_EVENT_HTTP_RESPONSE_TRANSFORM = 60009, + TS_EVENT_HTTP_SELECT_ALT = 60010, + TS_EVENT_HTTP_TXN_START = 60011, + TS_EVENT_HTTP_TXN_CLOSE = 60012, + TS_EVENT_HTTP_SSN_START = 60013, + TS_EVENT_HTTP_SSN_CLOSE = 60014, + TS_EVENT_HTTP_CACHE_LOOKUP_COMPLETE = 60015, + TS_EVENT_HTTP_PRE_REMAP = 60016, + TS_EVENT_HTTP_POST_REMAP = 60017, + TS_EVENT_HTTP_REQUEST_BUFFER_READ_COMPLETE = 60018, + TS_EVENT_HTTP_RESPONSE_CLIENT = 60019, + + TS_EVENT_LIFECYCLE_PORTS_INITIALIZED = 60100, + TS_EVENT_LIFECYCLE_PORTS_READY = 60101, + TS_EVENT_LIFECYCLE_CACHE_READY = 60102, + TS_EVENT_LIFECYCLE_SERVER_SSL_CTX_INITIALIZED = 60103, + TS_EVENT_LIFECYCLE_CLIENT_SSL_CTX_INITIALIZED = 60104, + TS_EVENT_LIFECYCLE_MSG = 60105, + TS_EVENT_LIFECYCLE_TASK_THREADS_READY = 60106, + TS_EVENT_LIFECYCLE_SHUTDOWN = 60107, + + TS_EVENT_INTERNAL_60200 = 60200, + TS_EVENT_INTERNAL_60201 = 60201, + TS_EVENT_INTERNAL_60202 = 60202, + TS_EVENT_SSL_CERT = 60203, + TS_EVENT_SSL_SERVERNAME = 60204, + TS_EVENT_SSL_VERIFY_SERVER = 60205, + TS_EVENT_SSL_VERIFY_CLIENT = 60206, + TS_EVENT_SSL_CLIENT_HELLO = 60207, + TS_EVENT_SSL_SECRET = 60208, + + TS_EVENT_MGMT_UPDATE = 60300 +} TSEvent; +#define TS_EVENT_HTTP_READ_REQUEST_PRE_REMAP TS_EVENT_HTTP_PRE_REMAP /* backwards compat */ + /** This set of enums represents the possible hooks where you can set up continuation callbacks. The functions used to register a @@ -316,28 +432,52 @@ typedef enum { TS_HTTP_LAST_HOOK _must_ be the last element. Only right place to insert a new element is just before TS_HTTP_LAST_HOOK. + + @note The TS_HTTP hooks below have to be in the same order as their + corresponding TS_EVENT counterparts. We use this in calculating the + corresponding event from a hook ID by summing + TS_EVENT_HTTP_READ_REQUEST_HDR with the hook ID (see the invoke call in + HttpSM::state_api_callout). For example, the following expression must be + true: + + TS_EVENT_HTTP_TXN_CLOSE == TS_EVENT_HTTP_READ_REQUEST_HDR + TS_HTTP_TXN_CLOSE_HOOK + */ typedef enum { - TS_HTTP_READ_REQUEST_HDR_HOOK, - TS_HTTP_OS_DNS_HOOK, - TS_HTTP_SEND_REQUEST_HDR_HOOK, - TS_HTTP_READ_CACHE_HDR_HOOK, - TS_HTTP_READ_RESPONSE_HDR_HOOK, - TS_HTTP_SEND_RESPONSE_HDR_HOOK, - TS_HTTP_REQUEST_TRANSFORM_HOOK, - TS_HTTP_RESPONSE_TRANSFORM_HOOK, - TS_HTTP_SELECT_ALT_HOOK, - TS_HTTP_TXN_START_HOOK, - TS_HTTP_TXN_CLOSE_HOOK, - TS_HTTP_SSN_START_HOOK, - TS_HTTP_SSN_CLOSE_HOOK, - TS_HTTP_CACHE_LOOKUP_COMPLETE_HOOK, - TS_HTTP_PRE_REMAP_HOOK, - TS_HTTP_POST_REMAP_HOOK, - TS_HTTP_RESPONSE_CLIENT_HOOK, +/* + The following macro helps maintain the relationship between the TS_HTTP + hooks and their TS_EVENT_HTTP counterparts as described in the above + doxygen comment. + */ +#define DERIVE_HOOK_VALUE_FROM_EVENT(COMMON) TS_HTTP_##COMMON##_HOOK = TS_EVENT_HTTP_##COMMON - TS_EVENT_HTTP_READ_REQUEST_HDR + DERIVE_HOOK_VALUE_FROM_EVENT(READ_REQUEST_HDR), // TS_HTTP_READ_REQUEST_HDR_HOOK + DERIVE_HOOK_VALUE_FROM_EVENT(OS_DNS), // TS_HTTP_OS_DNS_HOOK + DERIVE_HOOK_VALUE_FROM_EVENT(SEND_REQUEST_HDR), // TS_HTTP_SEND_REQUEST_HDR_HOOK + DERIVE_HOOK_VALUE_FROM_EVENT(READ_CACHE_HDR), // TS_HTTP_READ_CACHE_HDR_HOOK + DERIVE_HOOK_VALUE_FROM_EVENT(READ_RESPONSE_HDR), // TS_HTTP_READ_RESPONSE_HDR_HOOK + DERIVE_HOOK_VALUE_FROM_EVENT(SEND_RESPONSE_HDR), // TS_HTTP_SEND_RESPONSE_HDR_HOOK + DERIVE_HOOK_VALUE_FROM_EVENT(REQUEST_TRANSFORM), // TS_HTTP_REQUEST_TRANSFORM_HOOK + DERIVE_HOOK_VALUE_FROM_EVENT(RESPONSE_TRANSFORM), // TS_HTTP_RESPONSE_TRANSFORM_HOOK + DERIVE_HOOK_VALUE_FROM_EVENT(SELECT_ALT), // TS_HTTP_SELECT_ALT_HOOK + DERIVE_HOOK_VALUE_FROM_EVENT(TXN_START), // TS_HTTP_TXN_START_HOOK + DERIVE_HOOK_VALUE_FROM_EVENT(TXN_CLOSE), // TS_HTTP_TXN_CLOSE_HOOK + DERIVE_HOOK_VALUE_FROM_EVENT(SSN_START), // TS_HTTP_SSN_START_HOOK + DERIVE_HOOK_VALUE_FROM_EVENT(SSN_CLOSE), // TS_HTTP_SSN_CLOSE_HOOK + DERIVE_HOOK_VALUE_FROM_EVENT(CACHE_LOOKUP_COMPLETE), // TS_HTTP_CACHE_LOOKUP_COMPLETE_HOOK + DERIVE_HOOK_VALUE_FROM_EVENT(PRE_REMAP), // TS_HTTP_PRE_REMAP_HOOK + DERIVE_HOOK_VALUE_FROM_EVENT(POST_REMAP), // TS_HTTP_POST_REMAP_HOOK + DERIVE_HOOK_VALUE_FROM_EVENT(REQUEST_BUFFER_READ_COMPLETE), // TS_HTTP_REQUEST_BUFFER_READ_COMPLETE_HOOK + DERIVE_HOOK_VALUE_FROM_EVENT(RESPONSE_CLIENT), // TS_HTTP_RESPONSE_CLIENT_HOOK + +// NOTE: +// If adding any TS_HTTP hooks, be sure to understand the note above in the +// doxygen comment about the ordering of these enum values with respect to +// their corresponding TS_EVENT values. +#undef DERIVE_HOOK_VALUE_FROM_EVENT + // Putting the SSL hooks in the same enum space // So both sets of hooks can be set by the same Hook function - TS_SSL_FIRST_HOOK, + TS_SSL_FIRST_HOOK = 201, TS_VCONN_START_HOOK = TS_SSL_FIRST_HOOK, TS_VCONN_PRE_ACCEPT_HOOK = TS_VCONN_START_HOOK, // Deprecated but compatible for now. TS_VCONN_CLOSE_HOOK, @@ -352,7 +492,6 @@ typedef enum { TS_VCONN_OUTBOUND_START_HOOK, TS_VCONN_OUTBOUND_CLOSE_HOOK, TS_SSL_LAST_HOOK = TS_VCONN_OUTBOUND_CLOSE_HOOK, - TS_HTTP_REQUEST_BUFFER_READ_COMPLETE_HOOK, TS_HTTP_LAST_HOOK } TSHttpHookID; @@ -442,122 +581,6 @@ typedef enum { TS_LIFECYCLE_LAST_HOOK } TSLifecycleHookID; -/** - TSEvents are sent to continuations when they are called back. - The TSEvent provides the continuation's handler function with - information about the callback. Based on the event it receives, - the handler function can decide what to do. - - */ -typedef enum { - TS_EVENT_NONE = 0, - TS_EVENT_IMMEDIATE = 1, - TS_EVENT_TIMEOUT = 2, - TS_EVENT_ERROR = 3, - TS_EVENT_CONTINUE = 4, - - TS_EVENT_VCONN_READ_READY = 100, - TS_EVENT_VCONN_WRITE_READY = 101, - TS_EVENT_VCONN_READ_COMPLETE = 102, - TS_EVENT_VCONN_WRITE_COMPLETE = 103, - TS_EVENT_VCONN_EOS = 104, - TS_EVENT_VCONN_INACTIVITY_TIMEOUT = 105, - TS_EVENT_VCONN_ACTIVE_TIMEOUT = 106, - TS_EVENT_VCONN_START = 107, - TS_EVENT_VCONN_CLOSE = 108, - TS_EVENT_VCONN_OUTBOUND_START = 109, - TS_EVENT_VCONN_OUTBOUND_CLOSE = 110, - TS_EVENT_VCONN_PRE_ACCEPT = TS_EVENT_VCONN_START, // Deprecated but still compatible - - TS_EVENT_NET_CONNECT = 200, - TS_EVENT_NET_CONNECT_FAILED = 201, - TS_EVENT_NET_ACCEPT = 202, - TS_EVENT_NET_ACCEPT_FAILED = 204, - - TS_EVENT_INTERNAL_206 = 206, - TS_EVENT_INTERNAL_207 = 207, - TS_EVENT_INTERNAL_208 = 208, - TS_EVENT_INTERNAL_209 = 209, - TS_EVENT_INTERNAL_210 = 210, - TS_EVENT_INTERNAL_211 = 211, - TS_EVENT_INTERNAL_212 = 212, - - TS_EVENT_HOST_LOOKUP = 500, - - TS_EVENT_CACHE_OPEN_READ = 1102, - TS_EVENT_CACHE_OPEN_READ_FAILED = 1103, - TS_EVENT_CACHE_OPEN_WRITE = 1108, - TS_EVENT_CACHE_OPEN_WRITE_FAILED = 1109, - TS_EVENT_CACHE_REMOVE = 1112, - TS_EVENT_CACHE_REMOVE_FAILED = 1113, - TS_EVENT_CACHE_SCAN = 1120, - TS_EVENT_CACHE_SCAN_FAILED = 1121, - TS_EVENT_CACHE_SCAN_OBJECT = 1122, - TS_EVENT_CACHE_SCAN_OPERATION_BLOCKED = 1123, - TS_EVENT_CACHE_SCAN_OPERATION_FAILED = 1124, - TS_EVENT_CACHE_SCAN_DONE = 1125, - TS_EVENT_CACHE_LOOKUP = 1126, - TS_EVENT_CACHE_READ = 1127, - TS_EVENT_CACHE_DELETE = 1128, - TS_EVENT_CACHE_WRITE = 1129, - TS_EVENT_CACHE_WRITE_HEADER = 1130, - TS_EVENT_CACHE_CLOSE = 1131, - TS_EVENT_CACHE_LOOKUP_READY = 1132, - TS_EVENT_CACHE_LOOKUP_COMPLETE = 1133, - TS_EVENT_CACHE_READ_READY = 1134, - TS_EVENT_CACHE_READ_COMPLETE = 1135, - - TS_EVENT_INTERNAL_1200 = 1200, - - TS_EVENT_SSL_SESSION_GET = 2000, - TS_EVENT_SSL_SESSION_NEW = 2001, - TS_EVENT_SSL_SESSION_REMOVE = 2002, - - TS_EVENT_AIO_DONE = 3900, - - TS_EVENT_HTTP_CONTINUE = 60000, - TS_EVENT_HTTP_ERROR = 60001, - TS_EVENT_HTTP_READ_REQUEST_HDR = 60002, - TS_EVENT_HTTP_OS_DNS = 60003, - TS_EVENT_HTTP_SEND_REQUEST_HDR = 60004, - TS_EVENT_HTTP_READ_CACHE_HDR = 60005, - TS_EVENT_HTTP_READ_RESPONSE_HDR = 60006, - TS_EVENT_HTTP_SEND_RESPONSE_HDR = 60007, - TS_EVENT_HTTP_REQUEST_TRANSFORM = 60008, - TS_EVENT_HTTP_RESPONSE_TRANSFORM = 60009, - TS_EVENT_HTTP_SELECT_ALT = 60010, - TS_EVENT_HTTP_TXN_START = 60011, - TS_EVENT_HTTP_TXN_CLOSE = 60012, - TS_EVENT_HTTP_SSN_START = 60013, - TS_EVENT_HTTP_SSN_CLOSE = 60014, - TS_EVENT_HTTP_CACHE_LOOKUP_COMPLETE = 60015, - TS_EVENT_HTTP_PRE_REMAP = 60016, - TS_EVENT_HTTP_POST_REMAP = 60017, - TS_EVENT_HTTP_REQUEST_BUFFER_COMPLETE = 60018, - - TS_EVENT_LIFECYCLE_PORTS_INITIALIZED = 60100, - TS_EVENT_LIFECYCLE_PORTS_READY = 60101, - TS_EVENT_LIFECYCLE_CACHE_READY = 60102, - TS_EVENT_LIFECYCLE_SERVER_SSL_CTX_INITIALIZED = 60103, - TS_EVENT_LIFECYCLE_CLIENT_SSL_CTX_INITIALIZED = 60104, - TS_EVENT_LIFECYCLE_MSG = 60105, - TS_EVENT_LIFECYCLE_TASK_THREADS_READY = 60106, - TS_EVENT_LIFECYCLE_SHUTDOWN = 60107, - - TS_EVENT_INTERNAL_60200 = 60200, - TS_EVENT_INTERNAL_60201 = 60201, - TS_EVENT_INTERNAL_60202 = 60202, - TS_EVENT_SSL_CERT = 60203, - TS_EVENT_SSL_SERVERNAME = 60204, - TS_EVENT_SSL_VERIFY_SERVER = 60205, - TS_EVENT_SSL_VERIFY_CLIENT = 60206, - TS_EVENT_SSL_CLIENT_HELLO = 60207, - TS_EVENT_SSL_SECRET = 60208, - - TS_EVENT_MGMT_UPDATE = 60300 -} TSEvent; -#define TS_EVENT_HTTP_READ_REQUEST_PRE_REMAP TS_EVENT_HTTP_PRE_REMAP /* backwards compat */ - typedef enum { TS_SRVSTATE_STATE_UNDEFINED = 0, TS_SRVSTATE_ACTIVE_TIMEOUT, diff --git a/proxy/ProxySession.cc b/proxy/ProxySession.cc index f53d66b0daf..332b468ce9c 100644 --- a/proxy/ProxySession.cc +++ b/proxy/ProxySession.cc @@ -77,7 +77,7 @@ static const TSEvent eventmap[TS_HTTP_LAST_HOOK + 1] = { TS_EVENT_HTTP_CACHE_LOOKUP_COMPLETE, // TS_HTTP_CACHE_LOOKUP_COMPLETE_HOOK TS_EVENT_HTTP_PRE_REMAP, // TS_HTTP_PRE_REMAP_HOOK TS_EVENT_HTTP_POST_REMAP, // TS_HTTP_POST_REMAP_HOOK - TS_EVENT_NONE, // TS_HTTP_RESPONSE_CLIENT_HOOK + TS_EVENT_HTTP_RESPONSE_CLIENT, // TS_HTTP_RESPONSE_CLIENT_HOOK TS_EVENT_NONE, // TS_HTTP_LAST_HOOK }; diff --git a/proxy/http/HttpDebugNames.cc b/proxy/http/HttpDebugNames.cc index 2eb03f8e8f9..3138913308c 100644 --- a/proxy/http/HttpDebugNames.cc +++ b/proxy/http/HttpDebugNames.cc @@ -338,8 +338,8 @@ HttpDebugNames::get_event_name(int event) return "TS_EVENT_VCONN_CLOSE"; case TS_EVENT_LIFECYCLE_MSG: return "TS_EVENT_LIFECYCLE_MSG"; - case TS_EVENT_HTTP_REQUEST_BUFFER_COMPLETE: - return "TS_EVENT_HTTP_REQUEST_BUFFER_COMPLETE"; + case TS_EVENT_HTTP_REQUEST_BUFFER_READ_COMPLETE: + return "TS_EVENT_HTTP_REQUEST_BUFFER_READ_COMPLETE"; case TS_EVENT_MGMT_UPDATE: return "TS_EVENT_MGMT_UPDATE"; case TS_EVENT_INTERNAL_60200: diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc index ea4794fbd5a..91ab18df35f 100644 --- a/proxy/http/HttpSM.cc +++ b/proxy/http/HttpSM.cc @@ -1662,7 +1662,7 @@ plugins required to work with sni_routing. // void HttpSM::handle_api_return() // // Figures out what to do after calling api callouts -// have finished. This messy and I would like +// have finished. This is messy and I would like // to come up with a cleaner way to handle the api // return. The way we are doing things also makes a // mess of set_next_state() @@ -3713,7 +3713,14 @@ HttpSM::tunnel_handler_post_server(int event, HttpTunnelConsumer *c) { STATE_ENTER(&HttpSM::tunnel_handler_post_server, event); - server_request_body_bytes = c->bytes_written; + // If is_using_post_buffer has been used, then this handler gets called + // twice, once with the buffered request body bytes and a second time with + // the (now) zero length user agent buffer. See wait_for_full_body where + // these bytes are read. Don't clobber the server_request_body_bytes with + // zero on that second read. + if (server_request_body_bytes == 0) { + server_request_body_bytes = c->bytes_written; + } switch (event) { case VC_EVENT_EOS: @@ -5915,10 +5922,17 @@ HttpSM::do_setup_post_tunnel(HttpVC_t to_vc_type) // Next order of business if copy the remaining data from the // header buffer into new buffer - client_request_body_bytes = + int64_t num_body_bytes = post_buffer->write(ua_txn->get_remote_reader(), chunked ? ua_txn->get_remote_reader()->read_avail() : post_bytes); - ua_txn->get_remote_reader()->consume(client_request_body_bytes); + // If is_using_post_buffer has been used, then client_request_body_bytes + // will have already been set in wait_for_full_body and there will be + // zero bytes in this user agent buffer. We don't want to clobber + // client_request_body_bytes with a zero value here in those cases. + if (client_request_body_bytes == 0) { + client_request_body_bytes = num_body_bytes; + } + ua_txn->get_remote_reader()->consume(num_body_bytes); p = tunnel.add_producer(ua_entry->vc, post_bytes - transfered_bytes, buf_start, &HttpSM::tunnel_handler_post_ua, HT_HTTP_CLIENT, "user agent post"); } diff --git a/src/traffic_server/InkAPITest.cc b/src/traffic_server/InkAPITest.cc index af0374eca9f..733ddafc78b 100644 --- a/src/traffic_server/InkAPITest.cc +++ b/src/traffic_server/InkAPITest.cc @@ -6636,8 +6636,9 @@ enum ORIG_TSHttpHookID { ORIG_TS_HTTP_CACHE_LOOKUP_COMPLETE_HOOK, ORIG_TS_HTTP_PRE_REMAP_HOOK, ORIG_TS_HTTP_POST_REMAP_HOOK, + ORIG_TS_HTTP_REQUEST_BUFFER_READ_COMPLETE_HOOK, ORIG_TS_HTTP_RESPONSE_CLIENT_HOOK, - ORIG_TS_SSL_FIRST_HOOK, + ORIG_TS_SSL_FIRST_HOOK = 201, ORIG_TS_VCONN_START_HOOK = ORIG_TS_SSL_FIRST_HOOK, ORIG_TS_VCONN_CLOSE_HOOK, ORIG_TS_SSL_CLIENT_HELLO_HOOK, @@ -6649,7 +6650,6 @@ enum ORIG_TSHttpHookID { ORIG_TS_VCONN_OUTBOUND_START_HOOK, ORIG_TS_VCONN_OUTBOUND_CLOSE_HOOK, ORIG_TS_SSL_LAST_HOOK = ORIG_TS_VCONN_OUTBOUND_CLOSE_HOOK, - ORIG_TS_HTTP_REQUEST_BUFFER_READ_COMPLETE_HOOK, ORIG_TS_HTTP_LAST_HOOK }; diff --git a/tests/gold_tests/pluginTest/test_hooks/200.gold b/tests/gold_tests/pluginTest/test_hooks/200.gold new file mode 100644 index 00000000000..a8875735ebb --- /dev/null +++ b/tests/gold_tests/pluginTest/test_hooks/200.gold @@ -0,0 +1,28 @@ +`` +> POST /contentlength HTTP/1.1`` +> Host:`` +> User-Agent: curl/`` +> Accept: */*`` +> Content-Length: 22`` +> Content-Type: application/`` +`` +< HTTP/1.1 200 OK`` +< Server: ATS/`` +< Content-Length: 23`` +< Date:`` +< Age:`` +`` +> POST /chunked`` +> Host:`` +> User-Agent: curl/`` +> Accept: */*`` +> Transfer-Encoding: chunked`` +> Content-Type: application/`` +`` +< HTTP/1.1 200 OK`` +< Server: ATS/`` +< Date: `` +< Age: 0`` +< Transfer-Encoding: chunked`` +< Connection: keep-alive`` +`` diff --git a/tests/gold_tests/pluginTest/test_hooks/body_buffer.test.py b/tests/gold_tests/pluginTest/test_hooks/body_buffer.test.py new file mode 100644 index 00000000000..674274fb0a8 --- /dev/null +++ b/tests/gold_tests/pluginTest/test_hooks/body_buffer.test.py @@ -0,0 +1,121 @@ +''' +Verify HTTP body buffering. +''' +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os + + +def int_to_hex_string(int_value): + ''' + Convert the given int value to a hex string with no '0x' prefix. + + >>> int_to_hex_string(0) + '0' + >>> int_to_hex_string(1) + '1' + >>> int_to_hex_string(10) + 'a' + >>> int_to_hex_string(16) + '10' + >>> int_to_hex_string(17) + 'f1' + ''' + if not isinstance(int_value, int): + raise ValueError("Input should be an int type.") + return hex(int_value).split('x')[1] + + +class BodyBufferTest: + def __init__(cls, description): + Test.Summary = description + cls._origin_max_connections = 3 + cls.setupOriginServer() + cls.setupTS() + + def setupOriginServer(self): + self._server = Test.MakeOriginServer("server") + self.content_length_request_body = "content-length request" + self.content_length_size = len(self.content_length_request_body) + request_header = {"headers": "POST /contentlength HTTP/1.1\r\n" + "Host: www.example.com\r\n" + f"Content-Length: {self.content_length_size}\r\n\r\n", + "timestamp": "1469733493.993", + "body": self.content_length_request_body} + content_length_response_body = "content-length response" + content_length_response_size = len(content_length_response_body) + response_header = {"headers": "HTTP/1.1 200 OK\r\n" + "Server: microserver\r\n" + f"Content-Length: {content_length_response_size}\r\n\r\n" + "Connection: close\r\n\r\n", + "timestamp": "1469733493.993", + "body": content_length_response_body} + self._server.addResponse("sessionlog.json", request_header, response_header) + + self.chunked_request_body = "chunked request" + hex_size = int_to_hex_string(len(self.chunked_request_body)) + self.encoded_chunked_request = f"{hex_size}\r\n{self.chunked_request_body}\r\n0\r\n\r\n" + self.encoded_chunked_size = len(self.content_length_request_body) + request_header2 = {"headers": "POST /chunked HTTP/1.1\r\n" + "Transfer-Encoding: chunked\r\n" + "Host: www.example.com\r\n" + "Connection: keep-alive\r\n\r\n", + "timestamp": "1469733493.993", + "body": self.encoded_chunked_request} + self.chunked_response_body = "chunked response" + hex_size = int_to_hex_string(len(self.chunked_response_body)) + self.encoded_chunked_response = f"{hex_size}\r\n{self.chunked_response_body}\r\n0\r\n\r\n" + response_header2 = {"headers": "HTTP/1.1 200 OK\r\n" + "Transfer-Encoding: chunked\r\n" + "Server: microserver\r\n" + "Connection: close\r\n\r\n", + "timestamp": "1469733493.993", + "body": self.encoded_chunked_response} + self._server.addResponse("sessionlog.json", request_header2, response_header2) + + def setupTS(self): + self._ts = Test.MakeATSProcess("ts", select_ports=False) + self._ts.Disk.remap_config.AddLine( + f'map / http://127.0.0.1:{self._server.Variables.Port}' + ) + Test.PrepareInstalledPlugin('request_buffer.so', self._ts) + self._ts.Disk.records_config.update({ + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'request_buffer', + }) + + self._ts.Streams.stderr = Testers.ContainsExpression( + rf"request_buffer_plugin gets the request body with length\[{self.content_length_size}\]", + "Verify that the plugin parsed the content-length request body data.") + self._ts.Streams.stderr += Testers.ContainsExpression( + rf"request_buffer_plugin gets the request body with length\[{self.encoded_chunked_size}\]", + "Verify that the plugin parsed the chunked request body.") + + def run(self): + tr = Test.AddTestRun() + # Send both a Content-Length request and a chunked-encoded request. + tr.Processes.Default.Command = ( + f'curl -v http://127.0.0.1:{self._ts.Variables.port}/contentlength -d "{self.content_length_request_body}" --next ' + f'-v http://127.0.0.1:{self._ts.Variables.port}/chunked -H "Transfer-Encoding: chunked" -d "{self.chunked_request_body}"') + tr.Processes.Default.ReturnCode = 0 + tr.Processes.Default.StartBefore(self._server) + tr.Processes.Default.StartBefore(Test.Processes.ts) + tr.Processes.Default.Streams.stderr = "200.gold" + + +bodyBufferTest = BodyBufferTest("Test request body buffering.") +bodyBufferTest.run()