[bugfix] Reject huge dayTimeDuration values in date/time arithmetic#6301
[bugfix] Reject huge dayTimeDuration values in date/time arithmetic#6301joewiz wants to merge 3 commits into
Conversation
Adding a sufficiently large xs:dayTimeDuration to an xs:date,
xs:dateTime, or xs:time would lock up the JVM thread for hours of
CPU because the underlying Xerces XMLGregorianCalendar.add()
iterates per day. The bug was reported with
`current-date() + xs:dayTimeDuration("P1712073600000D")`, which
loops for over an hour before producing a result.
Pre-validate the duration's day-time magnitude in
OrderedDurationValue.addDurationToDate() and raise FODT0001
immediately when it exceeds 1,000,000 Gregorian years. This caps
worst-case latency at well under one second on modern hardware
while permitting any realistic date/time arithmetic. Xerces' add()
is unchanged for inputs below the threshold, preserving its tested
correctness for premodern, BCE, and year-zero-adjacent dates.
Adds JUnit regression cases in DateTimeTest covering plus/minus on
xs:dateTime, plus XQSuite cases in dateTimeOverflow.xqm covering
the xs:date / xs:dateTime / xs:time entry points. Existing 217
date/time JUnit tests and 1015 XQuery3Tests pass.
Closes eXist-db#5045
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…alue Behaviour-preserving cleanup of pre-existing PMD findings on the files touched by the eXist-dbGH-5045 bugfix above: DurationValue.java: - AvoidReassigningParameters (4x): nullIfZero / zeroIfNull helpers return the value directly via ternary instead of reassigning the parameter. - OneDeclarationPerLine: split combined BigInteger declaration in canonicalize() into one declaration per line. - SimplifyBooleanReturns (2x): equals() and durationsAreEqual() use short-circuit boolean expressions instead of if-return-true/false. - Convert getPart(), convertTo(), and compareTo(Collator, Comparison, AtomicValue) from break/return switch statements to switch expressions. The convertTo branches that produced YearMonthDuration and DayTimeDuration are extracted into toYearMonthDurationValue() and toDayTimeDurationValue() helpers; the EQ/NEQ duplication in compareTo is collapsed into durationsAreEqual(). - Fix a pre-existing typo in the convertTo() default-case error message ("Type error: cannot cast ' + ... 'to" had no string concatenation). OrderedDurationValue.java: - AvoidThrowingRawExceptionTypes: replace throw new RuntimeException(...) with IllegalStateException, since the condition (a totally-ordered duration pair reporting INDETERMINATE) is a programming-state invariant violation. - NPathComplexity (was 288, threshold 200): decompose compareTo(Collator, Comparison, AtomicValue) into compareMixedSubtypes, validateOrderingComparable, and resultMatchesOperator helpers. The outer method now reads as a sequence of guards followed by the final comparison, with each helper carrying its own narrow responsibility. No behaviour change: 1391 date / time / duration / XPath / XQuery3 tests pass with no diffs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| // The pre-validation guard must reject them with FODT0001 fast. | ||
|
|
||
| @Test | ||
| public void plus_hugeDayTimeDuration_rejectsFast() throws XPathException { |
There was a problem hiding this comment.
Address Codacy issue The JUnit 4 test method name 'plus_hugeDayTimeDuration_rejectsFast' doesn't match '[a-z][a-zA-Z0-9]*'
| } | ||
|
|
||
| @Test | ||
| public void minus_hugeDayTimeDuration_rejectsFast() throws XPathException { |
There was a problem hiding this comment.
Address Codacy issue The JUnit 4 test method name 'plus_hugeDayTimeDuration_rejectsFast' doesn't match '[a-z][a-zA-Z0-9]*'
| } | ||
|
|
||
| @Test | ||
| public void plus_largeButPermittedDayTimeDuration_works() throws XPathException { |
There was a problem hiding this comment.
Address Codacy issue The JUnit 4 test method name 'plus_hugeDayTimeDuration_rejectsFast' doesn't match '[a-z][a-zA-Z0-9]*'
line-o
left a comment
There was a problem hiding this comment.
I am in favour of this change and this limitation should be documented.
… camelCase per Codacy Codacy's JUnit naming convention requires test method names to match [a-z][a-zA-Z0-9]*. Rename the three eXist-dbGH-5045 test methods accordingly: - plus_hugeDayTimeDuration_rejectsFast -> plusHugeDayTimeDurationRejectsFast - minus_hugeDayTimeDuration_rejectsFast -> minusHugeDayTimeDurationRejectsFast - plus_largeButPermittedDayTimeDuration_works -> plusLargeButPermittedDayTimeDurationWorks No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
[This response was co-authored with Claude Code. -Joe] Done — renamed the three GH-5045 test methods to camelCase ( |
Summary
Closes #5045.
xs:date + xs:dayTimeDuration("P1712073600000D")(and similar forxs:dateTime/xs:time) used to lock up the JVM thread for over an hour of CPU. The XMLGregorianCalendar.add() implementation underneath iterates per day, so very large day-time durations cause the call to take seconds, then minutes, then hours.This PR adds a pre-validation guard in
OrderedDurationValue.addDurationToDate()that rejects day-time magnitudes exceeding 1,000,000 Gregorian years withFODT0001, before delegating togc.add(). The arithmetic engine itself is unchanged for any in-range input, preserving its tested correctness for premodern, BCE, and year-zero-adjacent dates.Empirical motivation
XMLGregorianCalendar.add(Duration)latency on Zulu 21 / Apple M-series, measured before this PR:P1000000DP100000000DP1000000000DP10000000000DP100000000000DP1712073600000D(the reported case)xs:yearMonthDurationis not affected — Xerces handles those in O(1) (sub-ms even at 1.2 billion months). Only the dayTime portion of a duration triggers the slow path, so the guard is conditioned onsecondsValueSigned()magnitude.What changed
exist-core/src/main/java/org/exist/xquery/value/DurationValue.javaDATE_ARITH_DAYS_LIMIT(1,000,000 Gregorian years) andMAX_DATE_ARITH_SECONDS. NewcheckDateArithMagnitude(BigDecimal)helper that raisesFODT0001if the duration's day-time magnitude exceeds the threshold.exist-core/src/main/java/org/exist/xquery/value/OrderedDurationValue.javacheckDateArithMagnitude(secondsValueSigned())) inaddDurationToDate(), between the input year check andgc.add(duration).exist-core/src/test/java/org/exist/xquery/value/DateTimeTest.javaFODT0001in <1 s; 100,000-year duration must still succeed.exist-core/src/test/xquery/xquery3/dateTimeOverflow.xqmxs:date,xs:dateTime, andxs:time.Threshold rationale: 1,000,000 Gregorian years caps worst-case latency at ~365 ms on the test hardware while permitting any realistic date/time arithmetic. The threshold is a hard-coded constant; per XQuery 3.1 §10.1.1 the duration value-space is implementation-defined.
Spec references
Test plan
mvn test -pl exist-core -Dtest='Date*Test,Time*Test,Duration*Test,XPathQueryTest'— 376 tests pass (4 pre-existing skips)mvn test -pl exist-core -Dtest='xquery.xquery3.XQuery3Tests'— 1015 tests pass, including the four new [BUG] Adding a large number of days to a date takes a very long time #5045 cases indateTimeOverflow.xqmcodacy-cli analyze --tool pmdon changed files — no new findings (pre-existing findings unchanged)mvn license:check -pl exist-core -o— cleanmvn install -pl exist-core -am -DskipTests🤖 Generated with Claude Code