rate_limit: reject unknown YAML configuration keys

doc/admin-guide/plugins/rate_limit.en.rst

@@ -142,7 +142,14 @@ configuration file. The basic use is as::
 
 
 The YAML configuration can have the following format, where the various sections
-and nodes are documented below.
+and nodes are documented below. Unknown keys at any level cause configuration
+loading to fail, with a diagnostic identifying the key, node, and line number.
+An invalid value, such as a non-numeric ``limit``, fails the load the same way.
+A failed reload keeps the previous configuration active. Use ``max_age`` (with
+an underscore) for the ``queue``, ``ip-rep``, and ``perma-block`` aging settings.
+
+The file must hold a YAML map. An empty file is an error. To load the plugin
+with no rules, write ``selector: []``.
 
    .. code-block:: yaml
 

doc/check_response_validity-empty-eos.md

@@ -0,0 +1,72 @@
+# Empty origin response with EOS
+
+This note walks the code path for the case where the origin server sends no response bytes at all and then closes the connection with EOS.
+
+## Short answer
+
+- In the real transaction path, `check_response_validity()` is not called.
+- The transaction reaches `HandleResponse()` with `current.state == CONNECTION_CLOSED`, so `is_response_valid()` returns `false` early and sets `response_error = CONNECTION_OPEN_FAILED`.
+- If `check_response_validity()` were called directly on the untouched `server_response` header object anyway, it would return `ResponseError_t::MISSING_STATUS_CODE`.
+
+## Code path
+
+1. `HttpSM::setup_server_read_response_header()` resets the origin response header with:
+   - `t_state.hdr_info.server_response.destroy();`
+   - `t_state.hdr_info.server_response.create(HTTPType::RESPONSE);`
+
+   That creates a valid `HTTPHdr` object with response polarity, but its fields are still zero-initialized. In particular, the response status is still `HTTPStatus::NONE`.
+
+2. `HttpSM::state_read_server_response_header()` receives `VC_EVENT_EOS` before any response bytes are read.
+   - It sets `server_entry->eos = true`.
+   - `server_response_hdr_bytes` is still `0`.
+   - It then calls `t_state.hdr_info.server_response.parse_resp(..., eof = true)`.
+
+3. `HTTPHdr::parse_resp(HTTPParser *, IOBufferReader *, int *, bool)` handles the empty-buffer EOF case specially:
+   - if `b_avail <= 0`
+   - and `eof == true`
+   - and `start == nullptr`
+   - then it returns `ParseResult::ERROR` immediately.
+
+   So an empty origin response plus EOS is treated as a parse failure.
+
+4. Back in `HttpSM::state_read_server_response_header()`, this lands in the `ParseResult::ERROR` branch.
+   - It sets `t_state.current.state = HttpTransact::PARSE_ERROR`.
+   - Because the event was `VC_EVENT_EOS`, it calls `handle_server_setup_error(VC_EVENT_EOS, data)`.
+
+5. `HttpSM::handle_server_setup_error()` then converts the state to the connection-close path:
+   - `t_state.current.state = HttpTransact::CONNECTION_CLOSED`
+   - `t_state.set_connect_fail(EPIPE)`
+   - `call_transact_and_set_next_state(HttpTransact::HandleResponse)`
+
+6. `HttpTransact::HandleResponse()` calls:
+
+   `HttpTransact::is_response_valid(s, &s->hdr_info.server_response)`
+
+   But `is_response_valid()` starts with a guard:
+
+   - if `s->current.state != CONNECTION_ALIVE`
+   - set `s->hdr_info.response_error = ResponseError_t::CONNECTION_OPEN_FAILED`
+   - return `false`
+
+   Because the state is already `CONNECTION_CLOSED`, execution returns here. `check_response_validity()` is never reached.
+
+## What `check_response_validity()` would return if called directly
+
+Even though the live path does not call it, the return value is straightforward from the initialized header state:
+
+1. `incoming_hdr` is not null.
+2. `incoming_hdr->type_get() == HTTPType::RESPONSE` because the header was created with `create(HTTPType::RESPONSE)`.
+3. `incoming_hdr->status_get() == HTTPStatus::NONE` because no bytes were parsed into the header.
+4. `HttpTransact::check_response_validity()` therefore hits:
+
+   `if (incoming_status == HTTPStatus::NONE) { return ResponseError_t::MISSING_STATUS_CODE; }`
+
+So the helper's direct return would be `ResponseError_t::MISSING_STATUS_CODE`.
+
+## Conclusion
+
+For an origin response of "0 bytes, then EOS":
+
+- Actual transaction behavior: `is_response_valid()` returns `false` early with `response_error = CONNECTION_OPEN_FAILED`.
+- `check_response_validity()` is not called on the live path.
+- Hypothetical direct call to `check_response_validity(&server_response)`: `ResponseError_t::MISSING_STATUS_CODE`.

