[NUAAwqy]

Which issue(s) this PR fixes:

None — self-found while auditing Fluent::Timezone.

What this PR does / why we need it:

NUMERIC_PATTERN in lib/fluent/timezone.rb is documented as [+-]HH:MM / [+-]HHMM / [+-]HH, but it was written as [+-]\d\d(:?\d\d)?\z, which puts no bound on either field. validate only compares against that pattern, so a mistyped zone is accepted by every setting that takes one: timezone, the parsers' time:… zone spec, timekey_zone, and in_tail's path_timezone.

Time.zone_offset then adds the fields instead of reading them, so minutes ≥ 60 carry into the hours:

timezone	what fluentd did
+09:60 / +0960	accepted as a spelling of +10:00; TimeFormatter.new(..., "+09:60").format(0) → 1970-01-01T10:00:00+1000 — a %z that contradicts the time printed next to it
+00:75	accepted as +01:15
+99:99 / +24:00	validation passes, the plugin starts, and the first formatted record raises ArgumentError: utc_offset out of range (Time#localtime accepts `

So minutes ≥ 60 mean the plugin runs and silently emits hour-shifted data, while hours ≥ 24 turn a two-line regex check into a per-record runtime crash — both shapes come from an offset that is not a time of day.

Red before the fix (the new test file against unpatched lib): 4 tests, 60 assertions, 3 failures — +09:60 should be invalid, <36000> was expected to be nil, and the formatter case.

The fix: require HH in 00..23 and MM in 00..59:

NUMERIC_PATTERN = %r{\A[+-](?:[01]\d|2[0-3])(?::?[0-5]\d)?\z}

The bound is not arbitrary: the largest offset the pattern allows, ±23:59 = 86340 s, is inside the |utc_offset| ≤ 86399 that Time#localtime accepts, so the accepted set is exactly the set of numeric offsets Ruby can apply. validate, validate!, utc_offset and formatter all branch on this constant, so an out-of-range offset now takes the existing "invalid timezone" path (validate! → ConfigError: Unsupported timezone '+09:60'); none of them needed touching.

Validation (all on one machine, TZ=UTC, Ruby 3.3.6):

check	result
new test/test_timezone.rb (new file — lib/fluent/timezone.rb had no test coverage at all)	4 tests, 88 assertions, 0 failures, 0 errors (3 failures red)
exhaustive census: all 40,000 [+-]HH(:?)MM strings, both spellings, both signs, checked against Time.zone_offset and Time#localtime	new pattern keeps 5,760 = 24 × 60 × 2 × 2, all resolving to the offset they spell; drops 34,240 that the old pattern accepted — 3,680 silent hour shifts and 30,560 offsets that raise as soon as anything is formatted with them
regression slice: test_time_formatter 77, test_time_parser 30, test_event_time 22, test_formatter 36, test_mixin 21, plugin/test_out_file 67, plugin/test_parser 23, plugin/test_parser_json 66, plugin_helper/test_extract 14, plugin_helper/test_compat_parameters 10	all 0 failures, 0 errors (370 tests)
rubocop 1.92.1 + rubocop-performance 1.27.0, 2 changed files (and whole repo)	no offenses

Two files in the wider timezone area are red at the base commit in this environment (plugin/test_out_secondary_file 3 failures, plugin_helper/test_inject 2 hostname auto-detection failures) and stay identical with the fix reverted — environment-shaped, not regressions.

Deliberate behaviour change to flag: a config that today starts and then emits hour-shifted data (or crashes per record) now fails at load with ConfigError: Unsupported timezone '+09:60'. That is the point — but an operator who was relying on +09:60 meaning +10:00 sees a startup error instead of wrong timestamps. If maintainers prefer to keep such values working, the alternative is to normalise them, which is what silently corrupting them already does. The new test file is additive; if it fits the repo better elsewhere, the four tests move as-is (they only need require 'fluent/timezone').

Docs Changes:

None needed — the doc-comment in the module is extended to state the range.

Release Note:

timezone: reject out-of-range offsets and unsupported zones at configure time

• Numeric offsets with out-of-range fields, such as +09:60 and +24:00, now fail with ConfigError. They used to shift timestamps or raise ArgumentError on each record.
• <inject> and <extract> now reject unsupported timezones such as UTC, JST and misspelled region names. Use Etc/UTC, Asia/Tokyo or a numeric offset such as +09:00 instead.

[Watson1978]

Thanks for the fix. The numeric bounds look correct, but some callers do not validate the timezone before using it. On those paths, this change causes invalid offsets to fall back silently or produce nil values instead of raising a configuration error.

I reproduced the following on 7f94b6a and on master (086fa17).

1. <inject> and <extract>

plugin_helper/inject.rb and plugin_helper/extract.rb copy TIME_PARAMETERS, but they never call validate!. For an out-of-range offset, Timezone.formatter and Timezone.utc_offset now return nil. TimeFormatter and TimeParser then fall back to the host timezone.

Conditions: TZ=America/New_York, time_key t and time_type string. <inject> uses time_format %Y-%m-%dT%H:%M:%S%z and event time 0. <extract> uses time_format %Y-%m-%d %H:%M:%S and the record {"t":"1970-01-02 00:00:00"}.

config	master	this PR
<inject> timezone +09:60	1970-01-01T10:00:00+1000	1969-12-31T19:00:00-0500
<inject> timezone +24:00	ArgumentError on every record	1969-12-31T19:00:00-0500
<extract> timezone +0960	50400	104400

For <inject>, +24:00 previously raised an error during formatting. After this change, it silently falls back to the host timezone. For <extract>, the event time itself changes.

2. types with time: in parsers

Parser#build_type_converters uses validate, not validate!. When it returns false, the whole option is used as the time format. So types {"date":"time:+0960:%Y-%m-%d %H:%M:%S"} still configures. For example, "2026-10-26 12:00:00" becomes nil without any warning. On master it was parsed as +10:00.

What I think is needed

• MUST: Reject out-of-range numeric timezones during configuration in both helpers. Calling Fluent::Timezone.validate! from their configure methods would address this, as TimeMixin::TimeParameters#configure does.
• MUST: Reject out-of-range numeric timezones in types during configuration. Raising ConfigError in build_type_converters when the zone part looks like a numeric offset would address this.
• MUST: Add tests that go through these helpers and the parser, not only through Fluent::Timezone.

MUST items block the merge. The SHOULD and NIT inline comments on the boundary wording do not.

[NUAAwqy]

Thanks for the careful review — you're right that tightening the pattern moved the failure from "wrong offset" to "silently ignored" on the paths that never validate. Addressed each point in 34c84a9.

MUST 1 — <inject> / <extract>. Fluent::Timezone.validate!(@inject_config.timezone) if @inject_config.timezone at the top of Inject#configure (lib/fluent/plugin_helper/inject.rb#L110) and the symmetric line in Extract#configure (lib/fluent/plugin_helper/extract.rb#L84), matching what TimeMixin::TimeParameters#configure does. Both helpers are the only places that copy TimeMixin::TIME_PARAMETERS (grep -rn TIME_PARAMETERS lib/), so the two lines cover the whole set.

MUST 2 — types with time:. In build_type_converters, when validate fails and the zone part starts with a sign followed by a digit, raise instead of re-reading the whole option as the time format (lib/fluent/plugin/parser.rb#L255):

raise Fluent::ConfigError, "Unsupported timezone '#{timezone}' for key:'#{field_name}'"

A strftime format never starts with [+-] + a digit, so the existing fallback is untouched for time:%Y-%m-%d %H:%M:%S, for time:unixtime / time:float, and for named zones — all three stay covered by tests.

MUST 3 — tests through the helpers and the parser. 8 new tests, and each of the four rejection cases fails on the previous head with "exception was expected but none was thrown", so they pin the behaviour you asked for rather than the pattern:

• test/plugin_helper/test_inject.rb — timezone +09:60 and timezone +24:00 now raise ConfigError at configure time (they were 1969-12-31T19:00:00-0500, and on master ArgumentError on every record); +10:00 still formats 2014-09-27T10:00:00+1000, so the neighbours of the rejected values keep working.
• test/plugin_helper/test_extract.rb — timezone +0960 raises (it was 104400 instead of 50400); Asia/Tokyo still parses.
• test/plugin/test_parser.rb — time:+0960:%Y-%m-%d %H:%M:%S raises (it converted every value to nil); time:+0100:… still yields +0100 and time:%Y-%m-%d %H:%M:%S still parses in the host zone.

Both inline suggestions are applied verbatim, and the stale "exactly the range Time can apply" claim is gone from test/test_timezone.rb too.

Measured on TZ=America/New_York with the ruby33 toolchain: test/test_timezone.rb 4 tests / 88 assertions, test/plugin_helper/test_extract.rb 16, test/plugin/test_parser.rb 26, test/plugin_helper/test_inject.rb 45 with only the two pre-existing injects hostname automatically detected failures that reproduce identically on pristine master (host-name auto-detect on this machine). rubocop repo-wide: 472 files, no offenses.

One path I did not touch, to keep this PR to the paths you reported: Fluent::Compat::HandleTagAndTimeMixin#configure builds a TimeFormatter from its own timezone config param without validating, so it has the same silent fallback. Say the word if you'd like it in here, or leave it as deprecated compat.

[NUAAwqy]

Thanks for the careful review — you're right that tightening the pattern moved the failure from "wrong offset" to "silently ignored" on the paths that never validate. Addressed each point in 34c84a9.

MUST 1 — <inject> / <extract>. Fluent::Timezone.validate!(@inject_config.timezone) if @inject_config.timezone at the top of Inject#configure (lib/fluent/plugin_helper/inject.rb#L110) and the symmetric line in Extract#configure (lib/fluent/plugin_helper/extract.rb#L84), matching what TimeMixin::TimeParameters#configure does. Both helpers are the only places that copy TimeMixin::TIME_PARAMETERS (grep -rn TIME_PARAMETERS lib/), so the two lines cover the whole set.

MUST 2 — types with time:. In build_type_converters, when validate fails and the zone part starts with a sign followed by a digit, raise instead of re-reading the whole option as the time format (lib/fluent/plugin/parser.rb#L255):

raise Fluent::ConfigError, "Unsupported timezone '#{timezone}' for key:'#{field_name}'"

A strftime format never starts with [+-] + a digit, so the existing fallback is untouched for time:%Y-%m-%d %H:%M:%S, for time:unixtime / time:float, and for named zones — all three stay covered by tests.

MUST 3 — tests through the helpers and the parser. 8 new tests, and each of the four rejection cases fails on the previous head with "exception was expected but none was thrown", so they pin the behaviour you asked for rather than the pattern:

• test/plugin_helper/test_inject.rb — timezone +09:60 and timezone +24:00 now raise ConfigError at configure time (they were 1969-12-31T19:00:00-0500, and on master ArgumentError on every record); +10:00 still formats 2014-09-27T10:00:00+1000, so the neighbours of the rejected values keep working.
• test/plugin_helper/test_extract.rb — timezone +0960 raises (it was 104400 instead of 50400); Asia/Tokyo still parses.
• test/plugin/test_parser.rb — time:+0960:%Y-%m-%d %H:%M:%S raises (it converted every value to nil); time:+0100:… still yields +0100 and time:%Y-%m-%d %H:%M:%S still parses in the host zone.

Both inline suggestions are applied verbatim, and the stale "exactly the range Time can apply" claim is gone from test/test_timezone.rb too.

Measured on TZ=America/New_York with the ruby33 toolchain: test/test_timezone.rb 4 tests / 88 assertions, test/plugin_helper/test_extract.rb 16, test/plugin/test_parser.rb 26, test/plugin_helper/test_inject.rb 45 with only the two pre-existing injects hostname automatically detected failures that reproduce identically on pristine master (host-name auto-detect on this machine). rubocop repo-wide: 472 files, no offenses.

One path I did not touch, to keep this PR to the paths you reported: Fluent::Compat::HandleTagAndTimeMixin#configure builds a TimeFormatter from its own timezone config param without validating, so it has the same silent fallback. Say the word if you'd like it in here, or leave it as deprecated compat.

[Watson1978]

Thanks for the update. The <inject>, <extract> and types cases from my previous review now raise ConfigError, and the new tests fail on the previous head. I confirmed both.

One problem remains in the new check in build_type_converters. Details are inline.

MUST items block the merge. The SHOULD and NIT items do not.

We will update the release note on our side. It will mention that <inject> and <extract> now reject any unsupported timezone, such as UTC.

[Watson1978]

We will not backport this to v1.19, to keep compatibility with existing configurations. We will reconsider if a concrete need for a backport comes up.

[NUAAwqy]

Thanks for the detailed review. All inline items are addressed in f45d7c2:

• The check now matches the whole value (/\A[+-]\d+\z/.match?(timezone)), so types {"date":"time:+0000 %Y-%m-%d %H:%M:%S"} parses as on master, and types {"date":"time:"} no longer raises NoMethodError.
• Added regression tests for both cases in test/plugin/test_parser.rb.
• Updated the out-of-range fallback comment as suggested.

The comment updates you suggested in the first review (lib/fluent/timezone.rb and test/test_timezone.rb) were already included in 34c84a9.

[Watson1978]

Thanks

[Watson1978]

The failed tests require #5583 fixes to pass.

[NUAAwqy]

Thanks for merging, and for the clear review process! For the record: the windows-11-arm job failures on the final head were the pre-existing test breakage noted in this thread (the fixes land via #5583) — not related to this change. Happy to help if anything comes up around the timezone validation.