Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

time: Parse incorrectly parses fractional seconds with more than 9 digits #48685

Closed
jtbandes opened this issue Sep 29, 2021 · 5 comments
Closed

time: Parse incorrectly parses fractional seconds with more than 9 digits #48685

jtbandes opened this issue Sep 29, 2021 · 5 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@jtbandes
Copy link

jtbandes commented Sep 29, 2021 •
edited

What version of Go are you using (go version)?

1.17.1 (play.golang.org)

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

play.golang.org

What did you do?

https://play.golang.org/p/S21r1Xz1VlZ

package main

import (
	"fmt"
	"time"
)

func main() {
	t, err := time.Parse(time.RFC3339, "2021-09-29T16:04:33.00123456789Z")
	fmt.Println(t, t.Nanosecond(), err)
	t, err = time.Parse(time.RFC3339, "2021-09-29T16:04:33.10123456789Z")
	fmt.Println(t, t.Nanosecond(), err)
}

What did you expect to see?

either fractional seconds truncated to nanosecond precision:

2021-09-29 16:04:33.001234567 +0000 UTC 1234567 <nil>
2021-09-29 16:04:33.101234567 +0000 UTC 101234567 <nil>

or fractional seconds rounded to nearest nanosecond:

2021-09-29 16:04:33.001234568 +0000 UTC 1234568 <nil>
2021-09-29 16:04:33.101234568 +0000 UTC 101234568 <nil>

or an error in both cases.

What did you see instead?

2021-09-29 16:04:33.123456789 +0000 UTC 123456789 <nil>
0001-01-01 00:00:00 +0000 UTC 0 parsing time "2021-09-29T16:04:33.10123456789Z": fractional second out of range

It appears that if more than 9 digits are provided for the fractional second part, they are interpreted all together as a numeric value for the nanoseconds part, regardless of leading zeros. Thus .0012345678Z is interpreted the same as .012345678Z.

Also, when more than 9 significant digits are provided, rather than truncating or rounding to the nearest nanosecond, parsing produces a fractional second out of range error because this error condition is triggered:

go/src/time/format.go

Lines 1426 to 1428 in 163871f

