MM-10232, MM-10259: Improve error handling from invalid json (#8668)
* MM-10232: improve error handling from malformed slash command responses Switch to json.Unmarshal, which doesn't obscure JSON parse failures like json.Decode. The latter is primarily designed for streams of JSON, not necessarily unmarshalling just a single object. * rework HumanizedJsonError to expose Line and Character discretely * MM-10259: pinpoint line and character where json config error occurs * tweak HumanizeJsonError to accept err first
Этот коммит содержится в:
Jesse Hallam
коммит произвёл
GitHub
GitHub
родитель
d3f09b54e2
Коммит
6d50d836f5
@@ -4,6 +4,7 @@
|
||||
package utils
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"io"
|
||||
@@ -23,6 +24,7 @@ import (
|
||||
|
||||
"github.com/mattermost/mattermost-server/einterfaces"
|
||||
"github.com/mattermost/mattermost-server/model"
|
||||
"github.com/mattermost/mattermost-server/utils/jsonutils"
|
||||
)
|
||||
|
||||
const (
|
||||
@@ -214,9 +216,19 @@ func (w *ConfigWatcher) Close() {
|
||||
|
||||
// ReadConfig reads and parses the given configuration.
|
||||
func ReadConfig(r io.Reader, allowEnvironmentOverrides bool) (*model.Config, map[string]interface{}, error) {
|
||||
v := newViper(allowEnvironmentOverrides)
|
||||
// Pre-flight check the syntax of the configuration file to improve error messaging.
|
||||
configData, err := ioutil.ReadAll(r)
|
||||
if err != nil {
|
||||
return nil, nil, err
|
||||
} else {
|
||||
var rawConfig interface{}
|
||||
if err := json.Unmarshal(configData, &rawConfig); err != nil {
|
||||
return nil, nil, jsonutils.HumanizeJsonError(err, configData)
|
||||
}
|
||||
}
|
||||
|
||||
if err := v.ReadConfig(r); err != nil {
|
||||
v := newViper(allowEnvironmentOverrides)
|
||||
if err := v.ReadConfig(bytes.NewReader(configData)); err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
package utils
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"io/ioutil"
|
||||
"os"
|
||||
"path/filepath"
|
||||
@@ -23,6 +24,19 @@ func TestConfig(t *testing.T) {
|
||||
InitTranslations(cfg.LocalizationSettings)
|
||||
}
|
||||
|
||||
func TestReadConfig(t *testing.T) {
|
||||
TranslationsPreInit()
|
||||
|
||||
_, _, err := ReadConfig(bytes.NewReader([]byte(``)), false)
|
||||
require.EqualError(t, err, "parsing error at line 1, character 1: unexpected end of JSON input")
|
||||
|
||||
_, _, err = ReadConfig(bytes.NewReader([]byte(`
|
||||
{
|
||||
malformed
|
||||
`)), false)
|
||||
require.EqualError(t, err, "parsing error at line 3, character 5: invalid character 'm' looking for beginning of object key string")
|
||||
}
|
||||
|
||||
func TestTimezoneConfig(t *testing.T) {
|
||||
TranslationsPreInit()
|
||||
supportedTimezones := LoadTimezones("timezones.json")
|
||||
|
||||
56
utils/jsonutils/json.go
Обычный файл
56
utils/jsonutils/json.go
Обычный файл
@@ -0,0 +1,56 @@
|
||||
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
|
||||
// See License.txt for license information.
|
||||
|
||||
package jsonutils
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/json"
|
||||
|
||||
"github.com/pkg/errors"
|
||||
)
|
||||
|
||||
type HumanizedJsonError struct {
|
||||
Err error
|
||||
Line int
|
||||
Character int
|
||||
}
|
||||
|
||||
func (e *HumanizedJsonError) Error() string {
|
||||
return e.Err.Error()
|
||||
}
|
||||
|
||||
// HumanizeJsonError extracts error offsets and annotates the error with useful context
|
||||
func HumanizeJsonError(err error, data []byte) error {
|
||||
if syntaxError, ok := err.(*json.SyntaxError); ok {
|
||||
return NewHumanizedJsonError(syntaxError, data, syntaxError.Offset)
|
||||
} else if unmarshalError, ok := err.(*json.UnmarshalTypeError); ok {
|
||||
return NewHumanizedJsonError(unmarshalError, data, unmarshalError.Offset)
|
||||
} else {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
func NewHumanizedJsonError(err error, data []byte, offset int64) *HumanizedJsonError {
|
||||
if err == nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
if offset < 0 || offset > int64(len(data)) {
|
||||
return &HumanizedJsonError{
|
||||
Err: errors.Wrapf(err, "invalid offset %d", offset),
|
||||
}
|
||||
}
|
||||
|
||||
lineSep := []byte{'\n'}
|
||||
|
||||
line := bytes.Count(data[:offset], lineSep) + 1
|
||||
lastLineOffset := bytes.LastIndex(data[:offset], lineSep)
|
||||
character := int(offset) - (lastLineOffset + 1) + 1
|
||||
|
||||
return &HumanizedJsonError{
|
||||
Line: line,
|
||||
Character: character,
|
||||
Err: errors.Wrapf(err, "parsing error at line %d, character %d", line, character),
|
||||
}
|
||||
}
|
||||
237
utils/jsonutils/json_test.go
Обычный файл
237
utils/jsonutils/json_test.go
Обычный файл
@@ -0,0 +1,237 @@
|
||||
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
|
||||
// See License.txt for license information.
|
||||
|
||||
package jsonutils_test
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"reflect"
|
||||
"testing"
|
||||
|
||||
"github.com/pkg/errors"
|
||||
"github.com/stretchr/testify/assert"
|
||||
|
||||
"github.com/mattermost/mattermost-server/utils/jsonutils"
|
||||
)
|
||||
|
||||
func TestHumanizeJsonError(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
type testType struct{}
|
||||
|
||||
testCases := []struct {
|
||||
Description string
|
||||
Data []byte
|
||||
Err error
|
||||
ExpectedErr string
|
||||
}{
|
||||
{
|
||||
"nil error",
|
||||
[]byte{},
|
||||
nil,
|
||||
"",
|
||||
},
|
||||
{
|
||||
"non-special error",
|
||||
[]byte{},
|
||||
errors.New("test"),
|
||||
"test",
|
||||
},
|
||||
{
|
||||
"syntax error, offset 17, middle of line 3",
|
||||
[]byte("line 1\nline 2\nline 3"),
|
||||
&json.SyntaxError{
|
||||
// msg can't be set
|
||||
Offset: 17,
|
||||
},
|
||||
"parsing error at line 3, character 4: ",
|
||||
},
|
||||
{
|
||||
"unmarshal type error, offset 17, middle of line 3",
|
||||
[]byte("line 1\nline 2\nline 3"),
|
||||
&json.UnmarshalTypeError{
|
||||
Value: "bool",
|
||||
Type: reflect.TypeOf(testType{}),
|
||||
Offset: 17,
|
||||
Struct: "struct",
|
||||
Field: "field",
|
||||
},
|
||||
"parsing error at line 3, character 4: json: cannot unmarshal bool into Go struct field struct.field of type jsonutils_test.testType",
|
||||
},
|
||||
}
|
||||
|
||||
for _, testCase := range testCases {
|
||||
testCase := testCase
|
||||
t.Run(testCase.Description, func(t *testing.T) {
|
||||
actual := jsonutils.HumanizeJsonError(testCase.Err, testCase.Data)
|
||||
if testCase.ExpectedErr == "" {
|
||||
assert.NoError(t, actual)
|
||||
} else {
|
||||
assert.EqualError(t, actual, testCase.ExpectedErr)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestNewHumanizedJsonError(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
type testType struct{}
|
||||
|
||||
testCases := []struct {
|
||||
Description string
|
||||
Data []byte
|
||||
Offset int64
|
||||
Err error
|
||||
Expected *jsonutils.HumanizedJsonError
|
||||
}{
|
||||
{
|
||||
"nil error",
|
||||
[]byte{},
|
||||
0,
|
||||
nil,
|
||||
nil,
|
||||
},
|
||||
{
|
||||
"offset -1, before start of string",
|
||||
[]byte("line 1\nline 2\nline 3"),
|
||||
-1,
|
||||
errors.New("message"),
|
||||
&jsonutils.HumanizedJsonError{
|
||||
Err: errors.Wrap(errors.New("message"), "invalid offset -1"),
|
||||
},
|
||||
},
|
||||
{
|
||||
"offset 0, start of string",
|
||||
[]byte("line 1\nline 2\nline 3"),
|
||||
0,
|
||||
errors.New("message"),
|
||||
&jsonutils.HumanizedJsonError{
|
||||
Err: errors.Wrap(errors.New("message"), "parsing error at line 1, character 1"),
|
||||
Line: 1,
|
||||
Character: 1,
|
||||
},
|
||||
},
|
||||
{
|
||||
"offset 5, end of line 1",
|
||||
[]byte("line 1\nline 2\nline 3"),
|
||||
5,
|
||||
errors.New("message"),
|
||||
&jsonutils.HumanizedJsonError{
|
||||
Err: errors.Wrap(errors.New("message"), "parsing error at line 1, character 6"),
|
||||
Line: 1,
|
||||
Character: 6,
|
||||
},
|
||||
},
|
||||
{
|
||||
"offset 6, new line at end end of line 1",
|
||||
[]byte("line 1\nline 2\nline 3"),
|
||||
6,
|
||||
errors.New("message"),
|
||||
&jsonutils.HumanizedJsonError{
|
||||
Err: errors.Wrap(errors.New("message"), "parsing error at line 1, character 7"),
|
||||
Line: 1,
|
||||
Character: 7,
|
||||
},
|
||||
},
|
||||
{
|
||||
"offset 7, start of line 2",
|
||||
[]byte("line 1\nline 2\nline 3"),
|
||||
7,
|
||||
errors.New("message"),
|
||||
&jsonutils.HumanizedJsonError{
|
||||
Err: errors.Wrap(errors.New("message"), "parsing error at line 2, character 1"),
|
||||
Line: 2,
|
||||
Character: 1,
|
||||
},
|
||||
},
|
||||
{
|
||||
"offset 12, end of line 2",
|
||||
[]byte("line 1\nline 2\nline 3"),
|
||||
12,
|
||||
errors.New("message"),
|
||||
&jsonutils.HumanizedJsonError{
|
||||
Err: errors.Wrap(errors.New("message"), "parsing error at line 2, character 6"),
|
||||
Line: 2,
|
||||
Character: 6,
|
||||
},
|
||||
},
|
||||
{
|
||||
"offset 13, newline at end of line 2",
|
||||
[]byte("line 1\nline 2\nline 3"),
|
||||
13,
|
||||
errors.New("message"),
|
||||
&jsonutils.HumanizedJsonError{
|
||||
Err: errors.Wrap(errors.New("message"), "parsing error at line 2, character 7"),
|
||||
Line: 2,
|
||||
Character: 7,
|
||||
},
|
||||
},
|
||||
{
|
||||
"offset 17, middle of line 3",
|
||||
[]byte("line 1\nline 2\nline 3"),
|
||||
17,
|
||||
errors.New("message"),
|
||||
&jsonutils.HumanizedJsonError{
|
||||
Err: errors.Wrap(errors.New("message"), "parsing error at line 3, character 4"),
|
||||
Line: 3,
|
||||
Character: 4,
|
||||
},
|
||||
},
|
||||
{
|
||||
"offset 19, end of string",
|
||||
[]byte("line 1\nline 2\nline 3"),
|
||||
19,
|
||||
errors.New("message"),
|
||||
&jsonutils.HumanizedJsonError{
|
||||
Err: errors.Wrap(errors.New("message"), "parsing error at line 3, character 6"),
|
||||
Line: 3,
|
||||
Character: 6,
|
||||
},
|
||||
},
|
||||
{
|
||||
"offset 20, offset = length of string",
|
||||
[]byte("line 1\nline 2\nline 3"),
|
||||
20,
|
||||
errors.New("message"),
|
||||
&jsonutils.HumanizedJsonError{
|
||||
Err: errors.Wrap(errors.New("message"), "parsing error at line 3, character 7"),
|
||||
Line: 3,
|
||||
Character: 7,
|
||||
},
|
||||
},
|
||||
{
|
||||
"offset 21, offset = length of string, after newline",
|
||||
[]byte("line 1\nline 2\nline 3\n"),
|
||||
21,
|
||||
errors.New("message"),
|
||||
&jsonutils.HumanizedJsonError{
|
||||
Err: errors.Wrap(errors.New("message"), "parsing error at line 4, character 1"),
|
||||
Line: 4,
|
||||
Character: 1,
|
||||
},
|
||||
},
|
||||
{
|
||||
"offset 21, offset > length of string",
|
||||
[]byte("line 1\nline 2\nline 3"),
|
||||
21,
|
||||
errors.New("message"),
|
||||
&jsonutils.HumanizedJsonError{
|
||||
Err: errors.Wrap(errors.New("message"), "invalid offset 21"),
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, testCase := range testCases {
|
||||
testCase := testCase
|
||||
t.Run(testCase.Description, func(t *testing.T) {
|
||||
actual := jsonutils.NewHumanizedJsonError(testCase.Err, testCase.Data, testCase.Offset)
|
||||
if testCase.Expected != nil && actual.Err != nil {
|
||||
if assert.EqualValues(t, testCase.Expected.Err.Error(), actual.Err.Error()) {
|
||||
actual.Err = testCase.Expected.Err
|
||||
}
|
||||
}
|
||||
assert.Equal(t, testCase.Expected, actual)
|
||||
})
|
||||
}
|
||||
}
|
||||
Ссылка в новой задаче
Block a user