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.

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

cannon: Make jsonutil error when loading json payload with extra data #10542

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion op-service/jsonutil/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,14 @@ func LoadJSON[X any](inputPath string) (*X, error) {
}
defer f.Close()
var state X
if err := json.NewDecoder(f).Decode(&state); err != nil {
decoder := json.NewDecoder(f)
if err := decoder.Decode(&state); err != nil {
return nil, fmt.Errorf("failed to decode file %q: %w", inputPath, err)
}
// We are only expecting 1 JSON object - confirm there is no trailing data
if _, err := decoder.Token(); err != io.EOF {
return nil, fmt.Errorf("unexpected trailing data in file %q", inputPath)
}
return &state, nil
}

Expand Down
107 changes: 107 additions & 0 deletions op-service/jsonutil/json_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package jsonutil

import (
"encoding/json"
"fmt"
"os"
"path/filepath"
"testing"
Expand Down Expand Up @@ -47,7 +48,113 @@ func TestRoundTripJSONWithGzip(t *testing.T) {
require.EqualValues(t, data, result)
}

func TestLoadJSONWithExtraDataAppended(t *testing.T) {
data := &jsonTestData{A: "yay", B: 3}

cases := []struct {
name string
extraData func() ([]byte, error)
}{
{
name: "duplicate json object",
extraData: func() ([]byte, error) {
return json.Marshal(data)
},
},
{
name: "duplicate comma-separated json object",
extraData: func() ([]byte, error) {
data, err := json.Marshal(data)
if err != nil {
return nil, err
}
return append([]byte(","), data...), nil
},
},
{
name: "additional characters",
extraData: func() ([]byte, error) {
return []byte("some text"), nil
},
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
dir := t.TempDir()
file := filepath.Join(dir, "test.json")
extraData, err := tc.extraData()
require.NoError(t, err)

// Write primary json payload + extra data to the file
err = WriteJSON(file, data, 0o755)
require.NoError(t, err)
err = appendDataToFile(file, extraData)
require.NoError(t, err)

var result *jsonTestData
result, err = LoadJSON[jsonTestData](file)
require.ErrorContains(t, err, fmt.Sprintf("unexpected trailing data in file %q", file))
require.Nil(t, result)
})
}
}

func TestLoadJSONWithTrailingWhitespace(t *testing.T) {
cases := []struct {
name string
extraData []byte
}{
{
name: "space",
extraData: []byte(" "),
},
{
name: "tab",
extraData: []byte("\t"),
},
{
name: "new line",
extraData: []byte("\n"),
},
{
name: "multiple chars",
extraData: []byte(" \t\n"),
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
dir := t.TempDir()
file := filepath.Join(dir, "test.json")
data := &jsonTestData{A: "yay", B: 3}

// Write primary json payload + extra data to the file
err := WriteJSON(file, data, 0o755)
require.NoError(t, err)
err = appendDataToFile(file, tc.extraData)
require.NoError(t, err)

var result *jsonTestData
result, err = LoadJSON[jsonTestData](file)
require.NoError(t, err)
require.EqualValues(t, data, result)
})
}
}

type jsonTestData struct {
A string `json:"a"`
B int `json:"b"`
}

func appendDataToFile(outputPath string, data []byte) error {
file, err := os.OpenFile(outputPath, os.O_APPEND|os.O_WRONLY, 0644)
if err != nil {
return err
}
defer file.Close()

_, err = file.Write(data)
return err
}