if ns < 0 || 1e9 <= ns {
rangeErrString = "fractional second"
return

This bug affects json.Unmarshal / time.UnmarshalJSON which uses the RFC3339 format internally:

go/src/time/time.go

Line 1305 in 163871f

*t, err = Parse(`"`+RFC3339+`"`, string(data))

The code at fault is probably one or more of these call sites of parseNanoseconds:

go/src/time/format.go

Lines 1082 to 1086 in 163871f

// No fractional second in the layout but we have one in the input.
n := 2
for ; n < len(value) && isDigit(value, n); n++ {
}
nsec, rangeErrString, err = parseNanoseconds(value, n)

go/src/time/format.go

Lines 1194 to 1201 in 163871f

// stdFracSecond0 requires the exact number of digits as specified in
// the layout.
ndigit := 1 + digitsLen(std)
if len(value) < ndigit {
err = errBad
break
}
nsec, rangeErrString, err = parseNanoseconds(value, ndigit)

go/src/time/format.go

Lines 1209 to 1215 in 163871f

// Take any number of digits, even more than asked for,
// because it is what the stdSecond case would do.
i := 0
for i < 9 && i+1 < len(value) && '0' <= value[i+1] && value[i+1] <= '9' {
i++
}
nsec, rangeErrString, err = parseNanoseconds(value, 1+i)

@jtbandes jtbandes mentioned this issue Sep 29, 2021
Merged
AlexanderYastrebov added a commit to AlexanderYastrebov/go that referenced this issue Oct 3, 2021
@AlexanderYastrebov
time: Truncates fractional seconds longer than 9 digits
086a242 
Fixes Fixes golang#48685
@AlexanderYastrebov
Copy link
Contributor

The problem is parseNanoseconds that is parsing a value less than 1 as an integer:

go/src/time/format.go

Lines 1418 to 1438 in 163871f

func parseNanoseconds(value string, nbytes int) (ns int, rangeErrString string, err error) {
if !commaOrPeriod(value[0]) {
err = errBad
return
}
if ns, err = atoi(value[1:nbytes]); err != nil {
return
}
if ns < 0 || 1e9 <= ns {
rangeErrString = "fractional second"
return
}
// We need nanoseconds, which means scaling by the number
// of missing digits in the format, maximum length 10. If it's
// longer than 10, we won't scale.
scaleDigits := 10 - nbytes
for i := 0; i < scaleDigits; i++ {
ns *= 10
}
return
}

E.g. time.Parse(time.RFC3339, "2021-09-29T16:04:33.1000000000Z") produces error while it is just 0.1 second
https://play.golang.org/p/BAQYrNkxktg

AlexanderYastrebov added a commit to AlexanderYastrebov/go that referenced this issue Oct 3, 2021
@AlexanderYastrebov
time: truncates fractional seconds longer than 9 digits
527deb5 
Fixes golang#48685
AlexanderYastrebov added a commit to AlexanderYastrebov/go that referenced this issue Oct 3, 2021
@AlexanderYastrebov
time: truncate fractional seconds longer than 9 digits
7bda6bd 
Fixes golang#48685
AlexanderYastrebov added a commit to AlexanderYastrebov/go that referenced this issue Oct 3, 2021
@AlexanderYastrebov
time: truncate fractional seconds longer than 9 digits
99bb5d7 
Fixes golang#48685
AlexanderYastrebov added a commit to AlexanderYastrebov/go that referenced this issue Oct 3, 2021
@AlexanderYastrebov
time: truncate fractional seconds longer than 9 digits
4d98519 
Fixes golang#48685
@AlexanderYastrebov AlexanderYastrebov mentioned this issue Oct 3, 2021
Closed
@gopherbot
Copy link

Change https://golang.org/cl/353713 mentions this issue: time: truncate fractional seconds longer than 9 digits

@mknyszek mknyszek changed the title time.Parse incorrectly parses fractional seconds with more than 9 digits time: Parse incorrectly parses fractional seconds with more than 9 digits Oct 4, 2021
@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 4, 2021
@mknyszek mknyszek added this to the Backlog milestone Oct 4, 2021
@mknyszek
Copy link
Contributor

mknyszek commented Oct 4, 2021

CC @rsc via https://dev.golang.org/owners

@myitcv myitcv mentioned this issue Jan 25, 2022
Closed
@gopherbot
Copy link

Change https://golang.org/cl/381155 mentions this issue: time: document that Parse truncates to nanosecond precision

gopherbot pushed a commit that referenced this issue Feb 15, 2022
@ianlancetaylor
time: document that Parse truncates to nanosecond precision
293ecd8 
For #48685
Fixes #50806

Change-Id: Ie8be40e5794c0998538890a651ef8ec92cb72d3a
Reviewed-on: https://go-review.googlesource.com/c/go/+/381155
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Paul Jolly <paul@myitcv.org.uk>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
@gopherbot
Copy link

Change https://go.dev/cl/425037 mentions this issue: time: fix Parse to ignore extra sub-nanosecond digits

gopherbot pushed a commit that referenced this issue Aug 23, 2022
@dsnet @gopherbot
time: fix Parse to ignore extra sub-nanosecond digits
4e9183c 
This modifies the code to match the comment such that
the behavior truly is identical to stdSecond case.
Also, it modifies the behavior to match the documented
behavior where:

    Fractional seconds are truncated to nanosecond precision.

Fixes #54567
Updates #48685

Change-Id: Ie64549e4372ab51624c105ad8ab4cc99b9b5a0b3
Reviewed-on: https://go-review.googlesource.com/c/go/+/425037
Run-TryBot: Joseph Tsai <joetsai@digital-static.net>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Joseph Tsai <joetsai@digital-static.net>
@golang golang locked and limited conversation to collaborators Aug 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

Successfully merging a pull request may close this issue.

time: truncate fractional seconds longer than 9 digits AlexanderYastrebov/go
4 participants
@jtbandes @AlexanderYastrebov @mknyszek @gopherbot