doc/connection_error_keepalive.md

@@ -0,0 +1,154 @@
+# Connection-Error Handling and Client Keep-Alive Shutdown
+
+This note traces the Traffic Server (TS) code path from a failed parent/origin connection through to the moment TS forces the downstream client connection into non-keepalive, and describes how the client reacts. Each section includes the exact code that participates in the decision.
+
+## 1. Detecting the Connect Failure
+
+`HttpSM::state_http_server_open` drives the non-blocking connect. Any connect timeout or socket error drops into the `VC_EVENT_*` branch below, which stamps the state and records the errno before closing the half-open `NetVConnection`:
+
+```c++
+int
+HttpSM::state_http_server_open(int event, void *data)
+{
+  ...
+  case VC_EVENT_INACTIVITY_TIMEOUT:
+  case VC_EVENT_ACTIVE_TIMEOUT:
+    t_state.set_connect_fail(ETIMEDOUT);
+  /* fallthrough */
+  case VC_EVENT_ERROR:
+  case VC_EVENT_EOS:
+  case NET_EVENT_OPEN_FAILED: {
+    t_state.current.state = HttpTransact::CONNECTION_ERROR;
+    ...
+    if (_netvc != nullptr) {
+      if (event == VC_EVENT_ERROR || event == NET_EVENT_OPEN_FAILED) {
+        t_state.set_connect_fail(_netvc->lerrno);
+      }
+      _netvc->do_io_close();
+      _netvc = nullptr;
+    }
+    ...
+  }
+  ...
+}
+```
+
+If the error happens later—while writing the request header or waiting for the response—`HttpSM::handle_server_setup_error` applies the same bookkeeping (`t_state.current.state = CONNECTION_ERROR`, `set_connect_fail`, close VC) before handing control back to Traffic Cop.
+
+## 2. Transact Logs the Failure
+
+`HttpTransact::handle_response_from_server` looks at `t_state.current.state`. When it sees `CONNECTION_ERROR` it logs the failure and either retries or bails:
+
+```c++
+void
+HttpTransact::handle_response_from_server(State *s)
+{
+  ...
+  case CONNECTION_ERROR:
+    ...
+    if (is_request_retryable(s) && s->current.retry_attempts.get() < max_connect_retries &&
+        !HttpTransact::is_response_valid(s, &s->hdr_info.server_response)) {
+      ...
+      retry_server_connection_not_open(s, s->current.state, max_connect_retries);
+      ...
+    } else {
+      error_log_connection_failure(s, s->current.state);
+      TxnDbg(..., "Error. No more retries.");
+      SET_VIA_STRING(VIA_DETAIL_SERVER_CONNECT, VIA_DETAIL_SERVER_FAILURE);
+      handle_server_connection_not_open(s);
+    }
+    break;
+  ...
+}
+```
+
+Every retry first forces the failed server session out of the keep-alive pool:
+
+```c++
+void
+HttpTransact::retry_server_connection_not_open(State *s, ServerState_t conn_state, unsigned max_retries)
+{
+  ...
+  s->current.server->keep_alive = HTTPKeepAlive::NO_KEEPALIVE;
+  s->current.retry_attempts.increment();
+}
+```
+
+## 3. Forcing the User-Agent Connection to Close
+
+When the retry budget is exhausted, `handle_server_connection_not_open` routes to `handle_parent_down` / `handle_server_down`, which build an internal error. `HttpTransact::build_error_response` deliberately disables user-agent keep-alive before those headers are emitted:
+
+```c++
+void
+HttpTransact::build_error_response(State *s, HTTPStatus status_code, const char *reason, const char *body_type)
+{
+  ...
+  if (status_code == HTTPStatus::REQUEST_TIMEOUT || s->hdr_info.client_request.get_content_length() != 0 ||
+      s->client_info.transfer_encoding == HttpTransact::TransferEncoding_t::CHUNKED) {
+    s->client_info.keep_alive = HTTPKeepAlive::NO_KEEPALIVE;
+  }
+  ...
+  if ((s->state_machine->get_ua_txn() && s->state_machine->get_ua_txn()->is_outbound_transparent()) &&
+      (status_code == HTTPStatus::INTERNAL_SERVER_ERROR || status_code == HTTPStatus::GATEWAY_TIMEOUT ||
+       status_code == HTTPStatus::BAD_GATEWAY || status_code == HTTPStatus::SERVICE_UNAVAILABLE)) {
+    s->client_info.keep_alive = HTTPKeepAlive::NO_KEEPALIVE;
+  }
+  ...
+}
+```
+
+`handle_response_keep_alive_headers` then honors that flag by inserting `Connection: close` (or `Proxy-Connection: close`) into the proxy response and calling `set_close_connection` on the client session:
+
+```c++
+void
+HttpTransact::handle_response_keep_alive_headers(State *s, HTTPVersion ver, HTTPHdr *heads)
+{
+  ...
+  if (s->client_info.keep_alive != HTTPKeepAlive::KEEPALIVE) {
+    ka_action = KA_Action_t::DISABLED;
+  }
+  ...
+  case KA_Action_t::CLOSE:
+  case KA_Action_t::DISABLED:
+    if (s->client_info.keep_alive != HTTPKeepAlive::NO_KEEPALIVE || (ver == HTTP_1_1)) {
+      if (s->client_info.proxy_connect_hdr) {
+        heads->value_set(..., "close"sv);
+      } else if (s->state_machine->get_ua_txn() != nullptr) {
+        s->state_machine->get_ua_txn()->set_close_connection(*heads);
+      }
+      s->client_info.keep_alive = HTTPKeepAlive::NO_KEEPALIVE;
+    }
+    break;
+  ...
+}
+```
+
+## 4. Client-Side Behavior
+
+On the downstream (first-layer) side, `HttpSM::tunnel_handler_ua` trusts the `keep_alive` flag. Because the earlier logic set it to `NO_KEEPALIVE`, the write-complete handler leaves `close_connection = true`, which causes the user-agent session to terminate instead of re-entering the keep-alive pool:
+
+```c++
+int
+HttpSM::tunnel_handler_ua(int event, HttpTunnelConsumer *c)
+{
+  ...
+  case VC_EVENT_WRITE_COMPLETE:
+    c->write_success          = true;
+    t_state.client_info.abort = HttpTransact::DIDNOT_ABORT;
+    if (t_state.client_info.keep_alive == HTTPKeepAlive::KEEPALIVE) {
+      ...
+      close_connection = false;
+    }
+    break;
+  ...
+}
+```
+
+Earlier, `HttpSM::do_drain_request_body` (and `set_close_connection`) already updated the downstream headers, so the first-layer ATS receives an explicit `Connection: close` and, per RFC 7230 and `Http1ClientSession::set_close_connection`, immediately closes its side instead of tracking the socket for reuse.
+
+## Summary
+
+1. Connect failure → `CONNECTION_ERROR` state (`HttpSM` sets it when the connect VC fails).
+2. `HttpTransact` logs `CONNECT: … CONNECTION_ERROR`, optionally retries, and forces the failed server session out of the pool.
+3. The generated error response clears `client_info.keep_alive`, inserts `Connection: close`, and calls `set_close_connection`.
+4. The downstream client (the first-layer ATS) sees the header, closes the socket, and drops it from its keep-alive pool—ensuring sockets used for failed multi-layer connects can’t be reused.

