diff --git a/CHANGES b/CHANGES index 4dfae08c1..88ec9bf84 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,12 @@ +v3.0.18 - to be released +------------------------ + + - fix: DURATION now measures elapsed wall-clock time (monotonic + std::chrono::steady_clock) instead of process CPU time, and is reported + as an integer number of microseconds, like v2. It also works on macOS + and Windows. Together with #3687 this changes the value that rules see + [PR #3721 - @Nikhil-jaiswal007] + v3.0.17 - 2026-Sep-29 --------------------- diff --git a/headers/modsecurity/transaction.h b/headers/modsecurity/transaction.h index 5a98c1e33..89b622683 100644 --- a/headers/modsecurity/transaction.h +++ b/headers/modsecurity/transaction.h @@ -433,9 +433,10 @@ class Transaction : public TransactionAnchoredVariables, public TransactionSecMa * need to be filled if there is no rule using the variable * `duration'. * - * @note Must be a floating point type: utils::cpu_seconds() returns a - * double and storing it in an integer type (as the former clock_t - * did) truncated the fractional seconds, making `duration' wrong. + * @note Holds a utils::steady_micros() timestamp: a monotonic + * (std::chrono::steady_clock) time in whole microseconds. It is + * kept as a double so the layout of this public class does not + * change; a double represents integers exactly up to 2^53. */ const double m_creationTimeStamp; diff --git a/src/transaction.cc b/src/transaction.cc index 792ac0bd7..9b54917b1 100644 --- a/src/transaction.cc +++ b/src/transaction.cc @@ -118,7 +118,7 @@ Transaction::Transaction(ModSecurity *ms, RulesSet *rules, const char *id, void Transaction::Transaction(ModSecurity *ms, RulesSet *rules, const char *id, void *logCbData, const time_t timestamp) - : m_creationTimeStamp(utils::cpu_seconds()), + : m_creationTimeStamp(utils::steady_micros()), m_ARGScombinedSizeDouble(0), m_clientPort(0), m_highestSeverityAction(255), diff --git a/src/utils/system.cc b/src/utils/system.cc index 830b34243..10aa159cb 100644 --- a/src/utils/system.cc +++ b/src/utils/system.cc @@ -28,6 +28,7 @@ #include #include +#include #include #include #include @@ -95,6 +96,14 @@ double cpu_seconds(void) { return 0; } +double steady_micros(void) { + // Monotonic wall-clock time, in whole microseconds. A double holds + // integers exactly up to 2^53, so no precision is lost. + return static_cast( + std::chrono::duration_cast( + std::chrono::steady_clock::now().time_since_epoch()).count()); +} + std::string find_resource(const std::string& resource, const std::string& config, std::string *err) { diff --git a/src/utils/system.h b/src/utils/system.h index d6b0adf63..5b6dff192 100644 --- a/src/utils/system.h +++ b/src/utils/system.h @@ -27,6 +27,7 @@ namespace utils { double cpu_seconds(void); +double steady_micros(void); std::string find_resource(const std::string& file, const std::string& config, std::string *err); std::string get_path(const std::string& file); diff --git a/src/variables/duration.cc b/src/variables/duration.cc index cf3dea6d9..b7f4a0047 100644 --- a/src/variables/duration.cc +++ b/src/variables/duration.cc @@ -15,6 +15,7 @@ #include "src/variables/duration.h" +#include #include #include #include @@ -30,7 +31,11 @@ namespace variables { void Duration::evaluate(Transaction *transaction, RuleWithActions *rule, std::vector *l) { - double e = utils::cpu_seconds() - transaction->m_creationTimeStamp; + // Elapsed wall-clock time since the transaction was created, in whole + // microseconds (same unit and type as v2: apr_time_now() - request_time). + const double now = utils::steady_micros(); + const int64_t e = static_cast( + now - transaction->m_creationTimeStamp); transaction->m_variableDuration.assign(std::to_string(e)); @@ -40,4 +45,4 @@ void Duration::evaluate(Transaction *transaction, } // namespace variables -} // namespace modsecurity +} // namespace modsecurity \ No newline at end of file diff --git a/test/test-cases/regression/issue-1725.json b/test/test-cases/regression/issue-1725.json index 092c8862e..ba99189b0 100644 --- a/test/test-cases/regression/issue-1725.json +++ b/test/test-cases/regression/issue-1725.json @@ -37,7 +37,7 @@ ] }, "expected": { - "debug_log": "/test; 0.[0-9]+; 0.[0-9]+;", + "debug_log": "/test; [0-9]+; [0-9]+;", "http_code": 200 }, "rules": [ diff --git a/test/test-cases/regression/variable-DURATION.json b/test/test-cases/regression/variable-DURATION.json index adcc6583c..0816a9466 100644 --- a/test/test-cases/regression/variable-DURATION.json +++ b/test/test-cases/regression/variable-DURATION.json @@ -38,7 +38,7 @@ ] }, "expected": { - "debug_log": "Target value: \"[0-9\\.]+\"", + "debug_log": "Target value: \"[0-9]+\"", "http_code": 200 }, "rules": [ @@ -47,5 +47,48 @@ "SecRule DURATION \"@contains test \" \"id:2,phase:3,pass,t:trim\"", "SecRule DURATION \"@contains test \" \"id:3,phase:3,pass,t:trim\"" ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Testing Variables :: DURATION is a non-zero integer", + "client": { + "ip": "200.249.12.31", + "port": 2313 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "User-Agent": "curl/7.38.0", + "Accept": "*/*", + "Content-Length": "0" + }, + "uri": "/test", + "method": "GET", + "http_version": 1.1, + "body": [ + "" + ] + }, + "response": { + "headers": { + "Content-Length": "0" + }, + "body": [ + "" + ] + }, + "expected": { + "debug_log": "duration=[1-9][0-9]*;", + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecRule DURATION \"@unconditionalMatch\" \"phase:2,id:10002,log,auditlog,pass,msg:'duration=%{DURATION};',logdata:'duration=%{DURATION};'\"" + ] } -] +] \ No newline at end of file