Skip to content

Fix 12am and 12pm being swapped in parseATTime - #2925

Open
dylanpulver wants to merge 1 commit into
graphite-project:masterfrom
dylanpulver:fix/12am-12pm
Open

Fix 12am and 12pm being swapped in parseATTime#2925
dylanpulver wants to merge 1 commit into
graphite-project:masterfrom
dylanpulver:fix/12am-12pm

Conversation

@dylanpulver

Copy link
Copy Markdown

webapp/graphite/render/attime.py:84,90,96 maps an am hour straight through and a pm hour through (hour + 12) % 24. That is correct for 1 through 11 and inverted at 12:

12am    -> 2020-03-15T12:00:00+00:00     (expected 00:00)
12:30am -> 2020-03-15T12:30:00+00:00     (expected 00:30)
12pm    -> 2020-03-15T00:00:00+00:00     (expected 12:00)
12:30pm -> 2020-03-15T00:30:00+00:00     (expected 12:30)

so &from=12am&until=12pm graphs noon → midnight — a different 12-hour window, silently, with no error.

docs/render_api.rst:188 calls the format at(1)-compatible, and POSIX at(1) defines midnight as "the time 12:00 am (00:00)" and noon as "the time 12:00 pm". The same module already resolves noon to 12:00 and midnight to 00:00, so today one request can get two different times for the same instant depending on which spelling is used.

Measured with parseATTime(ref, pytz.utc, now=2020-03-15 09:30) over midnight, 12am, 12:30am, 1am, 11am, noon, 12pm, 12:30pm, 1pm, 11pm, teatime: 4/11 wrong before, 0/11 after; the seven controls pass on both sides.

webapp/tests/test_attime.py never used hour 12 — 8:50am, 8:50pm, 8am, 10pm — and 12 is the only hour where (h + 12) % 24 is wrong, so the whole suite passed. Four cases added, in both parseTimeReferenceTest and parseTimeReferenceTestNow.

Mutants: reverting the source leaves exactly the 6 new tests failing and nothing else; fixing only the bare Xam/Xpm branches leaves 12:30am/12:30pm failing; fixing only the pm side leaves the three 12am cases failing.

manage.py test, Python 3.13 (in the CI matrix): 859 tests / 18 errors on clean master, 865 tests / 18 errors with this change — the 18 are pre-existing module-import errors from optional dependencies I don't have installed (whisper, ceres, rrdtool, redis), identical on both sides.

Not tested: any hour above 12 with an am/pm suffix. (h + 12) % 24 and h % 12 + 12 disagree there (13pm was 01:00, is now 13:00), but POSIX at restricts wallclock_hour to 1–12 so both are garbage-in. I also have not run the mysql/postgresql tox targets or anything on Windows.

Written with Claude Code. On provenance: this did not come out of a production incident. I was checking attime.py's hour handling against the at(1) semantics the docs claim, hour by hour, and 12 is where it breaks.

attime.py mapped an am hour straight through and a pm hour through
(hour + 12) % 24, which is right for 1 through 11 and inverted at 12:
"12am" parsed to 12:00 and "12pm" to 00:00. The same module already
resolves "noon" to 12:00 and "midnight" to 00:00, so a single request
using both spellings got two different times for the same instant.

render_api.rst documents the format as at(1)-compatible, and POSIX at(1)
defines midnight as "the time 12:00 am (00:00)" and noon as "the time
12:00 pm".

Co-Authored-By: Claude <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant