[I] cache.config: Exact `url=` rules are dropped after successful parsing [trafficserver]

["icpd (via GitHub)" <gi...@apache.org>]

icpd opened a new issue, #13421:
URL: https://github.com/apache/trafficserver/issues/13421

   ## Description
   
   `cache.config` accepts `url=` as a primary destination. A valid rule passes parsing, but `UrlMatcher` does not add it to the URL hash table. The configuration loads without an error, and the rule never matches a request.
   
   The insertion code is in `UrlMatcher<Data, Result>::NewEntry()` in `src/proxy/ControlMatcher.cc`:
   
   ```cpp
   cur_d = data_array + num_el;
   error = cur_d->Init(line_info);
   
   if (error.failed()) {
     url_str[num_el]   = ats_strdup(pattern);
     url_value[num_el] = num_el;
     url_ht.emplace(url_str[num_el], url_value[num_el]);
     num_el++;
   }
   
   return error;
   ```
   
   `error.failed()` is `true` when initialization fails. This condition is backwards: a successfully initialized rule skips the insertion into `url_ht`, while the function still returns success.
   
   ## Expected behavior
   
   When `cur_d->Init(line_info)` succeeds, ATS should add the rule to `url_ht`. The rule's cache action should apply when a request URL exactly matches the configured value.
   
   ## Cause
   
   The exact URL matcher was added for TS-1280. The original code used a string pointer for errors and inserted the rule only after successful initialization:
   
   ```cpp
   errBuf = cur_d->Init(line_info);
   
   if (errBuf == NULL) {
     // Insert into url_ht.
   }
   ```
   
   Commit `70cc2d4ce1762b182dd52f43a6313c61772c3a8a` replaced the error string with `config_parse_error`. That type evaluates to `true` when an error is present, but the same change made the `UrlMatcher` condition `if (error)`, reversing the old success check.
   
   Commit `39f8e5c057386bffd51f5ba2284d2e715543e100` later changed the condition to `if (error.failed())` without correcting the reversed logic.
   
   Related commits:
   
   - [[Original implementation](https://github.com/apache/trafficserver/commit/2d9ad48c5075b40ebfd864b1e9ce4f097d84b85c)](https://github.com/apache/trafficserver/commit/2d9ad48c5075b40ebfd864b1e9ce4f097d84b85c)
   - [[Commit that introduced the regression](https://github.com/apache/trafficserver/commit/70cc2d4ce1762b182dd52f43a6313c61772c3a8a)](https://github.com/apache/trafficserver/commit/70cc2d4ce1762b182dd52f43a6313c61772c3a8a)
   - [Conversion to `Result`](https://github.com/apache/trafficserver/commit/39f8e5c057386bffd51f5ba2284d2e715543e100)
   
   ## Suggested fix
   
   ```diff
    error = cur_d->Init(line_info);
   
   -if (error.failed()) {
   +if (!error.failed()) {
      url_str[num_el]   = ats_strdup(pattern);
      url_value[num_el] = num_el;
      url_ht.emplace(url_str[num_el], url_value[num_el]);
      num_el++;
    }
   ```
   
   Regression tests should cover:
   
   - A valid `url=` rule is inserted and matches the complete URL.
   - A different URL does not match the rule.
   - A failed `Init()` call does not insert a rule and returns a configuration error.
   - A duplicate `url=` value still returns the duplicate-rule error.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficserver.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] cache.config: Exact `url=` rules are dropped after successful parsing [trafficserver]

["bneradt (via GitHub)" <gi...@apache.org>]

bneradt closed issue #13421: cache.config: Exact `url=` rules are dropped after successful parsing
URL: https://github.com/apache/trafficserver/issues/13421


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org