diff --git a/CMakeLists.txt b/CMakeLists.txt index 8a829a8d9ac..b517fcf7105 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -304,7 +304,10 @@ else() endif() check_openssl_is_quictls(SSLLIB_IS_QUICTLS "${OPENSSL_INCLUDE_DIR}") -if(OPENSSL_VERSION VERSION_GREATER_EQUAL "3.0.0") +if(NOT SSLLIB_IS_BORINGSSL + AND NOT SSLLIB_IS_AWSLC + AND OPENSSL_VERSION VERSION_GREATER_EQUAL "3.0.0" +) set(SSLLIB_IS_OPENSSL3 TRUE) add_compile_definitions(OPENSSL_API_COMPAT=10002 OPENSSL_IS_OPENSSL3) endif() diff --git a/CMakePresets.json b/CMakePresets.json index 2bf61df3969..18676c1fec3 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -176,10 +176,12 @@ "inherits": ["ci"], "cacheVariables": { "ENABLE_PROBES": "ON", - "OPENSSL_ROOT_DIR": "/opt/openssl-quic", + "OPENSSL_ROOT_DIR": "/usr", "opentelemetry_ROOT": "/opt", - "CURL_ROOT": "/opt", + "CURL_ROOT": "/usr", "wamr_ROOT": "/opt", + "ENABLE_OTEL_TRACER": "ON", + "ENABLE_WASM_WAMR": "ON", "ENABLE_CRIPTS": "ON" } }, @@ -201,15 +203,17 @@ "OPENSSL_ROOT_DIR": "/opt/h3-tools-boringssl/boringssl", "quiche_ROOT": "/opt/h3-tools-boringssl/quiche", "opentelemetry_ROOT": "/opt", - "CURL_ROOT": "/opt", + "CURL_ROOT": "/opt/h3-tools-boringssl", "wamr_ROOT": "/opt", + "ENABLE_OTEL_TRACER": "ON", + "ENABLE_WASM_WAMR": "ON", "CMAKE_INSTALL_PREFIX": "/tmp/ats-quiche", "ENABLE_QUICHE": "ON" } }, { "name": "ci-fedora-autest", - "displayName": "CI Fedora Quiche Autest", + "displayName": "CI Fedora Autest", "description": "CI Pipeline config for Fedora Linux (autest build)", "inherits": ["ci-fedora", "autest"] }, @@ -250,6 +254,8 @@ "description": "CI Pipeline config for running clang-analyzer", "inherits": ["ci"], "cacheVariables": { + "CMAKE_C_COMPILER": "clang", + "CMAKE_CXX_COMPILER": "clang++", "CMAKE_EXPORT_COMPILE_COMMANDS": "ON", "ENABLE_CCACHE": "OFF", "ENABLE_EXAMPLE": "OFF", diff --git a/plugins/experimental/memcache/tsmemcache.cc b/plugins/experimental/memcache/tsmemcache.cc index cce2ab829c5..2d33cdde760 100644 --- a/plugins/experimental/memcache/tsmemcache.cc +++ b/plugins/experimental/memcache/tsmemcache.cc @@ -249,7 +249,7 @@ MC::add_binary_header(uint16_t err, uint8_t hdr_len, uint16_t key_len, uint32_t int MC::write_binary_error(protocol_binary_response_status err, int swallow) { - const char *errstr = "Unknown error"; + const char *errstr{nullptr}; switch (err) { case PROTOCOL_BINARY_RESPONSE_ENOMEM: errstr = "Out of memory"; diff --git a/plugins/experimental/wasm/CMakeLists.txt b/plugins/experimental/wasm/CMakeLists.txt index 0cccbc2082c..c10011e5fb4 100644 --- a/plugins/experimental/wasm/CMakeLists.txt +++ b/plugins/experimental/wasm/CMakeLists.txt @@ -29,7 +29,7 @@ if(wasmedge_FOUND) list(APPEND WASM_RUNTIME wasmedge::wasmedge) endif() -target_link_libraries(wasm PRIVATE ${WASM_RUNTIME}) +target_link_libraries(wasm PRIVATE OpenSSL::SSL ${WASM_RUNTIME}) if(wamr_FOUND) target_compile_options(wasm PRIVATE -DWAMR) diff --git a/plugins/experimental/wasm/lib/CMakeLists.txt b/plugins/experimental/wasm/lib/CMakeLists.txt index f838461d7bf..1a07cd3a882 100644 --- a/plugins/experimental/wasm/lib/CMakeLists.txt +++ b/plugins/experimental/wasm/lib/CMakeLists.txt @@ -38,6 +38,7 @@ endif() add_library(wasmlib STATIC ${CC_FILES}) target_compile_options(wasmlib PUBLIC -Wno-unused-parameter) +target_link_libraries(wasmlib PRIVATE OpenSSL::Crypto) if(wamr_FOUND) target_compile_options(wasmlib PRIVATE -Wno-missing-field-initializers) target_link_libraries(wasmlib PUBLIC wamr::wamr) diff --git a/plugins/healthchecks/healthchecks.cc b/plugins/healthchecks/healthchecks.cc index f31a43d0cc1..ece4319aee2 100644 --- a/plugins/healthchecks/healthchecks.cc +++ b/plugins/healthchecks/healthchecks.cc @@ -28,6 +28,8 @@ limitations under the License. #include #include #include +#include +#include /* ToDo: Linux specific */ #include @@ -42,9 +44,8 @@ static const char SEPARATORS[] = " \t\n"; static DbgCtl dbg_ctl{PLUGIN_NAME}; -#define MAX_PATH_LEN 4096 -#define MAX_BODY_LEN 16384 -#define FREELIST_TIMEOUT 300 +#define MAX_PATH_LEN 4096 +#define MAX_BODY_LEN 16384 /* Directories that we are watching for inotify IN_CREATE events. */ typedef struct HCDirEntry_t { @@ -53,66 +54,100 @@ typedef struct HCDirEntry_t { struct HCDirEntry_t *_next; /* Linked list */ } HCDirEntry; -/* Information about a status file. This is never modified (only replaced, see HCFileInfo_t) */ -typedef struct HCFileData_t { - int exists; /* Does this file exist */ - char body[MAX_BODY_LEN]; /* Body from fname. Empty string means file is missing */ - int b_len; /* Length of data */ - time_t remove; /* Used for deciding when the old object can be permanently removed */ - struct HCFileData_t *_next; /* Only used when these guys end up on the freelist */ -} HCFileData; - -/* The only thing that should change in this struct is data, atomically swapping ptrs */ -typedef struct HCFileInfo_t { - char fname[MAX_PATH_LEN]; /* Filename */ - char *basename; /* The "basename" of the file */ - unsigned basename_len = 0; /* The length of the basename */ - char path[PATH_NAME_MAX]; /* URL path for this HC */ - int p_len; /* Length of path */ - const char *ok; /* Header for an OK result */ - int o_len; /* Length of OK header */ - const char *miss; /* Header for miss results */ - int m_len; /* Length of miss header */ - std::atomic data; /* Holds the current data for this health check file */ - int wd; /* Watch descriptor */ - HCDirEntry *dir; /* Reference to the directory this file resides in */ - struct HCFileInfo_t *_next; /* Linked list */ -} HCFileInfo; +/* Information about a status file. This is never modified (only replaced, see HCFileInfo) */ +struct HCFileData { + int exists = 0; /* Does this file exist */ + int b_len = 0; /* Length of data */ + char body[MAX_BODY_LEN] = {}; /* Body from fname. Empty string means file is missing */ +}; + +using HCFileDataPtr = std::shared_ptr; + +/* The only thing that should change in this struct is data, which is replaced (never modified) by + the inotify thread. Readers take a reference to the current data via get_data(), which keeps + that snapshot alive for as long as the transaction needs it. */ +struct HCFileInfo { + char fname[MAX_PATH_LEN] = {}; /* Filename */ + char *basename = nullptr; /* The "basename" of the file */ + unsigned basename_len = 0; /* The length of the basename */ + char path[PATH_NAME_MAX] = {}; /* URL path for this HC */ + int p_len = 0; /* Length of path */ + const char *ok = nullptr; /* Header for an OK result */ + int o_len = 0; /* Length of OK header */ + const char *miss = nullptr; /* Header for miss results */ + int m_len = 0; /* Length of miss header */ + int wd = 0; /* Watch descriptor */ + HCDirEntry *dir = nullptr; /* Reference to the directory this file resides in */ + HCFileInfo *_next = nullptr; /* Linked list */ + + /* Take a reference to the current data for this health check file. */ + HCFileDataPtr + get_data() + { +#if defined(__cpp_lib_atomic_shared_ptr) && __cpp_lib_atomic_shared_ptr >= 201711L + return _data.load(std::memory_order_acquire); +#else + return std::atomic_load_explicit(&_data, std::memory_order_acquire); +#endif + } + + /* Replace the current data for this health check file. Snapshots handed out by get_data() stay + valid until their last reference is dropped. */ + void + set_data(HCFileDataPtr data) + { +#if defined(__cpp_lib_atomic_shared_ptr) && __cpp_lib_atomic_shared_ptr >= 201711L + _data.store(std::move(data), std::memory_order_release); +#else + std::atomic_store_explicit(&_data, std::move(data), std::memory_order_release); +#endif + } + +private: +#if defined(__cpp_lib_atomic_shared_ptr) && __cpp_lib_atomic_shared_ptr >= 201711L + std::atomic _data; /* Holds the current data for this health check file */ +#else + HCFileDataPtr _data; /* Holds the current data for this health check file */ +#endif +}; /* Global configuration */ HCFileInfo *g_config; /* State used for the intercept plugin. ToDo: Can this be improved ? */ -typedef struct HCState_t { - TSVConn net_vc; - TSVIO read_vio; - TSVIO write_vio; +struct HCState { + TSVConn net_vc = nullptr; + TSVIO read_vio = nullptr; + TSVIO write_vio = nullptr; - TSIOBuffer req_buffer; - TSIOBuffer resp_buffer; - TSIOBufferReader resp_reader; + TSIOBuffer req_buffer = nullptr; + TSIOBuffer resp_buffer = nullptr; + TSIOBufferReader resp_reader = nullptr; - int output_bytes; + int output_bytes = 0; - /* We actually need both here, so that our lock free switches works safely */ - HCFileInfo *info; - HCFileData *data; -} HCState; + /* We hold a reference to the data so that it cannot be replaced from under us mid transaction */ + HCFileInfo *info = nullptr; + HCFileDataPtr data; +}; /* Read / check the status files */ -static void -reload_status_file(HCFileInfo *info, HCFileData *data) +static HCFileDataPtr +load_status_file(HCFileInfo *info) { + auto data = std::make_shared(); FILE *fd; - memset(data, 0, sizeof(HCFileData)); if (nullptr != (fd = fopen(info->fname, "r"))) { data->exists = 1; - do { - data->b_len = fread(data->body, 1, MAX_BODY_LEN, fd); - } while (!feof(fd)); /* Only save the last 16KB of the file ... */ + data->b_len = static_cast(fread(data->body, 1, MAX_BODY_LEN, fd)); + if (ferror(fd)) { + data->b_len = 0; + } fclose(fd); } + + return data; } /* Find a HCDirEntry from the linked list */ @@ -198,49 +233,16 @@ event_matches_config(struct inotify_event *event, HCFileInfo *finfo) static void * hc_thread(void *data ATS_UNUSED) { - int inotify_fd = inotify_init(); - HCFileData *fl_head = nullptr; - char buffer[INOTIFY_BUFLEN]; - struct timeval last_free, now; - - gettimeofday(&last_free, nullptr); + int inotify_fd = inotify_init(); + char buffer[INOTIFY_BUFLEN]; /* Setup watchers for the directories, these are a one time setup */ setup_watchers(inotify_fd); // This is a leak, but since we enter an infinite loop this is ok? while (true) { - HCFileData *fdata = fl_head, *fdata_prev = nullptr; - - gettimeofday(&now, nullptr); /* Read the inotify events, blocking until we get something */ int len = read(inotify_fd, buffer, INOTIFY_BUFLEN); - /* The fl_head is a linked list of previously released data entries. They - are ordered "by time", so once we find one that is scheduled for deletion, - we can also delete all entries after it in the linked list. */ - while (fdata) { - if (now.tv_sec > fdata->remove) { - /* Now drop off the "tail" from the freelist */ - if (fdata_prev) { - fdata_prev->_next = nullptr; - } else { - fl_head = nullptr; - } - - /* free() everything in the "tail" */ - do { - HCFileData *next = fdata->_next; - - Dbg(dbg_ctl, "Cleaning up entry from freelist"); - TSfree(fdata); - fdata = next; - } while (fdata); - break; /* Stop the loop, there's nothing else left to examine */ - } - fdata_prev = fdata; - fdata = fdata->_next; - } - if (len >= 0) { int i = 0; @@ -253,9 +255,6 @@ hc_thread(void *data ATS_UNUSED) finfo = finfo->_next; } if (finfo) { - auto *new_data = TSRalloc(); - HCFileData *old_data; - if (event->mask & (IN_CLOSE_WRITE | IN_ATTRIB)) { Dbg(dbg_ctl, "Modify file event (%d) on %s", event->mask, finfo->fname); } else if (event->mask & (IN_CREATE | IN_MOVED_TO)) { @@ -267,16 +266,12 @@ hc_thread(void *data ATS_UNUSED) } else { Dbg(dbg_ctl, "Unhandled event (%d) on %s", event->mask, finfo->fname); } - /* Load the new data and then swap this atomically */ - memset(new_data, 0, sizeof(HCFileData)); - reload_status_file(finfo, new_data); - Dbg(dbg_ctl, "Reloaded %s, len == %d, exists == %d", finfo->fname, new_data->b_len, new_data->exists); - old_data = finfo->data.exchange(new_data); + /* Load the new data and then publish it. The previous data is released once the last + transaction referencing it completes. */ + auto new_data = load_status_file(finfo); - /* Add the old data to the head of the freelist */ - old_data->remove = now.tv_sec + FREELIST_TIMEOUT; - old_data->_next = fl_head; - fl_head = old_data; + Dbg(dbg_ctl, "Reloaded %s, len == %d, exists == %d", finfo->fname, new_data->b_len, new_data->exists); + finfo->set_data(std::move(new_data)); } /* coverity[ -tainted_data_return] */ i += sizeof(struct inotify_event) + event->len; @@ -338,71 +333,66 @@ parse_configs(const char *fname) return nullptr; } - while (!feof(fd)) { + while (fgets(buf, sizeof(buf) - 1, fd) != nullptr) { char *str, *save; char *ok = nullptr, *miss = nullptr, *mime = nullptr; - finfo = TSRalloc(); - memset(static_cast(finfo), 0, sizeof(HCFileInfo)); - - if (fgets(buf, sizeof(buf) - 1, fd)) { - str = strtok_r(buf, SEPARATORS, &save); - int state = 0; - while (nullptr != str) { - if (strlen(str) > 0) { - switch (state) { - case 0: - if ('/' == *str) { - ++str; - } - strncpy(finfo->path, str, PATH_NAME_MAX - 1); - finfo->path[PATH_NAME_MAX - 1] = '\0'; - finfo->p_len = strlen(finfo->path); - break; - case 1: - strncpy(finfo->fname, str, MAX_PATH_LEN - 1); - finfo->fname[MAX_PATH_LEN - 1] = '\0'; - finfo->basename = strrchr(finfo->fname, '/'); - if (finfo->basename) { - ++(finfo->basename); - finfo->basename_len = strlen(finfo->basename); - } - break; - case 2: - mime = str; - break; - case 3: - ok = str; - break; - case 4: - miss = str; - break; + finfo = new HCFileInfo(); + + str = strtok_r(buf, SEPARATORS, &save); + int state = 0; + while (nullptr != str) { + if (strlen(str) > 0) { + switch (state) { + case 0: + if ('/' == *str) { + ++str; + } + strncpy(finfo->path, str, PATH_NAME_MAX - 1); + finfo->path[PATH_NAME_MAX - 1] = '\0'; + finfo->p_len = strlen(finfo->path); + break; + case 1: + strncpy(finfo->fname, str, MAX_PATH_LEN - 1); + finfo->fname[MAX_PATH_LEN - 1] = '\0'; + finfo->basename = strrchr(finfo->fname, '/'); + if (finfo->basename) { + ++(finfo->basename); + finfo->basename_len = strlen(finfo->basename); } - ++state; + break; + case 2: + mime = str; + break; + case 3: + ok = str; + break; + case 4: + miss = str; + break; } - str = strtok_r(nullptr, SEPARATORS, &save); + ++state; } + str = strtok_r(nullptr, SEPARATORS, &save); + } - /* Fill in the info if everything was ok */ - if (state > 4) { - Dbg(dbg_ctl, "Parsed: %s %s %s %s %s", finfo->path, finfo->fname, mime, ok, miss); - finfo->ok = gen_header(ok, mime, &finfo->o_len); - finfo->miss = gen_header(miss, mime, &finfo->m_len); - finfo->data = TSRalloc(); - memset(finfo->data, 0, sizeof(HCFileData)); - reload_status_file(finfo, finfo->data); - - /* Add it the linked list */ - Dbg(dbg_ctl, "Adding path=%s to linked list", finfo->path); - if (nullptr == head_finfo) { - head_finfo = finfo; - } else { - prev_finfo->_next = finfo; - } - prev_finfo = finfo; + /* Fill in the info if everything was ok */ + if (state > 4) { + Dbg(dbg_ctl, "Parsed: %s %s %s %s %s", finfo->path, finfo->fname, mime, ok, miss); + finfo->ok = gen_header(ok, mime, &finfo->o_len); + finfo->miss = gen_header(miss, mime, &finfo->m_len); + finfo->set_data(load_status_file(finfo)); + + /* Add it the linked list */ + Dbg(dbg_ctl, "Adding path=%s to linked list", finfo->path); + if (nullptr == head_finfo) { + head_finfo = finfo; } else { - TSfree(finfo); + prev_finfo->_next = finfo; } + prev_finfo = finfo; + } else { + delete finfo; } } fclose(fd); @@ -434,7 +424,7 @@ cleanup(TSCont contp, HCState *my_state) my_state->net_vc = nullptr; } - TSfree(my_state); + delete my_state; TSContDestroy(contp); } @@ -567,11 +557,10 @@ health_check_origin(TSCont contp ATS_UNUSED, TSEvent event ATS_UNUSED, void *eda TSHttpTxnCntlSet(txnp, TS_HTTP_CNTL_SKIP_REMAPPING, true); /* not strictly necessary, but speed is everything these days */ /* This is us -- register our intercept */ - icontp = TSContCreate(hc_intercept, TSMutexCreate()); - my_state = TSRalloc(); - memset(my_state, 0, sizeof(*my_state)); + icontp = TSContCreate(hc_intercept, TSMutexCreate()); + my_state = new HCState(); my_state->info = info; - my_state->data = info->data; + my_state->data = info->get_data(); TSContDataSet(icontp, my_state); TSHttpTxnIntercept(icontp, txnp); } diff --git a/plugins/webp_transform/ImageTransform.cc b/plugins/webp_transform/ImageTransform.cc index b532bbd93f9..e4ebc7f4d7c 100644 --- a/plugins/webp_transform/ImageTransform.cc +++ b/plugins/webp_transform/ImageTransform.cc @@ -24,6 +24,7 @@ #include #include #include +#include #include "ts/ts.h" @@ -64,6 +65,26 @@ bool config_convert_to_jpeg = false; Stat stat_convert_to_webp; Stat stat_convert_to_jpeg; +bool +has_signature_for(std::string_view data, ImageEncoding encoding) +{ + constexpr std::string_view png_signature{"\x89PNG\r\n\x1a\n", 8}; + + switch (encoding) { + case ImageEncoding::webp: + return data.size() >= 12 && data.substr(0, 4) == "RIFF" && data.substr(8, 4) == "WEBP"; + case ImageEncoding::jpeg: + return data.size() >= 3 && static_cast(data[0]) == 0xff && static_cast(data[1]) == 0xd8 && + static_cast(data[2]) == 0xff; + case ImageEncoding::png: + return data.starts_with(png_signature); + case ImageEncoding::unknown: + return false; + } + + return false; +} + // Cap the buffered (encoded) response body. 16 MiB fits every // realistic image asset while keeping the worst case bounded. The default is // overridable with the max_buffer_size plugin argument (see TSPluginInit). @@ -159,8 +180,10 @@ content_type_for(ImageEncoding encoding) class ImageTransform : public TransformationPlugin { public: - ImageTransform(Transaction &transaction, ImageEncoding input_image_type, ImageEncoding transform_image_type) + ImageTransform(Transaction &transaction, std::string input_content_type, ImageEncoding input_image_type, + ImageEncoding transform_image_type) : TransformationPlugin(transaction, TransformationPlugin::RESPONSE_TRANSFORMATION), + _input_content_type(std::move(input_content_type)), _input_image_type(input_image_type), _transform_image_type(transform_image_type) { @@ -168,6 +191,24 @@ class ImageTransform : public TransformationPlugin TransformationPlugin::registerHook(HOOK_SEND_RESPONSE_HEADERS); } + void + handleReadResponseHeaders(Transaction &transaction) override + { + // Label the server response so both the cached transform and the client + // copy carry the target type. On a degraded transform (pass-through or + // decode error) the body is the original encoding but the label still says + // the target; handleSendResponseHeaders below corrects the client-facing + // copy using _input_content_type in that case. The cached label can still + // end up wrong on a degraded transform; fixing that without mislabeling + // the cache is tracked as a separate correctness issue. + if (const char *ctype = content_type_for(_transform_image_type); ctype != nullptr) { + transaction.getServerResponse().getHeaders()["Content-Type"] = ctype; + } + transaction.getServerResponse().getHeaders()["Vary"] = "Accept"; // separate cache entry per Accept + Dbg(webp_dbg_ctl, "url %s", transaction.getServerRequest().getUrl().getUrlString().c_str()); + transaction.resume(); + } + void handleSendResponseHeaders(Transaction &transaction) override { @@ -185,23 +226,16 @@ class ImageTransform : public TransformationPlugin // not happen; this is an empty error response, not an image. response.getHeaders().erase("Content-Type"); response.getHeaders().erase("Vary"); + transaction.resume(); + return; } - transaction.resume(); - } - void - handleReadResponseHeaders(Transaction &transaction) override - { - // Label the server response so both the cached transform and the client - // copy carry the target type. On a degraded transform (pass-through or - // decode error) the body is the original encoding but the label still says - // the target; correcting that without mislabeling the cache is tracked as a - // separate correctness issue, out of scope for this DoS fix. - if (const char *ctype = content_type_for(_transform_image_type); ctype != nullptr) { - transaction.getServerResponse().getHeaders()["Content-Type"] = ctype; + // Signature mismatch or decode failure reverted us to the original + // encoding (see pass_through()); relabel the client-facing response to + // match the body we actually sent. + if (_transform_image_type == _input_image_type) { + transaction.getClientResponse().getHeaders()["Content-Type"] = _input_content_type; } - transaction.getServerResponse().getHeaders()["Vary"] = "Accept"; // separate cache entry per Accept - Dbg(webp_dbg_ctl, "url %s", transaction.getServerRequest().getUrl().getUrlString().c_str()); transaction.resume(); } @@ -237,6 +271,15 @@ class ImageTransform : public TransformationPlugin setOutputComplete(); // no body produced; handleSendResponseHeaders turns this into a 502 return; } + + if (!has_signature_for(_img, _input_image_type)) { + TSError("[webp_transform] input body does not match its declared image encoding: %d, length: %zu", + static_cast(_input_image_type), _img.length()); + pass_through(_img); + setOutputComplete(); + return; + } + Blob input_blob(_img.data(), _img.length()); Image image; @@ -257,12 +300,10 @@ class ImageTransform : public TransformationPlugin produce(std::string_view(reinterpret_cast(output_blob.data()), output_blob.length())); } catch (const Magick::Warning &warning) { TSError("ImageMagick++ warning: %s", warning.what()); - produce(std::string_view(reinterpret_cast(input_blob.data()), input_blob.length())); - _transform_image_type = _input_image_type; // Revert to original encoding on error + pass_through(std::string_view(reinterpret_cast(input_blob.data()), input_blob.length())); } catch (const Magick::Error &error) { TSError("ImageMagick++ error: %s _image_type: %d input length: %zu", error.what(), (int)_transform_image_type, _img.length()); - produce(std::string_view(reinterpret_cast(input_blob.data()), input_blob.length())); - _transform_image_type = _input_image_type; // Revert to original encoding on error + pass_through(std::string_view(reinterpret_cast(input_blob.data()), input_blob.length())); } catch (const std::exception &e) { // ImageMagick++ can throw other exception types (e.g. // std::bad_alloc on huge or malformed inputs). Catch them so an @@ -271,13 +312,11 @@ class ImageTransform : public TransformationPlugin // large inputs) is distinguishable from a one-off decode hiccup. TSError("[webp_transform] std::exception during transform: %s _image_type: %d input length: %zu", e.what(), (int)_transform_image_type, _img.length()); - produce(std::string_view(reinterpret_cast(input_blob.data()), input_blob.length())); - _transform_image_type = _input_image_type; + pass_through(std::string_view(reinterpret_cast(input_blob.data()), input_blob.length())); } catch (...) { TSError("[webp_transform] unknown exception during transform _image_type: %d input length: %zu", (int)_transform_image_type, _img.length()); - produce(std::string_view(reinterpret_cast(input_blob.data()), input_blob.length())); - _transform_image_type = _input_image_type; + pass_through(std::string_view(reinterpret_cast(input_blob.data()), input_blob.length())); } setOutputComplete(); @@ -286,7 +325,17 @@ class ImageTransform : public TransformationPlugin ~ImageTransform() override = default; private: + void + pass_through(std::string_view data) + { + if (!data.empty()) { + produce(data); + } + _transform_image_type = _input_image_type; + } + std::string _img; + std::string _input_content_type; bool _refused = false; ImageEncoding _input_image_type; ImageEncoding _transform_image_type; @@ -385,13 +434,13 @@ class GlobalHookPlugin : public GlobalPlugin if (!content_length_usable) { TSHttpTxnServerRespNoStoreSet(static_cast(transaction.getAtsHandle()), 1); } - transaction.addPlugin(new ImageTransform(transaction, input_image_type, ImageEncoding::webp)); + transaction.addPlugin(new ImageTransform(transaction, ctype, input_image_type, ImageEncoding::webp)); } else if (webp_supported == false && transaction_convert_to_jpeg == true) { Dbg(webp_dbg_ctl, "Content type is webp. Converting to jpeg"); if (!content_length_usable) { TSHttpTxnServerRespNoStoreSet(static_cast(transaction.getAtsHandle()), 1); } - transaction.addPlugin(new ImageTransform(transaction, input_image_type, ImageEncoding::jpeg)); + transaction.addPlugin(new ImageTransform(transaction, ctype, input_image_type, ImageEncoding::jpeg)); } else { Dbg(webp_dbg_ctl, "Nothing to convert"); } diff --git a/src/iocore/cache/CacheProcessor.cc b/src/iocore/cache/CacheProcessor.cc index b1acbfc6e08..b27c176cb8f 100644 --- a/src/iocore/cache/CacheProcessor.cc +++ b/src/iocore/cache/CacheProcessor.cc @@ -53,9 +53,11 @@ #include #include #include +#include #include #include #include +#include static void CachePeriodicMetricsUpdate(); static int64_t cache_bytes_used(int index); @@ -980,10 +982,8 @@ cplist_reconfigure() // else the size is greater... /* search the cp_list */ - int *sorted_vols = new int[gndisks]; - for (int i = 0; i < gndisks; i++) { - sorted_vols[i] = i; - } + std::vector sorted_vols(gndisks); + std::iota(sorted_vols.begin(), sorted_vols.end(), 0); for (int i = 0; i < gndisks - 1; i++) { int smallest = sorted_vols[i]; int smallest_ndx = i; @@ -1043,8 +1043,6 @@ cplist_reconfigure() size_to_alloc = size_in_blocks - cp->size; } - delete[] sorted_vols; - if (size_to_alloc) { if (create_volume(volume_number, size_to_alloc, cp->scheme, cp)) { return -1; diff --git a/src/iocore/cache/CacheVC.cc b/src/iocore/cache/CacheVC.cc index 11fa8699732..7707d9d8bb2 100644 --- a/src/iocore/cache/CacheVC.cc +++ b/src/iocore/cache/CacheVC.cc @@ -215,6 +215,14 @@ CacheVC::do_io_write(Continuation *c, int64_t nbytes, IOBufferReader *abuf, bool #ifdef DEBUG ink_assert(!c || c->mutex->thread_holding); #endif + if (nbytes == 0) { + // A zero-byte write represents an empty document, while closing without a + // write represents a header-only update. + f.allow_empty_doc = 1; + if (alternate.valid()) { + alternate.object_size_set(0); + } + } if (c && !trigger && !recursive) { trigger = c->mutex->thread_holding->schedule_imm_local(this); } @@ -225,6 +233,14 @@ void CacheVC::do_io_close(int alerrno) { ink_assert(mutex->thread_holding == this_ethread()); + if (alerrno == -1 && vio.op == VIO::WRITE && vio.get_reader() != nullptr && vio.nbytes == 0) { + // The write may have started with an unknown length and been finalized + // at zero after the response framing was parsed. + f.allow_empty_doc = 1; + if (alternate.valid()) { + alternate.object_size_set(0); + } + } int previous_closed = closed; closed = (alerrno == -1) ? 1 : -1; // Stupid default arguments DDbg(dbg_ctl_cache_close, "do_io_close %p %d %d", this, alerrno, closed); @@ -1042,7 +1058,8 @@ CacheVC::set_http_info(CacheHTTPInfo *ainfo) } MIMEField *field = ainfo->m_alt->m_response_hdr.field_find(MIME_FIELD_CONTENT_LENGTH, MIME_LEN_CONTENT_LENGTH); - if ((field && !field->value_get_int64()) || ainfo->m_alt->m_response_hdr.status_get() == HTTP_STATUS_NO_CONTENT) { + if ((field && !field->value_get_int64()) || ainfo->m_alt->m_response_hdr.status_get() == HTTP_STATUS_NO_CONTENT || + (f.allow_empty_doc && vio.nbytes == 0)) { f.allow_empty_doc = 1; // Set the object size here to zero in case this is a cache replace where the new object // length is zero but the old object was not. diff --git a/src/iocore/cache/P_CacheInternal.h b/src/iocore/cache/P_CacheInternal.h index 34faafe64e2..d90cfad4bb5 100644 --- a/src/iocore/cache/P_CacheInternal.h +++ b/src/iocore/cache/P_CacheInternal.h @@ -300,6 +300,7 @@ CacheVC::die() { if (vio.op == VIO::WRITE) { if (f.update && total_len) { + ink_assert(alternate.valid()); alternate.object_key_set(earliest_key); } if (!is_io_in_progress()) { diff --git a/src/iocore/dns/DNS.cc b/src/iocore/dns/DNS.cc index aab3f4cd3e5..b1c8a68ad8a 100644 --- a/src/iocore/dns/DNS.cc +++ b/src/iocore/dns/DNS.cc @@ -1889,8 +1889,9 @@ dns_process(DNSHandler *handler, HostEnt *buf, int len) memcpy((*hap++ = bp), cp, n); Dbg(dbg_ctl_dns, "received %s = %s", QtypeName(type), inet_ntop(T_AAAA == type ? AF_INET6 : AF_INET, bp, ip_string, sizeof(ip_string))); - bp += n; - cp += n; + bp += n; + cp += n; + buflen = sizeof(buf->hostbuf) - (bp - buf->hostbuf); } } else { goto Lerror; diff --git a/src/iocore/hostdb/HostDB.cc b/src/iocore/hostdb/HostDB.cc index 8bf388f2a3b..11230a6982b 100644 --- a/src/iocore/hostdb/HostDB.cc +++ b/src/iocore/hostdb/HostDB.cc @@ -1042,10 +1042,14 @@ HostDBContinuation::dnsEvent(int event, HostEnt *e) ts::LocalBuffer q_buf(valid_records); SRV **q = q_buf.data(); ink_assert(valid_records <= static_cast(hostdb_round_robin_max_count)); + // The loop below assigns every element, but ts::LocalBuffer hands back raw storage and the + // static analyzer cannot follow the loop well enough to see that. Pre-fill so the sort below + // is never reported as reading an uninitialized pointer. + std::fill_n(q, valid_records, nullptr); for (int i = 0; i < valid_records; ++i) { q[i] = &e->srv_hosts.hosts[i]; } - std::sort(q, q + valid_records, [](SRV *lhs, SRV *rhs) -> bool { return *lhs < *rhs; }); + std::stable_sort(q, q + valid_records, [](SRV *lhs, SRV *rhs) -> bool { return *lhs < *rhs; }); SRV **cur_srv = q; for (auto &item : rr_info) { diff --git a/src/iocore/net/OCSPStapling.cc b/src/iocore/net/OCSPStapling.cc index 3e5631f9974..6d246eff6c7 100644 --- a/src/iocore/net/OCSPStapling.cc +++ b/src/iocore/net/OCSPStapling.cc @@ -21,6 +21,8 @@ #include "P_OCSPStapling.h" +#include + #include #include #include @@ -903,19 +905,22 @@ ssl_stapling_init_cert(SSL_CTX *ctx, X509 *cert, const char *certname, const cha Dbg(dbg_ctl_ssl_ocsp, "using OCSP prefetched response file %s", rsp_file); FILE *fp = fopen(rsp_file, "r"); if (fp) { - fseek(fp, 0, SEEK_END); - long rsp_buf_len = ftell(fp); - if (rsp_buf_len >= 0) { - rewind(fp); - unsigned char *rsp_buf = static_cast(malloc(rsp_buf_len)); - auto read_len = fread(rsp_buf, 1, rsp_buf_len, fp); + long rsp_buf_len = -1; + + if (fseek(fp, 0, SEEK_END) == 0) { + rsp_buf_len = ftell(fp); + } + + if (rsp_buf_len > 0 && fseek(fp, 0, SEEK_SET) == 0) { + std::vector rsp_buf(rsp_buf_len); + auto read_len = fread(rsp_buf.data(), 1, rsp_buf.size(), fp); + if (read_len == static_cast(rsp_buf_len)) { - const unsigned char *p = rsp_buf; + const unsigned char *p = rsp_buf.data(); rsp = d2i_TS_OCSP_RESPONSE(nullptr, &p, rsp_buf_len); } else { Error("stapling_refresh_response: failed to read prefetched response file: %s", rsp_file); } - free(rsp_buf); } else { Error("stapling_refresh_response: failed to check the size of prefetched response file: %s", rsp_file); } diff --git a/src/iocore/net/QUICNetVConnection.cc b/src/iocore/net/QUICNetVConnection.cc index d0850992f79..1b239eb9b78 100644 --- a/src/iocore/net/QUICNetVConnection.cc +++ b/src/iocore/net/QUICNetVConnection.cc @@ -328,7 +328,8 @@ QUICNetVConnection::acceptEvent(int event, Event *e) MUTEX_TRY_LOCK(lock, h->mutex, t); if (!lock.is_locked()) { - if (event == EVENT_NONE) { + // Direct calls can have no event; reschedule on the thread in that case. + if (event == EVENT_NONE || e == nullptr) { t->schedule_in(this, HRTIME_MSECONDS(net_retry_delay)); return EVENT_DONE; } else { diff --git a/src/iocore/net/UnixNetVConnection.cc b/src/iocore/net/UnixNetVConnection.cc index 3edf9eca083..d30c5b43c14 100644 --- a/src/iocore/net/UnixNetVConnection.cc +++ b/src/iocore/net/UnixNetVConnection.cc @@ -668,7 +668,7 @@ UnixNetVConnection::net_write_io(NetHandler *nh) nh->write_ready_list.remove(this); } - int err, ret; + int err{0}, ret{0}; if (this->get_context() == NET_VCONNECTION_OUT) { ret = this->sslStartHandShake(SSL_EVENT_CLIENT, err); diff --git a/src/proxy/http/HttpBodyFactory.cc b/src/proxy/http/HttpBodyFactory.cc index d62380a5170..5f6cb07087e 100644 --- a/src/proxy/http/HttpBodyFactory.cc +++ b/src/proxy/http/HttpBodyFactory.cc @@ -262,6 +262,7 @@ HttpBodyFactory::reconfigure() unlock(); return; } // callbacks not setup right + unlock(); //////////////////////////////////////////// // extract relevant records.yaml values // @@ -272,15 +273,15 @@ HttpBodyFactory::reconfigure() all_found = true; // enable_customizations if records.yaml set - rec_err = RecGetRecordInt("proxy.config.body_factory.enable_customizations", &e); - enable_customizations = ((rec_err == REC_ERR_OKAY) ? e : 0); - all_found = all_found && (rec_err == REC_ERR_OKAY); - Dbg(dbg_ctl_body_factory, "enable_customizations = %d (found = %" PRId64 ")", enable_customizations, e); + rec_err = RecGetRecordInt("proxy.config.body_factory.enable_customizations", &e); + int new_enable_customizations = ((rec_err == REC_ERR_OKAY) ? e : 0); + all_found = all_found && (rec_err == REC_ERR_OKAY); + Dbg(dbg_ctl_body_factory, "enable_customizations = %d (found = %" PRId64 ")", new_enable_customizations, e); - rec_err = RecGetRecordInt("proxy.config.body_factory.enable_logging", &e); - enable_logging = ((rec_err == REC_ERR_OKAY) ? (e ? true : false) : false); - all_found = all_found && (rec_err == REC_ERR_OKAY); - Dbg(dbg_ctl_body_factory, "enable_logging = %d (found = %" PRId64 ")", enable_logging, e); + rec_err = RecGetRecordInt("proxy.config.body_factory.enable_logging", &e); + bool new_enable_logging = ((rec_err == REC_ERR_OKAY) ? (e ? true : false) : false); + all_found = all_found && (rec_err == REC_ERR_OKAY); + Dbg(dbg_ctl_body_factory, "enable_logging = %d (found = %" PRId64 ")", new_enable_logging, e); ats_scoped_str directory_of_template_sets; @@ -306,21 +307,16 @@ HttpBodyFactory::reconfigure() Warning("config changed, but can't fetch all proxy.config.body_factory values"); } - ///////////////////////////////////////////// - // clear out previous template hash tables // - ///////////////////////////////////////////// - - nuke_template_tables(); - - ///////////////////////////////////////////////////////////// - // at this point, the body hash table is gone, so we start // - // building a new one, by scanning the template directory. // - ///////////////////////////////////////////////////////////// - + std::unique_ptr new_table_of_sets; if (directory_of_template_sets) { - table_of_sets = load_sets_from_directory(directory_of_template_sets); + new_table_of_sets = load_sets_from_directory(directory_of_template_sets); } + lock(); + enable_customizations = new_enable_customizations; + enable_logging = new_enable_logging; + nuke_template_tables(); + table_of_sets = std::move(new_table_of_sets); unlock(); } @@ -728,7 +724,6 @@ HttpBodyFactory::nuke_template_tables() } } -// LOCKING: must be called with lock taken std::unique_ptr HttpBodyFactory::load_sets_from_directory(char *set_dir) { @@ -798,7 +793,6 @@ HttpBodyFactory::load_sets_from_directory(char *set_dir) return new_table_of_sets; } -// LOCKING: must be called with lock taken HttpBodySet * HttpBodyFactory::load_body_set_from_directory(char *set_name, char *tmpl_dir) { diff --git a/src/proxy/http/HttpProxyServerMain.cc b/src/proxy/http/HttpProxyServerMain.cc index 876b38bc83e..7308d52d987 100644 --- a/src/proxy/http/HttpProxyServerMain.cc +++ b/src/proxy/http/HttpProxyServerMain.cc @@ -192,19 +192,30 @@ MakeHttpProxyAcceptor(HttpProxyAcceptor &acceptor, HttpProxyPort &port, unsigned // XXX the protocol probe should be a configuration option. - ProtocolProbeSessionAccept *probe = new ProtocolProbeSessionAccept(); + // A QUIC port is dispatched to the QUIC acceptor below, which has no probe fallback, so building a + // probe for one only leaks it. Every other port type ends up behind the probe, either directly or + // as the SSL acceptor's fallback. Without QUIC compiled in no port can be a QUIC port, so this is + // always true there. + bool const needs_probe = !port.isQUIC(); + + ProtocolProbeSessionAccept *probe = nullptr; HttpSessionAccept *http = nullptr; // don't allocate this unless it will be used. - probe->proxyPort = &port; - probe->proxy_protocol_ipmap = &HttpConfig::m_master.config_proxy_protocol_ip_addrs; - if (port.m_session_protocol_preference.intersects(HTTP_PROTOCOL_SET)) { - http = new HttpSessionAccept(accept_opt); - probe->registerEndpoint(ProtocolProbeSessionAccept::PROTO_HTTP, http); - } + if (needs_probe) { + probe = new ProtocolProbeSessionAccept(); + probe->proxyPort = &port; + probe->proxy_protocol_ipmap = &HttpConfig::m_master.config_proxy_protocol_ip_addrs; + + if (port.m_session_protocol_preference.intersects(HTTP_PROTOCOL_SET)) { + http = new HttpSessionAccept(accept_opt); + probe->registerEndpoint(ProtocolProbeSessionAccept::PROTO_HTTP, http); + } - if (port.m_session_protocol_preference.intersects(HTTP2_PROTOCOL_SET)) { - probe->registerEndpoint(ProtocolProbeSessionAccept::PROTO_HTTP2, new Http2SessionAccept(accept_opt)); + if (port.m_session_protocol_preference.intersects(HTTP2_PROTOCOL_SET)) { + probe->registerEndpoint(ProtocolProbeSessionAccept::PROTO_HTTP2, new Http2SessionAccept(accept_opt)); + } } + ProtocolSessionCreateMap.insert({TS_ALPN_PROTOCOL_INDEX_HTTP_1_0, create_h1_server_session}); ProtocolSessionCreateMap.insert({TS_ALPN_PROTOCOL_INDEX_HTTP_1_1, create_h1_server_session}); ProtocolSessionCreateMap.insert({TS_ALPN_PROTOCOL_INDEX_HTTP_2_0, create_h2_server_session}); diff --git a/src/proxy/http/HttpTransact.cc b/src/proxy/http/HttpTransact.cc index be7fd0a3262..16aa7efd345 100644 --- a/src/proxy/http/HttpTransact.cc +++ b/src/proxy/http/HttpTransact.cc @@ -7713,7 +7713,7 @@ void HttpTransact::handle_server_down(State *s) { const char *reason = nullptr; - const char *body_type = "UNKNOWN"; + const char *body_type = nullptr; HTTPStatus status = HTTP_STATUS_BAD_GATEWAY; //////////////////////////////////////////////////////// diff --git a/src/proxy/http/HttpTunnel.cc b/src/proxy/http/HttpTunnel.cc index d859791cbdd..088255743ad 100644 --- a/src/proxy/http/HttpTunnel.cc +++ b/src/proxy/http/HttpTunnel.cc @@ -1205,9 +1205,17 @@ HttpTunnel::producer_run(HttpTunnelProducer *p) } if (c_write == 0) { - // Nothing to do, call back the cleanup handlers - c->write_vio = nullptr; - consumer_handler(VC_EVENT_WRITE_COMPLETE, c); + // Cache writes need a VIO even when the body is empty so that closing the + // cache VC commits the response metadata instead of aborting the write. + if (c->vc_type == HT_CACHE_WRITE) { + c->write_vio = c->vc->do_io_write(this, 0, c->buffer_reader); + if (c->write_vio == nullptr) { + consumer_handler(VC_EVENT_ERROR, c); + } + } else { + c->write_vio = nullptr; + consumer_handler(VC_EVENT_WRITE_COMPLETE, c); + } } else { // In the client half close case, all the data that will be sent // from the client is already in the buffer. Go ahead and set diff --git a/src/proxy/http3/Http3Frame.cc b/src/proxy/http3/Http3Frame.cc index b3de9d2bffe..a4c8adbc970 100644 --- a/src/proxy/http3/Http3Frame.cc +++ b/src/proxy/http3/Http3Frame.cc @@ -576,8 +576,7 @@ Http3FrameFactory::create_headers_frame(IOBufferReader *header_block_reader, siz { ats_unique_buf buf = ats_unique_malloc(header_block_len); - int64_t nread; - while ((nread = header_block_reader->read(buf.get(), header_block_len)) > 0) { + while (header_block_reader->read(buf.get(), header_block_len) > 0) { ; } diff --git a/src/proxy/http3/QPACK.cc b/src/proxy/http3/QPACK.cc index 80db3b497a8..fed8e7ecef0 100644 --- a/src/proxy/http3/QPACK.cc +++ b/src/proxy/http3/QPACK.cc @@ -291,11 +291,13 @@ QPACK::decode(uint64_t stream_id, const uint8_t *header_block, size_t header_blo if (largest_reference != 0 && (this->_dynamic_table.is_empty() || this->_dynamic_table.largest_index() < largest_reference)) { // Blocked - if (this->_add_to_blocked_list( - new DecodeRequest(largest_reference, thread, cont, stream_id, header_block, header_block_len, hdr))) { + auto *decode_request = new DecodeRequest(largest_reference, thread, cont, stream_id, header_block, header_block_len, hdr); + + if (this->_add_to_blocked_list(decode_request)) { return 1; } else { // Number of blocked streams exceed the limit + delete decode_request; return -2; } } diff --git a/src/proxy/logging/LogAccess.cc b/src/proxy/logging/LogAccess.cc index 667f8eaf497..b40b77d5904 100644 --- a/src/proxy/logging/LogAccess.cc +++ b/src/proxy/logging/LogAccess.cc @@ -1629,11 +1629,7 @@ int LogAccess::marshal_client_host_port(char *buf) { if (m_http_sm) { - auto txn = m_http_sm->get_ua_txn(); - if (txn) { - uint16_t port = txn->get_client_port(); - marshal_int(buf, port); - } + marshal_int(buf, m_http_sm->t_state.effective_client_addr.host_order_port()); } return INK_MIN_ALIGN; } diff --git a/src/tscore/ink_queue.cc b/src/tscore/ink_queue.cc index 9ddee365415..86cab240010 100644 --- a/src/tscore/ink_queue.cc +++ b/src/tscore/ink_queue.cc @@ -171,6 +171,7 @@ ink_freelist_init(InkFreeList **fl, const char *name, uint32_t type_size, uint32 } Dbg(dbg_ctl_freelist_init, "<%s> Alignment request/actual (%" PRIu32 "/%" PRIu32 ")", name, alignment, f->alignment); Dbg(dbg_ctl_freelist_init, "<%s> Type Size request/actual (%" PRIu32 "/%" PRIu32 ")", name, type_size, f->type_size); + ink_assert(f->type_size != 0); if (f->use_hugepages) { f->chunk_size = INK_ALIGN(chunk_size * f->type_size, ats_hugepage_size()) / f->type_size; } else { diff --git a/tests/gold_tests/autest-site/ats_replay.test.ext b/tests/gold_tests/autest-site/ats_replay.test.ext index 92332b3d76c..f258ea14497 100644 --- a/tests/gold_tests/autest-site/ats_replay.test.ext +++ b/tests/gold_tests/autest-site/ats_replay.test.ext @@ -19,9 +19,19 @@ Implement general-purpose ATS test extensions using proxy verifier replay files. from typing import Optional import os +import re import yaml +def _contains_expression(contains_entry: dict, default_description: str): + '''Create a ContainsExpression tester from a log validation entry.''' + expression = contains_entry['expression'] + description = contains_entry.get('description', default_description) + reflags = re.S | re.M if contains_entry.get('multiline', False) else 0 + + return Testers.ContainsExpression(expression, description, reflags=reflags) + + def configure_ats(obj: 'TestRun', server: 'Process', ats_config: dict, dns: Optional['Process'] = None): '''Configure ATS per the configuration in the replay file. @@ -109,8 +119,7 @@ def configure_ats(obj: 'TestRun', server: 'Process', ats_config: dict, dns: Opti traffic_out = log_validation.get('traffic_out', {}) for contains_entry in traffic_out.get('contains', []): expression = contains_entry['expression'] - description = contains_entry.get('description', f'Verify traffic_out contains: {expression}') - ts.Disk.traffic_out.Content += Testers.ContainsExpression(expression, description) + ts.Disk.traffic_out.Content += _contains_expression(contains_entry, f'Verify traffic_out contains: {expression}') for excludes_entry in traffic_out.get('excludes', []): expression = excludes_entry['expression'] description = excludes_entry.get('description', f'Verify traffic_out excludes: {expression}') @@ -124,8 +133,7 @@ def configure_ats(obj: 'TestRun', server: 'Process', ats_config: dict, dns: Opti diags_log = log_validation.get('diags_log', {}) for contains_entry in diags_log.get('contains', []): expression = contains_entry['expression'] - description = contains_entry.get('description', f'Verify diags_log contains: {expression}') - ts.Disk.diags_log.Content += Testers.ContainsExpression(expression, description) + ts.Disk.diags_log.Content += _contains_expression(contains_entry, f'Verify diags_log contains: {expression}') for excludes_entry in diags_log.get('excludes', []): expression = excludes_entry['expression'] description = excludes_entry.get('description', f'Verify diags_log excludes: {expression}') @@ -196,6 +204,7 @@ def ATSReplayTest(obj, replay_file: str): ats_config = autest_config['ats'] process_config = ats_config.get('process_config', {}) enable_tls = process_config.get('enable_tls', ats_config.get('enable_tls', False)) + enable_quic = process_config.get('enable_quic', ats_config.get('enable_quic', False)) ts = configure_ats(tr, server=server, ats_config=ats_config, dns=dns) # Proxy Verifier Client configuration. @@ -203,10 +212,12 @@ def ATSReplayTest(obj, replay_file: str): raise ValueError(f"Replay file {replay_file} does not contain 'autest.client' section") client_config = autest_config['client'] name = client_config.get('name', 'client') - process_config = client_config.get('process_config', {}) - https_ports = [ts.Variables.ssl_port] if enable_tls else None + process_config = client_config.get('process_config', {}).copy() + http_ports = process_config.pop('http_ports', [ts.Variables.port]) + https_ports = process_config.pop('https_ports', [ts.Variables.ssl_port] if enable_tls else None) + http3_ports = process_config.pop('http3_ports', [ts.Variables.ssl_port] if enable_quic else None) client = tr.AddVerifierClientProcess( - name, replay_file, http_ports=[ts.Variables.port], https_ports=https_ports, **process_config) + name, replay_file, http_ports=http_ports, https_ports=https_ports, http3_ports=http3_ports, **process_config) # Set expected return code for client if specified. A list of codes is # wrapped in Any() so any of the listed values is accepted. diff --git a/tests/gold_tests/autest-site/when.test.ext b/tests/gold_tests/autest-site/when.test.ext index ad147e9b756..8975d27cb18 100644 --- a/tests/gold_tests/autest-site/when.test.ext +++ b/tests/gold_tests/autest-site/when.test.ext @@ -89,7 +89,7 @@ def AddAwaitFileContainsTestRun(test, name, file_path, needle, desired_count=1) ''' tr = test.AddTestRun(name) p = tr.Processes.Default - p.Command = f'echo waiting for {needle} in {file_path}' + p.Command = f'echo waiting for file content in {file_path}' await_process = tr.Processes.Process('await', 'sleep 60') await_process.Ready = When.FileContains(file_path, needle, desired_count) await_process.StartupTimeout = 30 diff --git a/tests/gold_tests/cache/replay/negative-caching-300-second-timeout.replay.yaml b/tests/gold_tests/cache/replay/negative-caching-300-second-timeout.replay.yaml index 7cb667b5ede..8083dbc60e7 100644 --- a/tests/gold_tests/cache/replay/negative-caching-300-second-timeout.replay.yaml +++ b/tests/gold_tests/cache/replay/negative-caching-300-second-timeout.replay.yaml @@ -40,6 +40,18 @@ meta: # transaction. delay: 100ms + - request_200_item: &request_200_item + client-request: + method: "GET" + version: "1.1" + scheme: "http" + url: /path/200_empty_chunked + headers: + fields: + - [ Host, example.com ] + + delay: 100ms + sessions: - transactions: @@ -47,13 +59,16 @@ sessions: <<: *request_404_item # Populate the cache with a 404 response. + # Verify that an empty chunked response is cached (issue #11313). server-response: status: 404 reason: "Not Found" headers: fields: - - [ Content-Length, 32 ] + - [ Transfer-Encoding, chunked ] - [ Cache-Control, max-age=300 ] + content: + size: 0 proxy-response: status: 404 @@ -74,3 +89,37 @@ sessions: # Expect the cached 404 response. proxy-response: status: 404 + + - all: { headers: { fields: [[ uuid, 23 ]]}} + <<: *request_200_item + + # The empty chunked-body behavior is not specific to negative responses. + server-response: + status: 200 + reason: OK + headers: + fields: + - [ Transfer-Encoding, chunked ] + - [ Cache-Control, max-age=300 ] + content: + size: 0 + + proxy-response: + status: 200 + + - all: { headers: { fields: [[ uuid, 24 ]]}} + <<: *request_200_item + + proxy-request: + expect: absent + + server-response: + status: 502 + reason: Bad Gateway + headers: + fields: + - [ Content-Length, 0 ] + + # Expect the cached 200 response. + proxy-response: + status: 200 diff --git a/tests/gold_tests/logging/log-debug-client-ip.test.py b/tests/gold_tests/logging/log-debug-client-ip.test.py index 4516fb893c5..a6894d8a65d 100644 --- a/tests/gold_tests/logging/log-debug-client-ip.test.py +++ b/tests/gold_tests/logging/log-debug-client-ip.test.py @@ -1,4 +1,5 @@ ''' +Verify debug logging filtered by client IP. ''' # Licensed to the Apache Software Foundation (ASF) under one # or more contributor license agreements. See the NOTICE file @@ -16,39 +17,15 @@ # See the License for the specific language governing permissions and # limitations under the License. -import os - Test.Summary = ''' -Test log filter. +Verify per-client-IP debug logging emits request and response header dumps. ''' -ts = Test.MakeATSProcess("ts", enable_cache=False) -replay_file = "log-filter.replays.yaml" -server = Test.MakeVerifierServerProcess("server", replay_file) -nameserver = Test.MakeDNServer("dns", default='127.0.0.1') - -ts.Disk.records_config.update( - { - 'proxy.config.diags.debug.enabled': 2, - 'proxy.config.diags.debug.tags': 'http', - 'proxy.config.diags.debug.client_ip': '127.0.0.1', - 'proxy.config.dns.nameservers': f"127.0.0.1:{nameserver.Variables.Port}", - }) -ts.Disk.remap_config.AddLine('map / http://localhost:{}/'.format(server.Variables.http_port)) +Test.ContinueOnFail = True -# Verify that the various aspects of the expected debug output for the -# transaction are logged. -ts.Disk.traffic_out.Content = Testers.ContainsExpression( - r"\+ Incoming Request \+", "Make sure the client request information is present.") -ts.Disk.traffic_out.Content += Testers.ContainsExpression( - r"\+ Proxy's Request after hooks \+", "Make sure the proxy request information is present.") -ts.Disk.traffic_out.Content += Testers.ContainsExpression( - r"\+ Incoming O.S. Response \+", "Make sure the server's response information is present.") -ts.Disk.traffic_out.Content += Testers.ContainsExpression( - r"\+ Proxy's Response 2 \+", "Make sure the proxy response information is present.") +Test.ATSReplayTest(replay_file='replay/log-debug-client-ip-http.replay.yaml') +Test.ATSReplayTest(replay_file='replay/log-debug-client-ip-https.replay.yaml') +Test.ATSReplayTest(replay_file='replay/log-debug-client-ip-http2.replay.yaml') -tr = Test.AddTestRun() -tr.Processes.Default.StartBefore(server) -tr.Processes.Default.StartBefore(ts) -tr.Processes.Default.StartBefore(nameserver) -tr.AddVerifierClientProcess("client-1", replay_file, http_ports=[ts.Variables.port], other_args="--keys test-1") +if Condition.HasATSFeature('TS_HAS_QUICHE') and Condition.HasCurlFeature('http3'): + Test.ATSReplayTest(replay_file='replay/log-debug-client-ip-http3.replay.yaml') diff --git a/tests/gold_tests/logging/replay/log-debug-client-ip-http.replay.yaml b/tests/gold_tests/logging/replay/log-debug-client-ip-http.replay.yaml new file mode 100644 index 00000000000..f92d3336d62 --- /dev/null +++ b/tests/gold_tests/logging/replay/log-debug-client-ip-http.replay.yaml @@ -0,0 +1,129 @@ +# 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. + +meta: + version: "1.0" + +autest: + description: 'Verify client-IP-filtered debug logging for HTTP/1.1' + + server: + name: 'server-log-debug-client-ip-http' + + client: + name: 'client-log-debug-client-ip-http' + + ats: + name: 'ts-log-debug-client-ip-http' + process_config: + enable_cache: false + + records_config: + proxy.config.diags.debug.enabled: 2 + proxy.config.diags.debug.tags: 'http' + proxy.config.diags.debug.client_ip: '127.0.0.1' + + remap_config: + - from: 'http://debug-client-ip-http.test/' + to: 'http://127.0.0.1:{SERVER_HTTP_PORT}/' + + log_validation: + traffic_out: + contains: + - expression: '\+ Incoming Request \+(?:(?!\+ Proxy''s Request after hooks \+)[\s\S])*?uuid: log-debug-http-1' + description: 'HTTP transaction 1 logs the incoming request headers.' + multiline: true + - expression: '\+ Proxy''s Request after hooks \+(?:(?!\+ Incoming O.S. Response \+)[\s\S])*?uuid: log-debug-http-1' + description: 'HTTP transaction 1 logs the proxy request headers after hooks.' + multiline: true + - expression: '\+ Incoming O.S. Response \+(?:(?!\+ Proxy''s Response 2 \+)[\s\S])*?uuid: log-debug-http-1' + description: 'HTTP transaction 1 logs the incoming origin response headers.' + multiline: true + - expression: '\+ Proxy''s Response 2 \+(?:(?!\+ Incoming Request \+)[\s\S])*?uuid: log-debug-http-1' + description: 'HTTP transaction 1 logs the proxy response headers.' + multiline: true + - expression: '\+ Incoming Request \+(?:(?!\+ Proxy''s Request after hooks \+)[\s\S])*?uuid: log-debug-http-2' + description: 'HTTP transaction 2 logs the incoming request headers.' + multiline: true + - expression: '\+ Proxy''s Request after hooks \+(?:(?!\+ Incoming O.S. Response \+)[\s\S])*?uuid: log-debug-http-2' + description: 'HTTP transaction 2 logs the proxy request headers after hooks.' + multiline: true + - expression: '\+ Incoming O.S. Response \+(?:(?!\+ Proxy''s Response 2 \+)[\s\S])*?uuid: log-debug-http-2' + description: 'HTTP transaction 2 logs the incoming origin response headers.' + multiline: true + - expression: '\+ Proxy''s Response 2 \+(?:(?!\+ Incoming Request \+)[\s\S])*?uuid: log-debug-http-2' + description: 'HTTP transaction 2 logs the proxy response headers.' + multiline: true + +sessions: +- transactions: + - client-request: + method: GET + url: /debug-http-1 + version: '1.1' + headers: + fields: + - [Host, debug-client-ip-http.test] + - [Content-Length, 0] + - [uuid, log-debug-http-1] + + proxy-request: + headers: + fields: + - [uuid, { value: log-debug-http-1, as: equal }] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, 0] + - [uuid, log-debug-http-1] + + proxy-response: + status: 200 + headers: + fields: + - [uuid, { value: log-debug-http-1, as: equal }] + + - client-request: + method: GET + url: /debug-http-2 + version: '1.1' + headers: + fields: + - [Host, debug-client-ip-http.test] + - [Content-Length, 0] + - [uuid, log-debug-http-2] + + proxy-request: + headers: + fields: + - [uuid, { value: log-debug-http-2, as: equal }] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, 0] + - [uuid, log-debug-http-2] + + proxy-response: + status: 200 + headers: + fields: + - [uuid, { value: log-debug-http-2, as: equal }] diff --git a/tests/gold_tests/logging/replay/log-debug-client-ip-http2.replay.yaml b/tests/gold_tests/logging/replay/log-debug-client-ip-http2.replay.yaml new file mode 100644 index 00000000000..5230a7ebeb9 --- /dev/null +++ b/tests/gold_tests/logging/replay/log-debug-client-ip-http2.replay.yaml @@ -0,0 +1,134 @@ +# 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. + +meta: + version: "1.0" + +autest: + description: 'Verify client-IP-filtered debug logging for HTTP/2' + + server: + name: 'server-log-debug-client-ip-http2' + + client: + name: 'client-log-debug-client-ip-http2' + + ats: + name: 'ts-log-debug-client-ip-http2' + process_config: + enable_cache: false + enable_tls: true + + records_config: + proxy.config.diags.debug.enabled: 2 + proxy.config.diags.debug.tags: 'http' + proxy.config.diags.debug.client_ip: '127.0.0.1' + + remap_config: + - from: 'https://debug-client-ip-http2.test/' + to: 'http://127.0.0.1:{SERVER_HTTP_PORT}/' + + log_validation: + traffic_out: + contains: + - expression: '\+ Incoming Request \+(?:(?!\+ Proxy''s Request after hooks \+)[\s\S])*?uuid: log-debug-http2-1' + description: 'HTTP/2 transaction 1 logs the incoming request headers.' + multiline: true + - expression: '\+ Proxy''s Request after hooks \+(?:(?!\+ Incoming O.S. Response \+)[\s\S])*?uuid: log-debug-http2-1' + description: 'HTTP/2 transaction 1 logs the proxy request headers after hooks.' + multiline: true + - expression: '\+ Incoming O.S. Response \+(?:(?!\+ Proxy''s Response 2 \+)[\s\S])*?uuid: log-debug-http2-1' + description: 'HTTP/2 transaction 1 logs the incoming origin response headers.' + multiline: true + - expression: '\+ Proxy''s Response 2 \+(?:(?!\+ Incoming Request \+)[\s\S])*?uuid: log-debug-http2-1' + description: 'HTTP/2 transaction 1 logs the proxy response headers.' + multiline: true + - expression: '\+ Incoming Request \+(?:(?!\+ Proxy''s Request after hooks \+)[\s\S])*?uuid: log-debug-http2-2' + description: 'HTTP/2 transaction 2 logs the incoming request headers.' + multiline: true + - expression: '\+ Proxy''s Request after hooks \+(?:(?!\+ Incoming O.S. Response \+)[\s\S])*?uuid: log-debug-http2-2' + description: 'HTTP/2 transaction 2 logs the proxy request headers after hooks.' + multiline: true + - expression: '\+ Incoming O.S. Response \+(?:(?!\+ Proxy''s Response 2 \+)[\s\S])*?uuid: log-debug-http2-2' + description: 'HTTP/2 transaction 2 logs the incoming origin response headers.' + multiline: true + - expression: '\+ Proxy''s Response 2 \+(?:(?!\+ Incoming Request \+)[\s\S])*?uuid: log-debug-http2-2' + description: 'HTTP/2 transaction 2 logs the proxy response headers.' + multiline: true + +sessions: +- protocol: + stack: http2 + tls: + sni: debug-client-ip-http2.test + transactions: + - client-request: + headers: + fields: + - [":method", GET] + - [":scheme", https] + - [":authority", debug-client-ip-http2.test] + - [":path", /debug-http2-1] + - [Content-Length, 0] + - [uuid, log-debug-http2-1] + + proxy-request: + headers: + fields: + - [uuid, { value: log-debug-http2-1, as: equal }] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, 0] + - [uuid, log-debug-http2-1] + + proxy-response: + status: 200 + headers: + fields: + - [uuid, { value: log-debug-http2-1, as: equal }] + + - client-request: + headers: + fields: + - [":method", GET] + - [":scheme", https] + - [":authority", debug-client-ip-http2.test] + - [":path", /debug-http2-2] + - [Content-Length, 0] + - [uuid, log-debug-http2-2] + + proxy-request: + headers: + fields: + - [uuid, { value: log-debug-http2-2, as: equal }] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, 0] + - [uuid, log-debug-http2-2] + + proxy-response: + status: 200 + headers: + fields: + - [uuid, { value: log-debug-http2-2, as: equal }] diff --git a/tests/gold_tests/logging/replay/log-debug-client-ip-http3.replay.yaml b/tests/gold_tests/logging/replay/log-debug-client-ip-http3.replay.yaml new file mode 100644 index 00000000000..d0599f91a07 --- /dev/null +++ b/tests/gold_tests/logging/replay/log-debug-client-ip-http3.replay.yaml @@ -0,0 +1,136 @@ +# 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. + +meta: + version: "1.0" + +autest: + description: 'Verify client-IP-filtered debug logging for HTTP/3' + + server: + name: 'server-log-debug-client-ip-http3' + + client: + name: 'client-log-debug-client-ip-http3' + + ats: + name: 'ts-log-debug-client-ip-http3' + process_config: + enable_cache: false + enable_tls: true + enable_quic: true + + records_config: + proxy.config.diags.debug.enabled: 2 + proxy.config.diags.debug.tags: 'http' + proxy.config.diags.debug.client_ip: '127.0.0.1' + proxy.config.quic.no_activity_timeout_in: 0 + + remap_config: + - from: 'https://debug-client-ip-http3.test/' + to: 'http://127.0.0.1:{SERVER_HTTP_PORT}/' + + log_validation: + traffic_out: + contains: + - expression: '\+ Incoming Request \+(?:(?!\+ Proxy''s Request after hooks \+)[\s\S])*?uuid: log-debug-http3-1' + description: 'HTTP/3 transaction 1 logs the incoming request headers.' + multiline: true + - expression: '\+ Proxy''s Request after hooks \+(?:(?!\+ Incoming O.S. Response \+)[\s\S])*?uuid: log-debug-http3-1' + description: 'HTTP/3 transaction 1 logs the proxy request headers after hooks.' + multiline: true + - expression: '\+ Incoming O.S. Response \+(?:(?!\+ Proxy''s Response 2 \+)[\s\S])*?uuid: log-debug-http3-1' + description: 'HTTP/3 transaction 1 logs the incoming origin response headers.' + multiline: true + - expression: '\+ Proxy''s Response 2 \+(?:(?!\+ Incoming Request \+)[\s\S])*?uuid: log-debug-http3-1' + description: 'HTTP/3 transaction 1 logs the proxy response headers.' + multiline: true + - expression: '\+ Incoming Request \+(?:(?!\+ Proxy''s Request after hooks \+)[\s\S])*?uuid: log-debug-http3-2' + description: 'HTTP/3 transaction 2 logs the incoming request headers.' + multiline: true + - expression: '\+ Proxy''s Request after hooks \+(?:(?!\+ Incoming O.S. Response \+)[\s\S])*?uuid: log-debug-http3-2' + description: 'HTTP/3 transaction 2 logs the proxy request headers after hooks.' + multiline: true + - expression: '\+ Incoming O.S. Response \+(?:(?!\+ Proxy''s Response 2 \+)[\s\S])*?uuid: log-debug-http3-2' + description: 'HTTP/3 transaction 2 logs the incoming origin response headers.' + multiline: true + - expression: '\+ Proxy''s Response 2 \+(?:(?!\+ Incoming Request \+)[\s\S])*?uuid: log-debug-http3-2' + description: 'HTTP/3 transaction 2 logs the proxy response headers.' + multiline: true + +sessions: +- protocol: + stack: http3 + tls: + sni: debug-client-ip-http3.test + transactions: + - client-request: + headers: + fields: + - [":method", GET] + - [":scheme", https] + - [":authority", debug-client-ip-http3.test] + - [":path", /debug-http3-1] + - [Content-Length, 0] + - [uuid, log-debug-http3-1] + + proxy-request: + headers: + fields: + - [uuid, { value: log-debug-http3-1, as: equal }] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, 0] + - [uuid, log-debug-http3-1] + + proxy-response: + status: 200 + headers: + fields: + - [uuid, { value: log-debug-http3-1, as: equal }] + + - client-request: + headers: + fields: + - [":method", GET] + - [":scheme", https] + - [":authority", debug-client-ip-http3.test] + - [":path", /debug-http3-2] + - [Content-Length, 0] + - [uuid, log-debug-http3-2] + + proxy-request: + headers: + fields: + - [uuid, { value: log-debug-http3-2, as: equal }] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, 0] + - [uuid, log-debug-http3-2] + + proxy-response: + status: 200 + headers: + fields: + - [uuid, { value: log-debug-http3-2, as: equal }] diff --git a/tests/gold_tests/logging/replay/log-debug-client-ip-https.replay.yaml b/tests/gold_tests/logging/replay/log-debug-client-ip-https.replay.yaml new file mode 100644 index 00000000000..79e9b5aa4b7 --- /dev/null +++ b/tests/gold_tests/logging/replay/log-debug-client-ip-https.replay.yaml @@ -0,0 +1,134 @@ +# 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. + +meta: + version: "1.0" + +autest: + description: 'Verify client-IP-filtered debug logging for HTTPS' + + server: + name: 'server-log-debug-client-ip-https' + + client: + name: 'client-log-debug-client-ip-https' + + ats: + name: 'ts-log-debug-client-ip-https' + process_config: + enable_cache: false + enable_tls: true + + records_config: + proxy.config.diags.debug.enabled: 2 + proxy.config.diags.debug.tags: 'http' + proxy.config.diags.debug.client_ip: '127.0.0.1' + + remap_config: + - from: 'https://debug-client-ip-https.test/' + to: 'http://127.0.0.1:{SERVER_HTTP_PORT}/' + + log_validation: + traffic_out: + contains: + - expression: '\+ Incoming Request \+(?:(?!\+ Proxy''s Request after hooks \+)[\s\S])*?uuid: log-debug-https-1' + description: 'HTTPS transaction 1 logs the incoming request headers.' + multiline: true + - expression: '\+ Proxy''s Request after hooks \+(?:(?!\+ Incoming O.S. Response \+)[\s\S])*?uuid: log-debug-https-1' + description: 'HTTPS transaction 1 logs the proxy request headers after hooks.' + multiline: true + - expression: '\+ Incoming O.S. Response \+(?:(?!\+ Proxy''s Response 2 \+)[\s\S])*?uuid: log-debug-https-1' + description: 'HTTPS transaction 1 logs the incoming origin response headers.' + multiline: true + - expression: '\+ Proxy''s Response 2 \+(?:(?!\+ Incoming Request \+)[\s\S])*?uuid: log-debug-https-1' + description: 'HTTPS transaction 1 logs the proxy response headers.' + multiline: true + - expression: '\+ Incoming Request \+(?:(?!\+ Proxy''s Request after hooks \+)[\s\S])*?uuid: log-debug-https-2' + description: 'HTTPS transaction 2 logs the incoming request headers.' + multiline: true + - expression: '\+ Proxy''s Request after hooks \+(?:(?!\+ Incoming O.S. Response \+)[\s\S])*?uuid: log-debug-https-2' + description: 'HTTPS transaction 2 logs the proxy request headers after hooks.' + multiline: true + - expression: '\+ Incoming O.S. Response \+(?:(?!\+ Proxy''s Response 2 \+)[\s\S])*?uuid: log-debug-https-2' + description: 'HTTPS transaction 2 logs the incoming origin response headers.' + multiline: true + - expression: '\+ Proxy''s Response 2 \+(?:(?!\+ Incoming Request \+)[\s\S])*?uuid: log-debug-https-2' + description: 'HTTPS transaction 2 logs the proxy response headers.' + multiline: true + +sessions: +- protocol: + stack: https + tls: + sni: debug-client-ip-https.test + transactions: + - client-request: + method: GET + url: /debug-https-1 + version: '1.1' + headers: + fields: + - [Host, debug-client-ip-https.test] + - [Content-Length, 0] + - [uuid, log-debug-https-1] + + proxy-request: + headers: + fields: + - [uuid, { value: log-debug-https-1, as: equal }] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, 0] + - [uuid, log-debug-https-1] + + proxy-response: + status: 200 + headers: + fields: + - [uuid, { value: log-debug-https-1, as: equal }] + + - client-request: + method: GET + url: /debug-https-2 + version: '1.1' + headers: + fields: + - [Host, debug-client-ip-https.test] + - [Content-Length, 0] + - [uuid, log-debug-https-2] + + proxy-request: + headers: + fields: + - [uuid, { value: log-debug-https-2, as: equal }] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, 0] + - [uuid, log-debug-https-2] + + proxy-response: + status: 200 + headers: + fields: + - [uuid, { value: log-debug-https-2, as: equal }] diff --git a/tests/gold_tests/pluginTest/healthchecks/healthchecks.test.py b/tests/gold_tests/pluginTest/healthchecks/healthchecks.test.py index fb36c1d9548..d6348f88596 100644 --- a/tests/gold_tests/pluginTest/healthchecks/healthchecks.test.py +++ b/tests/gold_tests/pluginTest/healthchecks/healthchecks.test.py @@ -44,6 +44,9 @@ def __init__(self) -> None: self._expect_acme_ssl_404() self._re_add_acme_ssl() self._expect_positive_healthchecks() + self._expect_full_buffer_acme_body() + self._rewrite_acme_while_serving() + self._expect_rewritten_acme_body() def _configure_global_ts(self) -> None: '''Configure a global Traffic Server instance for the test runs. @@ -143,6 +146,57 @@ def _re_add_acme_ssl(self) -> None: p.Command = 'sleep 1' p.ReturnCode = 0 + def _rewrite_acme_while_serving(self) -> None: + '''Rewrite the acme file repeatedly while healthcheck requests are in flight. + + The plugin replaces the health check file data underneath transactions which may still be + reading the previous data. This drives that replacement so that an ASan enabled build + catches the old data being released while it is still referenced. + :return: None + ''' + tr = Test.AddTestRun('Rewrite acme while healthchecks are being served') + acme_file = os.path.join(Test.RunDirectory, 'acme') + url = f'http://127.0.0.1:{self._ts.Variables.port}/acme' + + # Note that autest runs the command through string.Template, so shell variables cannot be + # used here. The loop is therefore unrolled. + commands = [] + for iteration in range(10): + commands.append(f'echo "{CONTENT} {iteration}" > {acme_file};') + commands.append('{curl} -s -o /dev/null ' + url + ' &') + commands.append('{curl} -s -o /dev/null ' + url + ' &') + commands.append('wait') + + tr.MakeCurlCommandMulti(' '.join(commands), ts=self._ts) + tr.Processes.Default.ReturnCode = 0 + + def _expect_full_buffer_acme_body(self) -> None: + '''Verify that a MAX_BODY_LEN-sized file is not reported as empty. + :return: None + ''' + tr = Test.AddTestRun('Expect a full-sized healthcheck response body') + acme_file = os.path.join(Test.RunDirectory, 'acme') + url = f'http://127.0.0.1:{self._ts.Variables.port}/acme' + command = (f'dd if=/dev/zero of={acme_file} bs=16384 count=1 2>/dev/null && sleep 1 && ' + '{curl} -s ' + url + ' | wc -c') + tr.MakeCurlCommandMulti(command, ts=self._ts) + p = tr.Processes.Default + p.ReturnCode = 0 + p.Streams.All += Testers.ContainsExpression('16384', 'Verify the response contains 16 KiB') + + def _expect_rewritten_acme_body(self) -> None: + '''Verify that the most recently written acme content is what gets served. + :return: None + ''' + tr = Test.AddTestRun('Expect the last written acme content in the response body') + acme_file = os.path.join(Test.RunDirectory, 'acme') + url = f'http://127.0.0.1:{self._ts.Variables.port}/acme' + command = f'echo "{CONTENT} final" > {acme_file} && sleep 1 && ' + '{curl} -v ' + url + tr.MakeCurlCommandMulti(command, ts=self._ts) + p = tr.Processes.Default + p.ReturnCode = 0 + p.Streams.All += Testers.ContainsExpression('HTTP/1.1 200', 'Verify 200 response for /acme') + p.Streams.All += Testers.ContainsExpression(f'{CONTENT} final', 'Verify the reloaded acme content is served') + # Instantiate the test TestFileChangeBehavior() diff --git a/tests/gold_tests/pluginTest/slice/gold/slice_prefetch.gold b/tests/gold_tests/pluginTest/slice/gold/slice_prefetch.gold deleted file mode 100644 index 3445708de2c..00000000000 --- a/tests/gold_tests/pluginTest/slice/gold/slice_prefetch.gold +++ /dev/null @@ -1,26 +0,0 @@ -bytes 0-6/18 miss -bytes ``/18 miss -bytes ``/18 miss -bytes 14-17/18 hit-fresh -- miss, none -bytes 0-6/18 hit-fresh -bytes 7-13/18 hit-fresh -bytes 14-17/18 hit-fresh -- hit-fresh, none -bytes 0-6/18 hit-stale -bytes ``/18 hit-stale -bytes ``/18 hit-stale -bytes 14-17/18 hit-fresh -- hit-stale, none -bytes 0-6/18 hit-fresh -bytes 7-13/18 hit-fresh -bytes 14-17/18 hit-fresh -bytes 0-17/18 hit-fresh, none -bytes 0-4/18 miss -bytes ``/18 miss -bytes ``/18 miss -bytes ``/18 miss -bytes 15-17/18 hit-fresh -bytes 5-16/18 miss, none -bytes 0-6/18 hit-fresh -*/18 hit-fresh, none diff --git a/tests/gold_tests/pluginTest/slice/slice_prefetch.test.py b/tests/gold_tests/pluginTest/slice/slice_prefetch.test.py index 5b544e9d091..28eecfe2ee2 100644 --- a/tests/gold_tests/pluginTest/slice/slice_prefetch.test.py +++ b/tests/gold_tests/pluginTest/slice/slice_prefetch.test.py @@ -16,6 +16,7 @@ # See the License for the specific language governing permissions and # limitations under the License. import os +import re Test.Summary = ''' slice plugin prefetch feature test @@ -174,8 +175,39 @@ # 6 Test - All requests (client & slice internal) logs to see background fetches cache_file = os.path.join(ts.Variables.LOGDIR, 'cache.log') -# Wait for log file to appear, then wait one extra second to make sure TS is done writing it. -test_run = Test.AddTestRun("Checking debug logs for background fetches") -test_run.Processes.Default.Command = (os.path.join(Test.Variables.AtsTestToolsDir, 'condwait') + ' 60 1 -f ' + cache_file) -ts.Disk.File(cache_file).Content = "gold/slice_prefetch.gold" -test_run.Processes.Default.ReturnCode = 0 +# Wait for the final cache log line to be written. +test_run = Test.AddAwaitFileContainsTestRun( + "Checking debug logs for background fetches", + cache_file, + r'\*/18 hit-fresh, none$', +) +cache_log = ts.Disk.File(cache_file) +expected_cache_entries = [ + "bytes 0-6/18 miss", + "bytes 7-13/18 miss", + "bytes 14-17/18 miss", + "bytes 14-17/18 hit-fresh", + "- miss, none", + "bytes 0-6/18 hit-fresh", + "bytes 7-13/18 hit-fresh", + "bytes 14-17/18 hit-fresh", + "- hit-fresh, none", + "bytes 0-6/18 hit-stale", + "bytes 7-13/18 hit-stale", + "bytes 14-17/18 hit-stale", + "- hit-stale, none", + "bytes 0-17/18 hit-fresh, none", + "bytes 0-4/18 miss", + "bytes 5-9/18 miss", + "bytes 10-14/18 miss", + "bytes 15-17/18 miss", + "bytes 15-17/18 hit-fresh", + "bytes 5-16/18 miss, none", + "*/18 hit-fresh, none", +] +for index, entry in enumerate(expected_cache_entries): + tester = Testers.ContainsExpression(f'(?m)^{re.escape(entry)}$', f'Verify cache log contains: {entry}') + if index == 0: + cache_log.Content = tester + else: + cache_log.Content += tester diff --git a/tests/gold_tests/pluginTest/stale_response/stale_response.test.py b/tests/gold_tests/pluginTest/stale_response/stale_response.test.py index 40d7b9841e9..b294715dd1b 100644 --- a/tests/gold_tests/pluginTest/stale_response/stale_response.test.py +++ b/tests/gold_tests/pluginTest/stale_response/stale_response.test.py @@ -131,6 +131,8 @@ def setupTS(self) -> None: "proxy.config.http.server_session_sharing.pool": "global", # Turn off negative revalidating so that we can test stale-if-error. "proxy.config.http.negative_revalidating_enabled": 0, + # Keep the active log filename available for the final content check if the test spans UTC midnight. + "proxy.config.log.rolling_enabled": 0, }) ts.Disk.remap_config.AddLine(f"map / http://127.0.0.1:{self._server.Variables.http_port}/ {remap_plugin_config}") @@ -153,28 +155,22 @@ def verify_plugin_log(self) -> None: diagnostic, "Verify max-memory stale-if-error fallback is logged") return - tr = Test.AddTestRun("Verify stale_response plugin log") - name = f'log_waiter_{TestStaleResponse._ts_counter}' - log_waiter = tr.Processes.Process(name) - log_waiter.Command = 'sleep 30' + swr_log_pattern = "stale-while-revalidate:.*stale.jpeg" + sie_log_pattern = "stale-if-error:.*error.jpeg" + + def expect_log_entry(pattern: str, description: str) -> None: + tr = Test.AddAwaitFileContainsTestRun(f"Await {description}", self._ts.Disk.stale_responses_log.AbsPath, pattern) + tr.StillRunningBefore = self._ts + tr.StillRunningAfter = self._ts + self._ts.Disk.stale_responses_log.Content += Testers.ContainsExpression(pattern, f"Verify {description}") + if self._option_type == OptionType.FORCE_SWR: - log_waiter.Ready = When.FileContains(self._ts.Disk.stale_responses_log.Name, "stale-while-revalidate:") - self._ts.Disk.stale_responses_log.Content += Testers.ContainsExpression( - "stale-while-revalidate:.*stale.jpeg", "Verify stale-while-revalidate directive is logged") + expect_log_entry(swr_log_pattern, "stale-while-revalidate directive is logged") elif self._option_type == OptionType.FORCE_SIE: - log_waiter.Ready = When.FileContains(self._ts.Disk.stale_responses_log.Name, "stale-if-error:") - self._ts.Disk.stale_responses_log.Content += Testers.ContainsExpression( - "stale-if-error:.*error.jpeg", "Verify stale-if-error directive is logged") + expect_log_entry(sie_log_pattern, "stale-if-error directive is logged") else: - log_waiter.Ready = When.FileContains(self._ts.Disk.stale_responses_log.Name, "stale-if-error:") - self._ts.Disk.stale_responses_log.Content += Testers.ContainsExpression( - "stale-while-revalidate:.*stale.jpeg", "Verify stale-while-revalidate directive is logged") - self._ts.Disk.stale_responses_log.Content += Testers.ContainsExpression( - "stale-if-error:.*error.jpeg", "Verify stale-if-error directive is logged") - p = tr.Processes.Default - p.Command = 'echo "Waiting upon the stale response log."' - p.StartBefore(log_waiter) - p.StillRunningAfter = self._ts + expect_log_entry(swr_log_pattern, "stale-while-revalidate directive is logged") + expect_log_entry(sie_log_pattern, "stale-if-error directive is logged") TestStaleResponse(OptionType.NONE, is_global=True) diff --git a/tests/gold_tests/pluginTest/webp_transform/webp_transform_invalid_input.replay.yaml b/tests/gold_tests/pluginTest/webp_transform/webp_transform_invalid_input.replay.yaml new file mode 100644 index 00000000000..ccc8cc5de24 --- /dev/null +++ b/tests/gold_tests/pluginTest/webp_transform/webp_transform_invalid_input.replay.yaml @@ -0,0 +1,196 @@ +# 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. + +meta: + version: "1.0" + +autest: + description: 'Verify webp_transform safely passes through invalid image bodies' + + server: + name: 'webp-transform-server' + + client: + name: 'webp-transform-client' + + ats: + name: 'webp-transform-ts' + process_config: + enable_cache: false + disable_log_checks: true + + plugin_config: + - 'webp_transform.so convert_to_jpeg,convert_to_webp' + + records_config: + proxy.config.diags.debug.enabled: 1 + proxy.config.diags.debug.tags: 'webp_transform' + + remap_config: + - from: 'http://www.example.com/' + to: 'http://127.0.0.1:{SERVER_HTTP_PORT}/' + + log_validation: + diags_log: + contains: + - expression: 'input body does not match its declared image encoding' + description: 'Verify invalid input is rejected before conversion' + excludes: + - expression: 'zero-length blob not permitted|no decode delegate for this image format' + description: 'Verify ImageMagick never receives the invalid input' + +sessions: + - transactions: + - client-request: + method: GET + version: '1.1' + url: /empty.webp + headers: + fields: + - [Host, www.example.com] + - [Accept, 'image/jpeg'] + - [uuid, empty-webp] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Type, image/webp] + - [Content-Length, 0] + content: + size: 0 + + proxy-response: + status: 200 + headers: + fields: + - [Content-Type, {value: image/webp, as: equal}] + content: + size: 0 + + - client-request: + method: GET + version: '1.1' + url: /invalid.webp + headers: + fields: + - [Host, www.example.com] + - [Accept, 'image/jpeg'] + - [uuid, invalid-webp] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Type, image/webp] + - [Content-Length, 1] + content: + data: x + + proxy-response: + status: 200 + headers: + fields: + - [Content-Type, {value: image/webp, as: equal}] + content: + verify: {value: x, as: equal} + + - client-request: + method: GET + version: '1.1' + url: /invalid.jpg + headers: + fields: + - [Host, www.example.com] + - [Accept, 'image/webp'] + - [uuid, invalid-jpeg] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Type, image/jpeg] + - [Content-Length, 8] + content: + data: not-jpeg + + proxy-response: + status: 200 + headers: + fields: + - [Content-Type, {value: image/jpeg, as: equal}] + content: + verify: {value: not-jpeg, as: equal} + + - client-request: + method: GET + version: '1.1' + url: /valid.webp + headers: + fields: + - [Host, www.example.com] + - [Accept, 'image/jpeg'] + - [uuid, valid-webp] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Type, image/webp] + - [Content-Length, 68] + content: + encoding: uri + data: "%52%49%46%46%3c%00%00%00%57%45%42%50%56%50%38%20%30%00%00%00%d0%01%00%9d\ + %01%2a%01%00%01%00%02%00%34%25%a0%02%74%ba%01%f8%00%03%b0%00%fe%f0%c4%0b\ + %ff%20%b9%61%75%c8%d7%ff%20%3f%e4%07%fc%80%ff%f8%f2%00%00%00" + + proxy-response: + status: 200 + headers: + fields: + - [Content-Type, {value: image/jpeg, as: equal}] + - [Vary, {value: Accept, as: equal}] + content: + verify: {value: JFIF, as: contains} + + - client-request: + method: GET + version: '1.1' + url: /health + headers: + fields: + - [Host, www.example.com] + - [uuid, health] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Type, text/plain] + - [Content-Length, 2] + content: + data: OK + + proxy-response: + status: 200 + content: + verify: {value: OK, as: equal} diff --git a/tests/gold_tests/pluginTest/webp_transform/webp_transform_invalid_input.test.py b/tests/gold_tests/pluginTest/webp_transform/webp_transform_invalid_input.test.py new file mode 100644 index 00000000000..e5c4961b909 --- /dev/null +++ b/tests/gold_tests/pluginTest/webp_transform/webp_transform_invalid_input.test.py @@ -0,0 +1,25 @@ +''' +Verify webp_transform rejects invalid image input before invoking ImageMagick. +''' +# 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. + +Test.Summary = 'Verify webp_transform safely passes through invalid image bodies' + +Test.SkipUnless(Condition.PluginExists('webp_transform.so')) + +Test.ATSReplayTest(replay_file='webp_transform_invalid_input.replay.yaml') diff --git a/tests/gold_tests/tls/tls_async_handshake.test.py b/tests/gold_tests/tls/tls_async_handshake.test.py index 5ac4e289593..e0050010eee 100644 --- a/tests/gold_tests/tls/tls_async_handshake.test.py +++ b/tests/gold_tests/tls/tls_async_handshake.test.py @@ -33,7 +33,8 @@ ts = Test.MakeATSProcess("ts", enable_tls=True) server = Test.MakeOriginServer("server") -Test.PrepareTestPlugin(async_handshake, ts) +if os.path.isfile(async_handshake): + Test.PrepareTestPlugin(async_handshake, ts) server.addResponse( "sessionlog.json", { diff --git a/tests/proxy-verifier-checksum.txt b/tests/proxy-verifier-checksum.txt index 9c2ec8528be..90b31901ff1 100644 --- a/tests/proxy-verifier-checksum.txt +++ b/tests/proxy-verifier-checksum.txt @@ -1 +1 @@ -342286244d441329c12de1122520da6c86c581fd +5485a3cea4359e86458bf6d369dc18cdcfbd433d diff --git a/tests/proxy-verifier-version.txt b/tests/proxy-verifier-version.txt index 66cfae52b28..6d260c3af09 100644 --- a/tests/proxy-verifier-version.txt +++ b/tests/proxy-verifier-version.txt @@ -1 +1 @@ -v3.1.3 +v3.2.0