Merge branch 'master' into advanced-permissions-phase-2

2018-04-27 10:53:38 +01:00
--- a/ISSUE_TEMPLATE.md
+++ b/ISSUE_TEMPLATE.md
@@ -1,9 +1,13 @@
-Per Mattermost guidelines, GitHub issues are for bug reports: http://www.mattermost.org/filing-issues/
+Per Mattermost guidelines, GitHub issues are for bug reports: <http://www.mattermost.org/filing-issues/>.

 For troubleshooting see: http://forum.mattermost.org/.
 For feature proposals see: http://www.mattermost.org/feature-requests/

-If you've found a bug--something appears unintentional--please report using the following format: 
+If you've found a bug--something appears unintentional--please follow these steps:
+
+1. Confirm you’re filing a new issue. [Search existing tickets in Jira](https://mattermost.atlassian.net/issues/) to ensure that the ticket does not already exist.
+2. Confirm your issue does not involve security. Otherwise, please see our [Responsible Disclosure Policy](https://about.mattermost.com/report-security-issue/).
+3. [File a new issue](https://github.com/mattermost/mattermost-server/issues/new) using the format below. Mattermost will confirm steps to reproduce and file in Jira, or ask for more details if there is trouble reproducing it. If there's already an existing bug in Jira, it will be linked back to the GitHub issue so you can track when it gets fixed. 

 #### Summary
 Bug report in one concise sentence
--- a/api4/webhook.go
+++ b/api4/webhook.go
@@ -509,11 +509,15 @@ func commandWebhook(c *Context, w http.ResponseWriter, r *http.Request) {
 	params := mux.Vars(r)
 	id := params["id"]

-	response := model.CommandResponseFromHTTPBody(r.Header.Get("Content-Type"), r.Body)
-
-	err := c.App.HandleCommandWebhook(id, response)
+	response, err := model.CommandResponseFromHTTPBody(r.Header.Get("Content-Type"), r.Body)
 	if err != nil {
-		c.Err = err
+		c.Err = model.NewAppError("commandWebhook", "web.command_webhook.parse.app_error", nil, err.Error(), http.StatusBadRequest)
+		return
+	}
+
+	appErr := c.App.HandleCommandWebhook(id, response)
+	if appErr != nil {
+		c.Err = appErr
 		return
 	}

--- a/api4/webhook_test.go
+++ b/api4/webhook_test.go
@@ -917,17 +917,21 @@ func TestCommandWebhooks(t *testing.T) {
 		t.Fatal(err)
 	}

-	if resp, _ := http.Post(Client.Url+"/hooks/commands/123123123123", "application/json", bytes.NewBufferString("{\"text\":\"this is a test\"}")); resp.StatusCode != http.StatusNotFound {
+	if resp, _ := http.Post(Client.Url+"/hooks/commands/123123123123", "application/json", bytes.NewBufferString(`{"text":"this is a test"}`)); resp.StatusCode != http.StatusNotFound {
 		t.Fatal("expected not-found for non-existent hook")
 	}

+	if resp, err := http.Post(Client.Url+"/hooks/commands/"+hook.Id, "application/json", bytes.NewBufferString(`{"text":"invalid`)); err != nil || resp.StatusCode != http.StatusBadRequest {
+		t.Fatal(err)
+	}
+
 	for i := 0; i < 5; i++ {
-		if resp, err := http.Post(Client.Url+"/hooks/commands/"+hook.Id, "application/json", bytes.NewBufferString("{\"text\":\"this is a test\"}")); err != nil || resp.StatusCode != http.StatusOK {
+		if resp, err := http.Post(Client.Url+"/hooks/commands/"+hook.Id, "application/json", bytes.NewBufferString(`{"text":"this is a test"}`)); err != nil || resp.StatusCode != http.StatusOK {
 			t.Fatal(err)
 		}
 	}

-	if resp, _ := http.Post(Client.Url+"/hooks/commands/"+hook.Id, "application/json", bytes.NewBufferString("{\"text\":\"this is a test\"}")); resp.StatusCode != http.StatusBadRequest {
+	if resp, _ := http.Post(Client.Url+"/hooks/commands/"+hook.Id, "application/json", bytes.NewBufferString(`{"text":"this is a test"}`)); resp.StatusCode != http.StatusBadRequest {
 		t.Fatal("expected error for sixth usage")
 	}
 }
--- a/app/command.go
+++ b/app/command.go
@@ -246,8 +246,9 @@ func (a *App) ExecuteCommand(args *model.CommandArgs) (*model.CommandResponse, *
 					return nil, model.NewAppError("command", "api.command.execute_command.failed.app_error", map[string]interface{}{"Trigger": trigger}, err.Error(), http.StatusInternalServerError)
 				} else {
 					if resp.StatusCode == http.StatusOK {
-						response := model.CommandResponseFromHTTPBody(resp.Header.Get("Content-Type"), resp.Body)
-						if response == nil {
+						if response, err := model.CommandResponseFromHTTPBody(resp.Header.Get("Content-Type"), resp.Body); err != nil {
+							return nil, model.NewAppError("command", "api.command.execute_command.failed.app_error", map[string]interface{}{"Trigger": trigger}, err.Error(), http.StatusInternalServerError)
+						} else if response == nil {
 							return nil, model.NewAppError("command", "api.command.execute_command.failed_empty.app_error", map[string]interface{}{"Trigger": trigger}, "", http.StatusInternalServerError)
 						} else {
 							return a.HandleCommandResponse(cmd, args, response, false)
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -4686,6 +4686,10 @@
    "id": "model.channel_member_history.is_valid.user_id.app_error",
    "translation": "Invalid user id"
  },
+  {
+    "id": "model.client.command.parse.app_error",
+    "translation": "Unable to parse incoming data"
+  },
  {
    "id": "model.client.connecting.app_error",
    "translation": "We encountered an error while connecting to the server"
--- a/model/client.go
+++ b/model/client.go
@@ -831,8 +831,10 @@ func (c *Client) Command(channelId string, command string) (*Result, *AppError)
 		return nil, err
 	} else {
 		defer closeBody(r)
+
+		response, _ := CommandResponseFromJson(r.Body)
 		return &Result{r.Header.Get(HEADER_REQUEST_ID),
-			r.Header.Get(HEADER_ETAG_SERVER), CommandResponseFromJson(r.Body)}, nil
+			r.Header.Get(HEADER_ETAG_SERVER), response}, nil
 	}
 }

--- a/model/client4.go
+++ b/model/client4.go
@@ -2991,7 +2991,9 @@ func (c *Client4) ExecuteCommand(channelId, command string) (*CommandResponse, *
 		return nil, BuildErrorResponse(r, err)
 	} else {
 		defer closeBody(r)
-		return CommandResponseFromJson(r.Body), BuildResponse(r)
+
+		response, _ := CommandResponseFromJson(r.Body)
+		return response, BuildResponse(r)
 	}
 }

@@ -3007,7 +3009,9 @@ func (c *Client4) ExecuteCommandWithTeam(channelId, teamId, command string) (*Co
 		return nil, BuildErrorResponse(r, err)
 	} else {
 		defer closeBody(r)
-		return CommandResponseFromJson(r.Body), BuildResponse(r)
+
+		response, _ := CommandResponseFromJson(r.Body)
+		return response, BuildResponse(r)
 	}
 }

--- a/model/command_response.go
+++ b/model/command_response.go
@@ -8,6 +8,8 @@ import (
 	"io"
 	"io/ioutil"
 	"strings"
+
+	"github.com/mattermost/mattermost-server/utils/jsonutils"
 )

 const (
@@ -31,14 +33,14 @@ func (o *CommandResponse) ToJson() string {
 	return string(b)
 }

-func CommandResponseFromHTTPBody(contentType string, body io.Reader) *CommandResponse {
+func CommandResponseFromHTTPBody(contentType string, body io.Reader) (*CommandResponse, error) {
 	if strings.TrimSpace(strings.Split(contentType, ";")[0]) == "application/json" {
 		return CommandResponseFromJson(body)
 	}
 	if b, err := ioutil.ReadAll(body); err == nil {
-		return CommandResponseFromPlainText(string(b))
+		return CommandResponseFromPlainText(string(b)), nil
 	}
-	return nil
+	return nil, nil
 }

 func CommandResponseFromPlainText(text string) *CommandResponse {
@@ -47,15 +49,19 @@ func CommandResponseFromPlainText(text string) *CommandResponse {
 	}
 }

-func CommandResponseFromJson(data io.Reader) *CommandResponse {
-	decoder := json.NewDecoder(data)
-	var o CommandResponse
+func CommandResponseFromJson(data io.Reader) (*CommandResponse, error) {
+	b, err := ioutil.ReadAll(data)
+	if err != nil {
+		return nil, err
+	}

-	if err := decoder.Decode(&o); err != nil {
-		return nil
+	var o CommandResponse
+	err = json.Unmarshal(b, &o)
+	if err != nil {
+		return nil, jsonutils.HumanizeJsonError(err, b)
 	}

 	o.Attachments = StringifySlackFieldValue(o.Attachments)

-	return &o
+	return &o, nil
 }
--- a/model/command_response_test.go
+++ b/model/command_response_test.go
@@ -6,18 +6,10 @@ package model
 import (
 	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )

-func TestCommandResponseJson(t *testing.T) {
-	o := CommandResponse{Text: "test"}
-	json := o.ToJson()
-	ro := CommandResponseFromJson(strings.NewReader(json))
-
-	if o.Text != ro.Text {
-		t.Fatal("Ids do not match")
-	}
-}
-
 func TestCommandResponseFromHTTPBody(t *testing.T) {
 	for _, test := range []struct {
 		ContentType  string
@@ -29,95 +21,137 @@ func TestCommandResponseFromHTTPBody(t *testing.T) {
 		{"application/json", `{"text": "foo"}`, "foo"},
 		{"application/json; charset=utf-8", `{"text": "foo"}`, "foo"},
 	} {
-		response := CommandResponseFromHTTPBody(test.ContentType, strings.NewReader(test.Body))
-		if response.Text != test.ExpectedText {
-			t.Fatal()
-		}
+		response, err := CommandResponseFromHTTPBody(test.ContentType, strings.NewReader(test.Body))
+		assert.NoError(t, err)
+		assert.Equal(t, test.ExpectedText, response.Text)
 	}
 }

 func TestCommandResponseFromPlainText(t *testing.T) {
 	response := CommandResponseFromPlainText("foo")
-	if response.Text != "foo" {
-		t.Fatal("text should be foo")
-	}
+	assert.Equal(t, "foo", response.Text)
 }

 func TestCommandResponseFromJson(t *testing.T) {
-	json := `{
-		"response_type": "ephemeral",
-		"text": "response text",
-		"username": "response username",
-		"icon_url": "response icon url",
-		"goto_location": "response goto location",
-		"attachments": [{
-			"text": "attachment 1 text",
-			"pretext": "attachment 1 pretext"
-		},{
-			"text": "attachment 2 text",
-			"fields": [{
-				"title": "field 1",
-				"value": "value 1",
-				"short": true
-			},{
-				"title": "field 2",
-				"value": [],
-				"short": false
-			}]
-		}]
-	}`
+	t.Parallel()

-	response := CommandResponseFromJson(strings.NewReader(json))
-
-	if response == nil {
-		t.Fatal("should've received non-nil CommandResponse")
+	sToP := func(s string) *string {
+		return &s
 	}

-	if response.ResponseType != "ephemeral" {
-		t.Fatal("should've received correct response type")
-	} else if response.Text != "response text" {
-		t.Fatal("should've received correct response text")
-	} else if response.Username != "response username" {
-		t.Fatal("should've received correct response username")
-	} else if response.IconURL != "response icon url" {
-		t.Fatal("should've received correct response icon url")
-	} else if response.GotoLocation != "response goto location" {
-		t.Fatal("should've received correct response goto location")
+	testCases := []struct {
+		Description             string
+		Json                    string
+		ExpectedCommandResponse *CommandResponse
+		ExpectedError           *string
+	}{
+		{
+			"empty response",
+			"",
+			nil,
+			sToP("parsing error at line 1, character 1: unexpected end of JSON input"),
+		},
+		{
+			"malformed response",
+			`{"text": }`,
+			nil,
+			sToP("parsing error at line 1, character 11: invalid character '}' looking for beginning of value"),
+		},
+		{
+			"invalid response",
+			`{"text": "test", "response_type": 5}`,
+			nil,
+			sToP("parsing error at line 1, character 36: json: cannot unmarshal number into Go struct field CommandResponse.response_type of type string"),
+		},
+		{
+			"ephemeral response",
+			`{
+				"response_type": "ephemeral",
+				"text": "response text",
+				"username": "response username",
+				"icon_url": "response icon url",
+				"goto_location": "response goto location",
+				"attachments": [{
+					"text": "attachment 1 text",
+					"pretext": "attachment 1 pretext"
+				},{
+					"text": "attachment 2 text",
+					"fields": [{
+						"title": "field 1",
+						"value": "value 1",
+						"short": true
+					},{
+						"title": "field 2",
+						"value": [],
+						"short": false
+					}]
+				}]
+			}`,
+			&CommandResponse{
+				ResponseType: "ephemeral",
+				Text:         "response text",
+				Username:     "response username",
+				IconURL:      "response icon url",
+				GotoLocation: "response goto location",
+				Attachments: []*SlackAttachment{
+					{
+						Text:    "attachment 1 text",
+						Pretext: "attachment 1 pretext",
+					},
+					{
+						Text: "attachment 2 text",
+						Fields: []*SlackAttachmentField{
+							{
+								Title: "field 1",
+								Value: "value 1",
+								Short: true,
+							},
+							{
+								Title: "field 2",
+								Value: "[]",
+								Short: false,
+							},
+						},
+					},
+				},
+			},
+			nil,
+		},
+		{
+			"null array items",
+			`{"attachments":[{"fields":[{"title":"foo","value":"bar","short":true}, null]}, null]}`,
+			&CommandResponse{
+				Attachments: []*SlackAttachment{
+					{
+						Fields: []*SlackAttachmentField{
+							{
+								Title: "foo",
+								Value: "bar",
+								Short: true,
+							},
+						},
+					},
+				},
+			},
+			nil,
+		},
 	}

-	attachments := response.Attachments
-	if len(attachments) != 2 {
-		t.Fatal("should've received 2 attachments")
-	} else if attachments[0].Text != "attachment 1 text" {
-		t.Fatal("should've received correct first attachment text")
-	} else if attachments[0].Pretext != "attachment 1 pretext" {
-		t.Fatal("should've received correct first attachment pretext")
-	} else if attachments[1].Text != "attachment 2 text" {
-		t.Fatal("should've received correct second attachment text")
-	}
+	for _, testCase := range testCases {
+		testCase := testCase
+		t.Run(testCase.Description, func(t *testing.T) {
+			t.Parallel()

-	fields := attachments[1].Fields
-	if len(fields) != 2 {
-		t.Fatal("should've received 2 fields")
-	} else if fields[0].Value.(string) != "value 1" {
-		t.Fatal("should've received correct first attachment value")
-	} else if _, ok := fields[1].Value.(string); !ok {
-		t.Fatal("should've received second attachment value parsed as a string")
-	} else if fields[1].Value.(string) != "[]" {
-		t.Fatal("should've received correct second attachment value")
-	}
-}
-
-func TestCommandResponseNullArrayItems(t *testing.T) {
-	payload := `{"attachments":[{"fields":[{"title":"foo","value":"bar","short":true}, null]}, null]}`
-	cr := CommandResponseFromJson(strings.NewReader(payload))
-	if cr == nil {
-		t.Fatal("CommandResponse should not be nil")
-	}
-	if len(cr.Attachments) != 1 {
-		t.Fatalf("expected one attachment")
-	}
-	if len(cr.Attachments[0].Fields) != 1 {
-		t.Fatalf("expected one field")
+			response, err := CommandResponseFromJson(strings.NewReader(testCase.Json))
+			if testCase.ExpectedError != nil {
+				assert.EqualError(t, err, *testCase.ExpectedError)
+				assert.Nil(t, response)
+			} else {
+				assert.NoError(t, err)
+				if assert.NotNil(t, response) {
+					assert.Equal(t, testCase.ExpectedCommandResponse, response)
+				}
+			}
+		})
 	}
 }
--- a/store/sqlstore/supplier.go
+++ b/store/sqlstore/supplier.go
@@ -61,6 +61,8 @@ const (
 	EXIT_REMOVE_TABLE                = 134
 	EXIT_CREATE_INDEX_SQLITE         = 135
 	EXIT_REMOVE_INDEX_SQLITE         = 136
+	EXIT_TABLE_EXISTS_SQLITE         = 137
+	EXIT_DOES_COLUMN_EXISTS_SQLITE   = 138
 )

 type SqlSupplierOldStores struct {
@@ -367,6 +369,20 @@ func (ss *SqlSupplier) DoesTableExist(tableName string) bool {

 		return count > 0

+	} else if ss.DriverName() == model.DATABASE_DRIVER_SQLITE {
+		count, err := ss.GetMaster().SelectInt(
+			`SELECT name FROM sqlite_master WHERE type='table' AND name=?`,
+			tableName,
+		)
+
+		if err != nil {
+			l4g.Critical(utils.T("store.sql.table_exists.critical"), err)
+			time.Sleep(time.Second)
+			os.Exit(EXIT_TABLE_EXISTS_SQLITE)
+		}
+
+		return count > 0
+
 	} else {
 		l4g.Critical(utils.T("store.sql.column_exists_missing_driver.critical"))
 		time.Sleep(time.Second)
@@ -422,6 +438,21 @@ func (ss *SqlSupplier) DoesColumnExist(tableName string, columnName string) bool

 		return count > 0

+	} else if ss.DriverName() == model.DATABASE_DRIVER_SQLITE {
+		count, err := ss.GetMaster().SelectInt(
+			`SELECT COUNT(*) FROM pragma_table_info(?) WHERE name=?`,
+			tableName,
+			columnName,
+		)
+
+		if err != nil {
+			l4g.Critical(utils.T("store.sql.column_exists.critical"), err)
+			time.Sleep(time.Second)
+			os.Exit(EXIT_DOES_COLUMN_EXISTS_SQLITE)
+		}
+
+		return count > 0
+
 	} else {
 		l4g.Critical(utils.T("store.sql.column_exists_missing_driver.critical"))
 		time.Sleep(time.Second)
--- a/utils/config.go
+++ b/utils/config.go
@@ -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
 	}

--- a/utils/config_test.go
+++ b/utils/config_test.go
@@ -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")
--- a/utils/jsonutils/json.go
+++ b/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),
+	}
+}
--- a/utils/jsonutils/json_test.go
+++ b/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)
+		})
+	}
+}