Repository: exercism/cli
Pull request #1196 (2025-06-13 06:12:16)

Check HTTP response content type before trying to decode it as JSON

SleeplessByte: You should check for both this or ends with +json so that Jeremy can switch in the future to a +json type.

E.g.

`application/problem+json`
IsaacG: > You should check for both this or ends with +json so that Jeremy can switch in the future to a +json type.

Done. I'm not familiar with that new format. Is the intend that you can put any arbitrary string there? `application/<anything>+json`?
SleeplessByte: Yes, common is making a media type in the `vnd.` (vendor) tree, e.g.

```
application/vnd.exercism.submission.v1+json
```

That could be specced to always give the same *format* for the JSON, instead of merely the JSON syntax.

Similar:

```
# XLS
application/vnd.ms-excel

# XLSX (open format)
application/vnd.openxmlformats-officedocument.spreadsheetml.sheet
``` 

It's vital for HATEOAS (a way of designing HTTP APIs as described in [Roy Fieldings thesis](https://www.ics.uci.edu/~fielding/pubs/dissertation/rest_arch_style.htm)), used by [some of the big ones](https://docs.delftsolutions.nl/wiki/HATEOAS_API). Resource dump with surface level information: [Don't be smart and use web standards](https://blog.delftsolutions.nl/software/2024/05/24/dont-be-smart-and-use-web-standards.html). [Example of implementation of type-checked JSON requests and responses](https://github.com/XPBytes/media_types-serialization).

Patch:
diff --git a/cmd/cmd.go b/cmd/cmd.go
index dcca082..91597b5 100644
--- a/cmd/cmd.go
+++ b/cmd/cmd.go
@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"regexp"
 	"strings"
 
 	"io"
@@ -23,6 +24,8 @@ var (
 	Out io.Writer
 	// Err is used to write errors.
 	Err io.Writer
+	// jsonContentTypeRe is used to match Content-Type which contains JSON.
+	jsonContentTypeRe = regexp.MustCompile(`^application/([[:alpha:]]+\+)?json$`)
 )
 
 const msgWelcomePleaseConfigure = `
@@ -77,6 +80,13 @@ func validateUserConfig(cfg *viper.Viper) error {
 // decodedAPIError decodes and returns the error message from the API response.
 // If the message is blank, it returns a fallback message with the status code.
 func decodedAPIError(resp *http.Response) error {
+	if contentType := resp.Header.Get("Content-Type"); !jsonContentTypeRe.MatchString(contentType) {
+		return fmt.Errorf(
+			"expected response with Content-Type \"application/json\" but got status %q with Content-Type %q",
+			resp.Status,
+			contentType,
+		)
+	}
 	var apiError struct {
 		Error struct {
 			Type             string   `json:"type"`


Test patch:
diff --git a/cmd/cmd_test.go b/cmd/cmd_test.go
index 782be18..101c52c 100644
--- a/cmd/cmd_test.go
+++ b/cmd/cmd_test.go
@@ -2,7 +2,10 @@ package cmd
 
 import (
 	"io"
+	"io/ioutil"
+	"net/http"
 	"os"
+	"strings"
 	"testing"
 
 	"github.com/spf13/cobra"
@@ -119,3 +122,54 @@ func (co capturedOutput) reset() {
 	Out = co.oldOut
 	Err = co.oldErr
 }
+
+func errorResponse(contentType string, body string) *http.Response {
+	response := &http.Response{
+		Status:        "418 I'm a teapot",
+		StatusCode:    418,
+		Header:        make(http.Header),
+		Body:          ioutil.NopCloser(strings.NewReader(body)),
+		ContentLength: int64(len(body)),
+	}
+	response.Header.Set("Content-Type", contentType)
+	return response
+}
+
+func TestDecodeErrorResponse(t *testing.T) {
+	testCases := []struct {
+		response    *http.Response
+		wantMessage string
+	}{
+		{
+			response:    errorResponse("text/html", "Time for tea"),
+			wantMessage: `expected response with Content-Type "application/json" but got status "418 I'm a teapot" with Content-Type "text/html"`,
+		},
+		{
+			response:    errorResponse("application/json", `{"error": {"type": "json", "valid": no}}`),
+			wantMessage: "failed to parse API error response: invalid character 'o' in literal null (expecting 'u')",
+		},
+		{
+			response:    errorResponse("application/json", `{"error": {"type": "track_ambiguous", "message": "message", "possible_track_ids": ["a", "b"]}}`),
+			wantMessage: "message: a, b",
+		},
+		{
+			response:    errorResponse("application/json", `{"error": {"message": "message"}}`),
+			wantMessage: "message",
+		},
+		{
+			response:    errorResponse("application/problem+json", `{"error": {"message": "new json format"}}`),
+			wantMessage: "new json format",
+		},
+		{
+			response:    errorResponse("application/json", `{"error": {}}`),
+			wantMessage: "unexpected API response: 418",
+		},
+	}
+	tc := testCases[0]
+	got := decodedAPIError(tc.response)
+	assert.Equal(t, tc.wantMessage, got.Error())
+	for _, tc = range testCases {
+		got := decodedAPIError(tc.response)
+		assert.Equal(t, tc.wantMessage, got.Error())
+	}
+}
diff --git a/cmd/download_test.go b/cmd/download_test.go
index aadcf84..791be42 100644
--- a/cmd/download_test.go
+++ b/cmd/download_test.go
@@ -403,7 +403,7 @@ func TestDownloadError(t *testing.T) {
 
 	err = runDownload(cfg, flags, []string{})
 
-	assert.Equal(t, "test error", err.Error())
+	assert.Equal(t, `expected response with Content-Type "application/json" but got status "400 Bad Request" with Content-Type "text/plain; charset=utf-8"`, err.Error())
 
 }
 
diff --git a/cmd/submit_test.go b/cmd/submit_test.go
index 9b64596..7db152a 100644
--- a/cmd/submit_test.go
+++ b/cmd/submit_test.go
@@ -657,7 +657,7 @@ func TestSubmitServerErr(t *testing.T) {
 
 	err = runSubmit(cfg, pflag.NewFlagSet("fake", pflag.PanicOnError), files)
 
-	assert.Regexp(t, "test error", err.Error())
+	assert.Regexp(t, `expected response with Content-Type "application/json" but got status "400 Bad Request" with Content-Type "text/plain; charset=utf-8"`, err.Error())
 }
 
 func TestHandleErrorResponse(t *testing.T) {