Skip to content

Add Time.new specs for strings ending where a colon is expected - #1393

Open
aminmansuri wants to merge 1 commit into
ruby:masterfrom
aminmansuri:time-new-string-truncated-error-cases
Open

Add Time.new specs for strings ending where a colon is expected#1393
aminmansuri wants to merge 1 commit into
ruby:masterfrom
aminmansuri:time-new-string-truncated-error-cases

Conversation

@aminmansuri

Copy link
Copy Markdown

core/time/new_spec.rb covers the Time.new(String) parse errors thoroughly, but
every error-case string in it ends with a trailing zone suffix — +09:00, +0900
or Z. CRuby's own TestTime#test_new_from_string does the same. So no test
exercises a string that ends exactly at the byte where the parser expects a :.

That byte matters. These messages are formatted with %.*s and a length of one past
the consumed region, so they normally include the byte that failed the check —
"missing sec part: 00:56 " keeps its trailing space, "fraction min is not supported: 00:56." keeps its dot. When the string ends at that position there is no
such byte, the precision runs into the string's terminator, and the message stops
short: "missing sec part: 00:56". That form was untested.

Three examples for it:

input bytes message
"2020-12-25 00:56" 16 missing sec part: 00:56
"2020-12-25T00:56" 16 missing sec part: 00:56
"2020-12-25 00" 13 missing min part: 00

One byte shorter ("2020-12-25 00:5") and the two-digit check fires first; one byte
longer ("2020-12-25 00:56Z") and there is a byte to report. The boundary is exact.

The expectations are anchored with \A...\z, as several neighbouring examples in the
file already are, so they genuinely distinguish the truncated message from the form
that carries a trailing byte. They keep the usual |can't parse: alternative for
implementations that raise the generic error.

CRuby raises exactly these messages today. JRuby 10 does not — it reads one byte past
the end of the string while building the message, and an
ArrayIndexOutOfBoundsException escapes in place of the ArgumentError. Reported as
jruby/jruby#9623, fix in jruby/jruby#9624; until that ships, JRuby will need to tag
this example.

The omission was spotted by @sampokuokkanen while reviewing jruby/jruby#9624 — thanks
for noticing that everything in the existing set carries a zone suffix.

Every other error-case string in core/time/new_spec.rb carries a trailing
zone suffix, so no example ended at the byte where the parser expects a
':'. That is the one position where the message stops short of the
offending byte, and it was unexercised.

The gap turned up while investigating jruby/jruby#9623, where these three
inputs read past the end of the string instead of raising ArgumentError.
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