[go] encoding/json: fix Decoder.InputOffset regression in goexperiment.jsonv2
6 views
Skip to first unread message
Joseph Tsai (Gerrit)
Sep 15, 2025, 3:58:07 PMSep 15
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Joseph Tsai has uploaded the change for review![]( "Open in Gerrit")
Commit message
encoding/json: fix Decoder.InputOffset regression in goexperiment.jsonv2
The Decoder.InputOffset method was always ambiguous about the exact
offset returned since anything between the end of the previous token
to the start of the next token is a valid result.
Empirically, it seems that the behavior was to report
the end of the previous token unless Decoder.More is called,
in which case it reports the start of the next token.
This is an odd semantic since a relatively side-effect free method
like More is not quite so side-effect free.
However, our goal is to preserve historical v1 semantic when possible
regardless of whether they made sense.
Note that jsontext.Decoder.InputOffset consistently always reports
the end of the previous token. Users can explicitly choose the
position they want by introspecting the UnreadBuffer.
Fixes #75468
Change-Id: I1e946e83c9d29dfc09f2913ff8d6b2b80632f292
Change diff
diff --git a/src/encoding/json/v2_stream.go b/src/encoding/json/v2_stream.go
index 28e72c0..37569da 100644
--- a/src/encoding/json/v2_stream.go
+++ b/src/encoding/json/v2_stream.go
@@ -20,6 +20,10 @@
dec *jsontext.Decoder
opts jsonv2.Options
err error
+
+ // hadPeeked reports whether [Decoder.More] was called.
+ // It is reset by [Decoder.Decode] and [Decoder.Token].
+ hadPeeked bool
}
// NewDecoder returns a new decoder that reads from r.
@@ -66,6 +70,7 @@
if dec.err != nil {
return dec.err
}
+ dec.hadPeeked = false
b, err := dec.dec.ReadValue()
if err != nil {
dec.err = transformSyntacticError(err)
@@ -192,6 +197,7 @@
// to mark the start and end of arrays and objects.
// Commas and colons are elided.
func (dec *Decoder) Token() (Token, error) {
+ dec.hadPeeked = false
tok, err := dec.dec.ReadToken()
if err != nil {
// Historically, v1 would report just [io.EOF] if
@@ -230,6 +236,7 @@
// More reports whether there is another element in the
// current array or object being parsed.
func (dec *Decoder) More() bool {
+ dec.hadPeeked = true
k := dec.dec.PeekKind()
return k > 0 && k != ']' && k != '}'
}
@@ -238,5 +245,18 @@
// The offset gives the location of the end of the most recently returned token
// and the beginning of the next token.
func (dec *Decoder) InputOffset() int64 {
- return dec.dec.InputOffset()
+ offset := dec.dec.InputOffset()
+ if dec.hadPeeked {
+ // Historically, InputOffset reported the location of
+ // the end of the most recently returned token
+ // unless [Decoder.More] is called, in which case, it reported
+ // the beginning of the next token.
+ unreadBuffer := dec.dec.UnreadBuffer()
+ trailingTokens := bytes.TrimLeft(unreadBuffer, " \n\r\t")
+ if len(trailingTokens) > 0 {
+ leadingWhitespace := len(unreadBuffer) - len(trailingTokens)
+ offset += int64(leadingWhitespace)
+ }
+ }
+ return offset
}
diff --git a/src/encoding/json/v2_stream_test.go b/src/encoding/json/v2_stream_test.go
index b8951f8..26aa85d 100644
--- a/src/encoding/json/v2_stream_test.go
+++ b/src/encoding/json/v2_stream_test.go
@@ -537,3 +537,64 @@
}
}
}
+
+func TestDecoderInputOffset(t *testing.T) {
+ const input = ` [ [ ] , [ "one" ] , [ "one" , "two" ] , { } , { "alpha" : "bravo" } , { "alpha" : "bravo" , "fizz" : "buzz" } ] `
+ wantOffsets := []int64{
+ 0, 1, 2, 3, 4, 5, 6, 7, 10, 11, 16, 17, 18, 19, 22, 23, 28, 29,
+ 36, 37, 38, 39, 42, 43, 44, 45, 48, 49, 56, 57, 66, 67, 68, 69,
+ 72, 73, 80, 81, 90, 91, 99, 100, 108, 109, 110, 111, 112, 112,
+ 112, 112,
+ }
+ wantMores := []bool{
+ true, true, false, true, true, false, true, true, true, false,
+ true, false, true, true, true, false, true, true, true, true,
+ true, false, false, false, false,
+ }
+
+ d := NewDecoder(strings.NewReader(input))
+ checkOffset := func() {
+ t.Helper()
+ got := d.InputOffset()
+ if len(wantOffsets) == 0 {
+ t.Fatalf("InputOffset = %d, want nil", got)
+ }
+ want := wantOffsets[0]
+ if got != want {
+ t.Fatalf("InputOffset = %d, want %d", got, want)
+ }
+ wantOffsets = wantOffsets[1:]
+ }
+ checkMore := func() {
+ t.Helper()
+ got := d.More()
+ if len(wantMores) == 0 {
+ t.Fatalf("More = %v, want nil", got)
+ }
+ want := wantMores[0]
+ if got != want {
+ t.Fatalf("More = %v, want %v", got, want)
+ }
+ wantMores = wantMores[1:]
+ }
+ checkOffset()
+ checkMore()
+ checkOffset()
+ for {
+ if _, err := d.Token(); err == io.EOF {
+ break
+ } else if err != nil {
+ t.Fatalf("Token error: %v", err)
+ }
+ checkOffset()
+ checkMore()
+ checkOffset()
+ }
+ checkOffset()
+ checkMore()
+ checkOffset()
+
+ if len(wantOffsets)+len(wantMores) > 0 {
+ t.Fatal("unconsumed testdata")
+ }
+}
Change information
Files:
- M src/encoding/json/v2_stream.go
- M src/encoding/json/v2_stream_test.go
Change size: M
Delta: 2 files changed, 82 insertions(+), 1 deletion(-)
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joseph Tsai (Gerrit)
Sep 15, 2025, 3:58:22 PMSep 15
to goph...@pubsubhelper.golang.org, Damien Neil, golang-co...@googlegroups.com
Attention needed from Damien Neil
Joseph Tsai voted Commit-Queue+1![]( "Open in Gerrit")
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joseph Tsai (Gerrit)
Sep 15, 2025, 4:00:54 PMSep 15
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Damien Neil and Joseph Tsai
Joseph Tsai uploaded new patchset![]( "Open in Gerrit")
Attention is currently required from:
- Damien Neil
- Joseph Tsai
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joseph Tsai (Gerrit)
Sep 15, 2025, 4:00:59 PMSep 15
to goph...@pubsubhelper.golang.org, Go LUCI, Damien Neil, golang-co...@googlegroups.com
Attention needed from Damien Neil
Joseph Tsai voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- Damien Neil
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joseph Tsai (Gerrit)
Sep 15, 2025, 4:22:19 PMSep 15
to goph...@pubsubhelper.golang.org, Go LUCI, Damien Neil, golang-co...@googlegroups.com
Attention needed from Damien Neil
Joseph Tsai voted and added 1 comment![]( "Open in Gerrit")
Votes added by Joseph Tsai
| Commit-Queue | +1 |
1 comment
Patchset-level comments
File-level comment, Patchset 2 (Latest):
Joseph Tsai . resolved
While the TryBot failure looks JSON related, I can reproduce it had HEAD without my change.
Related details
Attention is currently required from:
- Damien Neil
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joseph Tsai (Gerrit)
Sep 15, 2025, 8:38:36 PMSep 15
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Damien Neil and Joseph Tsai
Joseph Tsai uploaded new patchset![]( "Open in Gerrit")
Attention is currently required from:
- Damien Neil
- Joseph Tsai
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joseph Tsai (Gerrit)
Sep 15, 2025, 8:38:47 PMSep 15
to goph...@pubsubhelper.golang.org, Go LUCI, Damien Neil, golang-co...@googlegroups.com
Attention needed from Damien Neil
Joseph Tsai voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- Damien Neil
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Damien Neil (Gerrit)
Sep 25, 2025, 12:08:19 PM (7 days ago) Sep 25
to Joseph Tsai, goph...@pubsubhelper.golang.org, Go LUCI, golang-co...@googlegroups.com
Attention needed from Joseph Tsai
Damien Neil voted and added 1 comment![]( "Open in Gerrit")
Votes added by Damien Neil
| Code-Review | +2 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- Joseph Tsai
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Junyang Shao (Gerrit)
Sep 26, 2025, 1:11:15 PM (6 days ago) Sep 26
to Joseph Tsai, goph...@pubsubhelper.golang.org, Damien Neil, Go LUCI, golang-co...@googlegroups.com
Attention needed from Joseph Tsai
Junyang Shao voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- Joseph Tsai
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joseph Tsai (Gerrit)
2:03 AM (2 hours ago) 2:03 AM
to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Junyang Shao, Damien Neil, Go LUCI, golang-co...@googlegroups.com
Joseph Tsai submitted the change![]( "Open in Gerrit")
Change information
Commit message:
encoding/json: fix Decoder.InputOffset regression in goexperiment.jsonv2
The Decoder.InputOffset method was always ambiguous about the exact
offset returned since anything between the end of the previous token
to the start of the next token could be a valid result.
Empirically, it seems that the behavior was to report
the end of the previous token unless Decoder.More is called,
in which case it reports the start of the next token.
This is an odd semantic since a relatively side-effect free method
like More is not quite so side-effect free.
However, our goal is to preserve historical v1 semantic when possible
regardless of whether it made sense.
Note that jsontext.Decoder.InputOffset consistently always reports
the end of the previous token. Users can explicitly choose the
exact position they want by inspecting the UnreadBuffer.
Fixes #75468
Change-Id: I1e946e83c9d29dfc09f2913ff8d6b2b80632f292
Reviewed-on: https://go-review.googlesource.com/c/go/+/703856
Reviewed-by: Damien Neil <dn...@google.com>
LUCI-TryBot-Result: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Junyang Shao <shaoj...@google.com>
Files:
- M src/encoding/json/stream_test.go
- M src/encoding/json/v2_stream.go
- M src/encoding/json/v2_stream_test.go
Change size: M
Delta: 3 files changed, 149 insertions(+), 1 deletion(-)
Branch: refs/heads/master
Submit Requirements:
Code-Review: +1 by Junyang Shao, +2 by Damien Neil
TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages