From: Eduard Bagdasaryan Date: Thu, 20 Feb 2020 21:13:27 +0000 (+0000) Subject: High precision time units (#443) X-Git-Tag: 4.15-20210522-snapshot~164 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=8da861a50ed63acecd111b3810b1a2c806aa8b68;p=thirdparty%2Fsquid.git High precision time units (#443) This patch introduces new time units of microsecond and nanosecond precision forming a new 'time-units-small' category. Also found and fixed several problems, related to time parameters parsing: * Obscure "integer overflow" fatal messages. For example, passing "0.0001 second" caused this message. After fixing, Squid reports that the value "is too small to be used in this context". * Ignoring possible zero-rounded values after parsing. For example, if a second-precision parameter was configured with 0.1 second, it became zero after rounding, which is unexpected. It is treated as a fatal error now. * Inconsistent parameter overflow type. For example, parameters with millisecond and second precision reported that 'time_msec_t' overflowed. Now we introduce an absolute time maximum allowed, equal to the maximum of chrono::nanoseconds type which is about 293 years. This absolute maximum allows to keep the time parsing code simple and at the same time should satisfy any reasonable configuration need. Note that this solution treats existing configurations with unreasonably huge time values > 293 years as fatal errors, such configurations should be fixed accordingly. * Time overflows for icap_service_failure_limit parameter were not checked at all. This is probably a result of code duplication. By fixing the latter problem, the former one was resolved automatically. * Unclear fatal message if a time parameter lacked time unit. Now Squid reports about "missing time unit". * Improved error reporting when an inapplicable time unit was used, for example a 'millisecond' instead of a 'second'. For the majority of time parameters, it reported only a common "FATAL: Bungled..." message. For url_rewrite_timeout parameter, it reported an irrelevant "unsupported option ..." message (since it began to treat the faulty time unit as the next option). Now in both cases it reports about the underlying time unit problem. While fixing these bugs I had to refactor and improve time parsing functions, using safer std::chrono types instead of raw integer types. --- diff --git a/src/cache_cf.cc b/src/cache_cf.cc index d69c6de6a9..26c8ca6b52 100644 --- a/src/cache_cf.cc +++ b/src/cache_cf.cc @@ -92,6 +92,7 @@ #if HAVE_GLOB_H #include #endif +#include #include #include #if HAVE_PWD_H @@ -137,6 +138,8 @@ static void free_ecap_service_type(Adaptation::Ecap::Config *); static peer_t parseNeighborType(const char *s); +static const char *const T_NANOSECOND_STR = "nanosecond"; +static const char *const T_MICROSECOND_STR = "microsecond"; static const char *const T_MILLISECOND_STR = "millisecond"; static const char *const T_SECOND_STR = "second"; static const char *const T_MINUTE_STR = "minute"; @@ -155,6 +158,9 @@ static const char *const B_GBYTES_STR = "GB"; static const char *const list_sep = ", \t\n\r"; +// std::chrono::years requires C++20. Do our own rough calculation for now. +static const double HoursPerYear = 24*365.2522; + static void parse_access_log(CustomLog ** customlog_definitions); static int check_null_access_log(CustomLog *customlog_definitions); static void dump_access_log(StoreEntry * entry, const char *name, CustomLog * definitions); @@ -163,7 +169,6 @@ static bool setLogformat(CustomLog *cl, const char *name, const bool dieWhenMiss static void configDoConfigure(void); static void parse_refreshpattern(RefreshPattern **); -static uint64_t parseTimeUnits(const char *unit, bool allowMsec); static void parse_u_short(unsigned short * var); static void parse_string(char **); static void default_all(void); @@ -539,9 +544,17 @@ parseOneConfigFile(const char *file_name, unsigned int depth) /* Handle includes here */ if (tmp_line_len >= 9 && strncmp(tmp_line, "include", 7) == 0 && xisspace(tmp_line[7])) { err_count += parseManyConfigFiles(tmp_line + 8, depth + 1); - } else if (!parse_line(tmp_line)) { - debugs(3, DBG_CRITICAL, HERE << cfg_filename << ":" << config_lineno << " unrecognized: '" << tmp_line << "'"); - ++err_count; + } else { + try { + if (!parse_line(tmp_line)) { + debugs(3, DBG_CRITICAL, ConfigParser::CurrentLocation() << ": unrecognized: '" << tmp_line << "'"); + ++err_count; + } + } catch (...) { + // fatal for now + debugs(3, DBG_CRITICAL, "configuration error: " << CurrentException); + self_destruct(); + } } } @@ -1044,88 +1057,134 @@ parse_obsolete(const char *name) } } -/* Parse a time specification from the config file. Store the - * result in 'tptr', after converting it to 'units' */ -static time_msec_t -parseTimeLine(const char *units, bool allowMsec, bool expectMoreArguments = false) -{ - time_msec_t u = parseTimeUnits(units, allowMsec); - if (u == 0) { - self_destruct(); - return 0; - } - - char *token = ConfigParser::NextToken(); - if (!token) { - self_destruct(); - return 0; +template +static const char * +TimeUnitToString() +{ + const auto minUnit = MinimalUnit(1); + if(minUnit == std::chrono::nanoseconds(1)) + return T_NANOSECOND_STR; + else if (minUnit == std::chrono::microseconds(1)) + return T_MICROSECOND_STR; + else if (minUnit == std::chrono::milliseconds(1)) + return T_MILLISECOND_STR; + else { + assert(minUnit >= std::chrono::seconds(1)); + return T_SECOND_STR; } +} - double d = xatof(token); +/// Assigns 'ns' the number of nanoseconds corresponding to 'unitName'. +/// \param MinimalUnit is a chrono duration type specifying the minimal +/// allowed time unit. +/// \returns true if unitName is correct and its time unit is not less +/// than MinimalUnit. +template +static bool +parseTimeUnit(const char *unitName, std::chrono::nanoseconds &ns) +{ + if (!unitName) + throw TexcHere("missing time unit"); + + if (!strncasecmp(unitName, T_NANOSECOND_STR, strlen(T_NANOSECOND_STR))) + ns = std::chrono::nanoseconds(1); + else if (!strncasecmp(unitName, T_MICROSECOND_STR, strlen(T_MICROSECOND_STR))) + ns = std::chrono::microseconds(1); + else if (!strncasecmp(unitName, T_MILLISECOND_STR, strlen(T_MILLISECOND_STR))) + ns = std::chrono::milliseconds(1); + else if (!strncasecmp(unitName, T_SECOND_STR, strlen(T_SECOND_STR))) + ns = std::chrono::seconds(1); + else if (!strncasecmp(unitName, T_MINUTE_STR, strlen(T_MINUTE_STR))) + ns = std::chrono::minutes(1); + else if (!strncasecmp(unitName, T_HOUR_STR, strlen(T_HOUR_STR))) + ns = std::chrono::hours(1); + else if (!strncasecmp(unitName, T_DAY_STR, strlen(T_DAY_STR))) + ns = std::chrono::hours(24); + else if (!strncasecmp(unitName, T_WEEK_STR, strlen(T_WEEK_STR))) + ns = std::chrono::hours(24 * 7); + else if (!strncasecmp(unitName, T_FORTNIGHT_STR, strlen(T_FORTNIGHT_STR))) + ns = std::chrono::hours(24 * 14); + else if (!strncasecmp(unitName, T_MONTH_STR, strlen(T_MONTH_STR))) + ns = std::chrono::hours(24 * 30); + else if (!strncasecmp(unitName, T_YEAR_STR, strlen(T_YEAR_STR))) + ns = std::chrono::hours(static_cast(HoursPerYear)); + else if (!strncasecmp(unitName, T_DECADE_STR, strlen(T_DECADE_STR))) + ns = std::chrono::hours(static_cast(HoursPerYear * 10)); + else + return false; - time_msec_t m = u; /* default to 'units' if none specified */ + if (ns < MinimalUnit(1)) { + throw TexcHere(ToSBuf("time unit '", unitName, "' is too small to be used in this context, the minimal unit is ", + TimeUnitToString())); + } - if (d) { - if ((token = ConfigParser::PeekAtToken()) && (m = parseTimeUnits(token, allowMsec))) { - (void)ConfigParser::NextToken(); + return true; +} - } else if (!expectMoreArguments) { - self_destruct(); - return 0; +static std::chrono::nanoseconds +CheckTimeValue(const double value, const std::chrono::nanoseconds &unit) +{ + if (value < 0) + throw TexcHere("time must have a positive value"); - } else { - token = NULL; // show default units if dying below - debugs(3, DBG_CRITICAL, "WARNING: No units on '" << config_input_line << "', assuming " << d << " " << units); - } - } else - token = NULL; // show default units if dying below. + const auto maxNanoseconds = std::chrono::nanoseconds::max().count(); + if (value > maxNanoseconds/static_cast(unit.count())) { + const auto maxYears = maxNanoseconds/(HoursPerYear*3600*1000000000); + throw TexcHere(ToSBuf("time values cannot exceed ", maxYears, " years")); + } - const auto result = static_cast(m * d); + return std::chrono::nanoseconds(static_cast(unit.count() * value)); +} - if (static_cast(result) * 2 != m * d * 2) { - debugs(3, DBG_CRITICAL, "FATAL: Invalid value '" << - d << " " << (token ? token : units) << ": integer overflow (time_msec_t)."); - self_destruct(); +template +static TimeUnit +FromNanoseconds(const std::chrono::nanoseconds &ns, const double parsedValue) +{ + const auto result = std::chrono::duration_cast(ns); + if (!result.count()) { + throw TexcHere(ToSBuf("time value '", parsedValue, + "' is too small to be used in this context, the minimal value is 1 ", + TimeUnitToString())); } return result; } -static uint64_t -parseTimeUnits(const char *unit, bool allowMsec) +/// Parses a time specification from the config file and +/// returns the time as a chrono duration object of 'TimeUnit' type. +template +static TimeUnit +parseTimeLine() { - if (allowMsec && !strncasecmp(unit, T_MILLISECOND_STR, strlen(T_MILLISECOND_STR))) - return 1; - - if (!strncasecmp(unit, T_SECOND_STR, strlen(T_SECOND_STR))) - return 1000; - - if (!strncasecmp(unit, T_MINUTE_STR, strlen(T_MINUTE_STR))) - return 60 * 1000; + const auto valueToken = ConfigParser::NextToken(); + if (!valueToken) + throw TexcHere("cannot read a time value"); - if (!strncasecmp(unit, T_HOUR_STR, strlen(T_HOUR_STR))) - return 3600 * 1000; + const auto parsedValue = xatof(valueToken); - if (!strncasecmp(unit, T_DAY_STR, strlen(T_DAY_STR))) - return 86400 * 1000; + if (parsedValue == 0) + return TimeUnit::zero(); - if (!strncasecmp(unit, T_WEEK_STR, strlen(T_WEEK_STR))) - return 86400 * 7 * 1000; + std::chrono::nanoseconds parsedUnitDuration; - if (!strncasecmp(unit, T_FORTNIGHT_STR, strlen(T_FORTNIGHT_STR))) - return 86400 * 14 * 1000; + const auto token = ConfigParser::PeekAtToken(); - if (!strncasecmp(unit, T_MONTH_STR, strlen(T_MONTH_STR))) - return static_cast(86400) * 30 * 1000; + if (!parseTimeUnit(token, parsedUnitDuration)) + throw TexcHere(ToSBuf("unknown time unit '", token, "'")); - if (!strncasecmp(unit, T_YEAR_STR, strlen(T_YEAR_STR))) - return static_cast(86400 * 1000 * 365.2522); + (void)ConfigParser::NextToken(); - if (!strncasecmp(unit, T_DECADE_STR, strlen(T_DECADE_STR))) - return static_cast(86400 * 1000 * 365.2522 * 10); + const auto nanoseconds = CheckTimeValue(parsedValue, parsedUnitDuration); - debugs(3, DBG_IMPORTANT, "parseTimeUnits: unknown time unit '" << unit << "'"); + // validate precisions (time-units-small only) + if (TimeUnit(1) <= std::chrono::microseconds(1)) { + if (0 < nanoseconds.count() && nanoseconds.count() < 3) { + debugs(3, DBG_PARSE_NOTE(DBG_IMPORTANT), ConfigParser::CurrentLocation() << ": WARNING: " << + "Squid time measurement precision is likely to be far worse than " << + "the nanosecond-level precision implied by the configured value: " << parsedValue << ' ' << token); + } + } - return 0; + return FromNanoseconds(nanoseconds, parsedValue); } static void @@ -2983,8 +3042,11 @@ dump_time_t(StoreEntry * entry, const char *name, time_t var) void parse_time_t(time_t * var) { - time_msec_t tval = parseTimeLine(T_SECOND_STR, false); - *var = static_cast(tval/1000); + const auto maxTime = std::numeric_limits::max(); + const auto seconds = parseTimeLine(); + if (maxTime < seconds.count()) + throw TexcHere(ToSBuf("directive supports time values up to ", maxTime, " but is given ", seconds.count(), " seconds")); + *var = static_cast(seconds.count()); } static void @@ -3005,7 +3067,7 @@ dump_time_msec(StoreEntry * entry, const char *name, time_msec_t var) void parse_time_msec(time_msec_t * var) { - *var = parseTimeLine(T_SECOND_STR, true); + *var = parseTimeLine().count(); } static void @@ -3014,6 +3076,27 @@ free_time_msec(time_msec_t * var) *var = 0; } +#if UNUSED_CODE +// TODO: add a parameter with 'time_nanoseconds' TYPE and uncomment +static void +dump_time_nanoseconds(StoreEntry *entry, const char *name, const std::chrono::nanoseconds &var) +{ + storeAppendPrintf(entry, "%s %" PRId64 " nanoseconds\n", name, var.count()); +} + +static void +parse_time_nanoseconds(std::chrono::nanoseconds *var) +{ + *var = parseTimeLine(); +} + +static void +free_time_nanoseconds(std::chrono::nanoseconds *var) +{ + *var = std::chrono::nanoseconds::zero(); +} +#endif + #if UNUSED_CODE static void dump_size_t(StoreEntry * entry, const char *name, size_t var) @@ -4419,8 +4502,6 @@ dump_ecap_service_type(StoreEntry * entry, const char *name, const Adaptation::E static void parse_icap_service_failure_limit(Adaptation::Icap::Config *cfg) { char *token; - time_t d; - time_t m; cfg->service_failure_limit = GetInteger(); if ((token = ConfigParser::NextToken()) == NULL) @@ -4432,27 +4513,7 @@ static void parse_icap_service_failure_limit(Adaptation::Icap::Config *cfg) return; } - if ((token = ConfigParser::NextToken()) == NULL) { - self_destruct(); - return; - } - - d = static_cast (xatoi(token)); - - m = static_cast (1); - - if (0 == d) - (void) 0; - else if ((token = ConfigParser::NextToken()) == NULL) { - debugs(3, DBG_CRITICAL, "No time-units on '" << config_input_line << "'"); - self_destruct(); - return; - } else if ((m = parseTimeUnits(token, false)) == 0) { - self_destruct(); - return; - } - - cfg->oldest_service_failure = (m * d); + parse_time_t(&cfg->oldest_service_failure); } static void dump_icap_service_failure_limit(StoreEntry *entry, const char *name, const Adaptation::Icap::Config &cfg) @@ -4884,11 +4945,45 @@ static void free_ftp_epsv(acl_access **ftp_epsv) FtpEspvDeprecated = false; } +/// Like parseTimeLine() but does not require the timeunit to be specified. +/// If missed, the default 'second' timeunit is assumed. +static std::chrono::seconds +ParseUrlRewriteTimeout() +{ + const auto timeValueToken = ConfigParser::NextToken(); + if (!timeValueToken) + throw TexcHere("cannot read a time value"); + + using Seconds = std::chrono::seconds; + + const auto parsedTimeValue = xatof(timeValueToken); + + if (parsedTimeValue == 0) + return std::chrono::seconds::zero(); + + std::chrono::nanoseconds parsedUnitDuration; + + const auto unitToken = ConfigParser::PeekAtToken(); + if (parseTimeUnit(unitToken, parsedUnitDuration)) + (void)ConfigParser::NextToken(); + else { + const auto defaultParsed = parseTimeUnit(T_SECOND_STR, parsedUnitDuration); + assert(defaultParsed); + debugs(3, DBG_PARSE_NOTE(DBG_IMPORTANT), ConfigParser::CurrentLocation() << + ": WARNING: missing time unit, using deprecated default '" << T_SECOND_STR << "'"); + } + + const auto nanoseconds = CheckTimeValue(parsedTimeValue, parsedUnitDuration); + + return FromNanoseconds(nanoseconds, parsedTimeValue); +} + static void parse_UrlHelperTimeout(SquidConfig::UrlHelperTimeout *config) { - const auto tval = parseTimeLine(T_SECOND_STR, false, true); - Config.Timeout.urlRewrite = static_cast(tval/1000); + // TODO: do not allow optional timeunit (as the documentation prescribes) + // and use parseTimeLine() instead. + Config.Timeout.urlRewrite = ParseUrlRewriteTimeout().count(); char *key, *value; while(ConfigParser::NextKvPair(key, value)) { diff --git a/src/cf.data.pre b/src/cf.data.pre index 88b9dd4de5..8b8f0db672 100644 --- a/src/cf.data.pre +++ b/src/cf.data.pre @@ -55,6 +55,24 @@ COMMENT_START MB - Megabyte GB - Gigabyte + Values with time units + + Time-related directives marked with either "time-units" or + "time-units-small" accept a time unit. The supported time units are: + + nanosecond (time-units-small only) + microsecond (time-units-small only) + millisecond + second + minute + hour + day + week + fortnight + month - 30 days + year - 31557790080 milliseconds (just over 365 days) + decade + Values with spaces, quotes, and other special characters Squid supports directive parameters with spaces, quotes, and other diff --git a/test-suite/squidconf/time_units b/test-suite/squidconf/time_units new file mode 100644 index 0000000000..34f3a48da0 --- /dev/null +++ b/test-suite/squidconf/time_units @@ -0,0 +1,57 @@ +## Copyright (C) 1996-2019 The Squid Software Foundation and contributors +## +## Squid software is distributed under GPLv2+ license and includes +## contributions from numerous individuals and organizations. +## Please see the COPYING and CONTRIBUTORS files for details. + +# +# Tests for directives with time units +# + +# a millisecond-precision parameter (time-units) + +# minimum checks +dns_timeout 1 millisecond +dns_timeout 0.001 second + +# maximum checks +dns_timeout 9223372036854 milliseconds +dns_timeout 9223372036 seconds +dns_timeout 153722867 minutes +dns_timeout 2562047 hours +dns_timeout 106751 days +dns_timeout 15250 weeks +dns_timeout 7625 fortnights +dns_timeout 3558 months +dns_timeout 292.27 years +dns_timeout 29.227 decades + + +# a second-precision parameter (time-units) + +# minimum checks +max_stale 1 second +max_stale 0.0167 minute + +# maximum checks +max_stale 9223372036 seconds +max_stale 153722867 minutes +max_stale 2562047 hours +max_stale 106751 days +max_stale 15250 weeks +max_stale 7625 fortnights +max_stale 3558 months +max_stale 292.27 years +max_stale 29.227 decades + +# a multiple-options parameter + +url_rewrite_timeout 1 second on_timeout=bypass +url_rewrite_timeout 0.001 year on_timeout=bypass +url_rewrite_timeout 292 year on_timeout=bypass + +# should be a WARNING (-k parse) +url_rewrite_timeout 1 on_timeout=bypass + +# TODO: a nanosecond-precison parameter (time-units-small) +