ishaaaan17 commented on code in PR #13934:
URL: https://github.com/apache/trafficserver/pull/13934#discussion_r4415339975
##########
plugins/regex_remap/regex_remap.cc:
##########
@@ -655,47 +656,45 @@ compile_rule_set(std::string const &filename, std::string
const &source)
}
pos1 = line.find_first_not_of(" \t\n");
- if (pos1 != std::string::npos) {
- if (line[pos1] == '#') {
- continue;
- }
+ if (pos1 == std::string::npos || line[pos1] == '#') {
+ continue;
+ }
Review Comment:
yes, this was intentional. previously, completely empty lines (`""`) were
already skipped silently via `if (line.empty()) continue;`, but lines
containing only whitespace (e.g. spaces, tabs, or `\r`) had `pos1 ==
std::string::npos`, fell through, and logged `no regexp found`. because the old
code used `continue;`, existing configurations containing whitespace-only lines
were still loaded despite the logged error.
under the new fatal error semantics (`return nullptr;`), treating a
whitespace-only line as a missing regex rule would reject any configuration
file containing blank lines with spaces or trailing carriage returns. treating
`pos1 == std::string::npos` the same as `line.empty()` avoids rejecting
otherwise valid configurations.
i have updated the pr description to document this behavior explicitly. if
the project prefers whitespace-only lines to be treated strictly as syntax
errors and reject the configuration instead, i am happy to adjust!
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