doc/hdrtoken-analysis.md

@@ -0,0 +1,64 @@
+# HdrToken System Analysis
+
+This document describes the purpose and inner workings of the `HdrToken` system in Traffic Server, including the algorithms used for initialization and runtime parsing.
+
+## Purpose
+
+The `HdrToken` system is an optimization designed to efficiently handle frequently occurring strings in HTTP headers, such as header names (e.g., "Content-Type") and common values (e.g., "chunked").
+
+Instead of repeatedly comparing strings, the system maps these "well-known strings" (WKS) to unique integer identifiers and stores them in a special memory heap. At runtime, incoming header strings can be quickly checked against this optimized data structure. If a match is found, the system can use the faster integer representation and canonical string pointer for internal logic instead of performing more expensive string operations.
+
+## Algorithms and Data Structures
+
+The `HdrToken` system uses different algorithms for its one-time initialization phase and its runtime parsing phase.
+
+### Initialization (`hdrtoken_init`)
+
+During server startup, the `hdrtoken_init` function is called once to set up the necessary data structures for runtime use.
+
+1.  **Well-Known Strings Lists**: Several static, compile-time arrays define the strings and their metadata:
+    *   `_hdrtoken_strs`: The master list of all well-known strings.
+    *   `_hdrtoken_strs_type_initializers`: Associates certain strings with a `HdrTokenType` (e.g., "GET" is a METHOD).
+    *   `_hdrtoken_strs_field_initializers`: Associates header field names with slot IDs, presence masks, and flags.
+
+2.  **DFA (Deterministic Finite Automaton)**:
+    *   **Algorithm**: A DFA is compiled from the master `_hdrtoken_strs` list.
+    *   **Purpose**: The DFA's sole purpose is to act as a temporary lookup mechanism during the initialization phase. It provides a robust and consistent way to find the canonical index of a string from the metadata arrays (`_hdrtoken_strs_type_initializers`, etc.) within the master `_hdrtoken_strs` array. This is crucial for correctly linking metadata to the right string before the primary runtime data structures are built. For example, it's used to find the index of "GET" so its `HdrTokenType` can be set to `METHOD`. The DFA is configured to be case-insensitive. After initialization is complete, the DFA is **not** used for parsing headers at runtime.
+
+3.  **Specialized Memory Heap**:
+    *   All well-known strings from the master list are copied into a single contiguous block of memory (a dedicated heap). Each string in the heap is prefixed with a `HdrTokenHeapPrefix` struct that stores its length, index, and other metadata.
+    *   This allows for a very fast runtime check: if a string's pointer falls within the address range of this heap, it is instantly identifiable as a well-known string without needing a hash or comparison.
+
+4.  **Hash Table (`hdrtoken_hash_table`)**:
+    *   **Algorithm**: FNV-1a (Fowler–Noll–Vo) hash.
+    *   **Purpose**: This is the primary data structure for fast lookups of unknown strings at runtime. After the heap is populated, the `hdrtoken_hash_init` function iterates through the well-known strings, calculates the FNV-1a hash for each one, and stores the canonical string pointer (from the heap) and its hash in the `hdrtoken_hash_table`.
+
+### Runtime Parsing and Tokenization
+
+When the server is running, headers are parsed and strings are "tokenized" (converted to a WKS representation) on the fly. This process is handled by the `hdrtoken_tokenize` function.
+
+1.  **Fast Path (Pointer Check)**: The function first checks if the string pointer `hdrtoken_is_wks()` is already a known token by seeing if its address falls within the specialized memory heap. If it is, the string has already been tokenized, and the function immediately returns its index.
+
+2.  **Slow Path (Hash Lookup)**: If the string is not already a known token pointer (i.e., it's a fresh string from the network), the following occurs:
+    *   **Algorithm**: FNV-1a hash and direct memory comparison.
+    *   The FNV-1a hash of the input string is calculated.
+    *   This hash is used to find a potential match in the `hdrtoken_hash_table`.
+    *   A three-part check is performed to validate the match:
+        1.  The hash bucket must contain a valid string (`bucket->wks != nullptr`).
+        2.  The hash of the string in the bucket must match the hash of the input string (`bucket->hash == hash`).
+        3.  The length of the string in the bucket must match the length of the input string (`hdrtoken_wks_to_length(bucket->wks) == string_len`).
+
+This final length check is critical. The combination of a matching hash and matching length provides a very strong guarantee of a **whole-string match**. It prevents a shorter string like "Accept" from incorrectly matching a longer string like "Accept-Ranges".
+
+If all checks pass, the string has been successfully "tokenized". The function returns the WKS index. Other parts of the system can then use `hdrtoken_index_to_wks(index)` to get a pointer to the canonical, shared string from the WKS heap. If the checks fail, the string is not a well-known token.
+
+## Sequence of Steps for Parsing a Header String
+
+1.  An unknown header string is passed to `hdrtoken_tokenize`.
+2.  A quick check (`hdrtoken_is_wks`) is performed to see if the string's memory address is already in the well-known string heap. If yes, the token is found, and the process ends.
+3.  If not, the string's FNV-1a hash is calculated.
+4.  A bucket in the `hdrtoken_hash_table` is located using the hash.
+5.  The hash stored in the bucket is compared to the input string's hash.
+6.  The length of the string in the bucket is compared to the input string's length.
+7.  If both the hash and the length match, the string is successfully tokenized. Its WKS index is returned for use by the caller.
+8.  If any of these checks fail, the string is not a well-known token.

doc/release-notes/upgrading.en.rst

@@ -30,6 +30,24 @@ with :cpp:func:`TSPortDescriptorDestroy`. The descriptor can be destroyed
 immediately after :cpp:func:`TSPortDescriptorAccept` returns because the
 listener does not retain it.
 
+Plugins
+-------
+
+Changes to Features
+~~~~~~~~~~~~~~~~~~~
+The following plugins have been changed in this version of ATS.
+
+* rate_limit - The YAML configuration is now validated strictly:
+
+  * An unknown key at any level makes the configuration fail to load. Correct any
+    misspelled key, such as ``max-age`` in place of ``max_age``.
+  * An invalid value, such as a non-numeric ``limit``, also fails the load.
+  * An empty configuration file is an error. Write ``selector: []`` to load the
+    plugin with no rules.
+
+  A failed reload keeps the previous configuration active. For more details, please
+  check :ref:`admin-plugins-rate-limit`.
+
 Upgrading to ATS v10.x
 ======================
 

plugins/experimental/rate_limit/ip_reputation.cc

@@ -81,6 +81,10 @@ SieveLru::hasher(const std::string &ip, u_short family) // Mostly a convenience
 bool
 SieveLru::parseYaml(const YAML::Node &node)
 {
+  if (!validate_yaml_keys(node, "ip-rep", {"name", "buckets", "size", "percentage", "max_age", "perma-block"})) {
+    return false;
+  }
+
   if (node["buckets"]) {
     _num_buckets = node["buckets"].as<uint32_t>();
   }
@@ -100,21 +104,20 @@ SieveLru::parseYaml(const YAML::Node &node)
   if (node["perma-block"]) {
     const YAML::Node &perma = node["perma-block"];
 
-    if (perma.IsMap()) {
-      if (perma["limit"]) {
-        _permablock_limit = perma["limit"].as<uint32_t>();
-      }
+    if (!validate_yaml_keys(perma, "perma-block", {"limit", "threshold", "max_age"})) {
+      return false;
+    }
 
-      if (perma["threshold"]) {
-        _permablock_threshold = perma["threshold"].as<uint32_t>();
-      }
+    if (perma["limit"]) {
+      _permablock_limit = perma["limit"].as<uint32_t>();
+    }
 
-      if (perma["max_age"]) {
-        _permablock_max_age = std::chrono::seconds(perma["max_age"].as<uint32_t>());
-      }
-    } else {
-      TSError("[%s] The perma-block node must be a map", PLUGIN_NAME);
-      return false;
+    if (perma["threshold"]) {
+      _permablock_threshold = perma["threshold"].as<uint32_t>();
+    }
+
+    if (perma["max_age"]) {
+      _permablock_max_age = std::chrono::seconds(perma["max_age"].as<uint32_t>());
     }
   }