Skip to content

Commit

Permalink
fix: recognize vendor-specific JSON mimetypes (#138)
Browse files Browse the repository at this point in the history
This commit modifies the Go core response-handling code
so that it recognizes vendor-specific JSON-based mimetypes
(e.g. application/vnd.docker.distribution.manifest.v2+json)
as being equivalent to "application/json" when deciding how
to unmarshal an operation response body.
Prior to this change, the Go core would treat such a response
as an error.
  • Loading branch information
padamstx authored Sep 15, 2021
1 parent 349c122 commit fb2c14a
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 8 deletions.
11 changes: 6 additions & 5 deletions v5/core/base_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -438,10 +438,11 @@ func (service *BaseService) Request(req *http.Request, result interface{}) (deta

// Operation was successful and we are expecting a response, so process the response.
if !IsNil(result) {
resultType := reflect.TypeOf(result).String()

// If 'result' is a io.ReadCloser, then pass the response body back reflectively via 'result'
// and bypass any further unmarshalling of the response.
if reflect.TypeOf(result).String() == "*io.ReadCloser" {
if resultType == "*io.ReadCloser" {
rResult := reflect.ValueOf(result).Elem()
rResult.Set(reflect.ValueOf(httpResponse.Body))
detailedResponse.Result = httpResponse.Body
Expand Down Expand Up @@ -480,24 +481,24 @@ func (service *BaseService) Request(req *http.Request, result interface{}) (deta
// Check to see if the caller wanted the response body as a string.
// If the caller passed in 'result' as the address of *string,
// then we'll reflectively set result to point to it.
if reflect.TypeOf(result).String() == "**string" {
if resultType == "**string" {
responseString := string(responseBody)
rResult := reflect.ValueOf(result).Elem()
rResult.Set(reflect.ValueOf(&responseString))

// And set the string in the Result field.
detailedResponse.Result = &responseString
} else if reflect.TypeOf(result).String() == "*[]uint8" { // byte is an alias for uint8
} else if resultType == "*[]uint8" { // byte is an alias for uint8
rResult := reflect.ValueOf(result).Elem()
rResult.Set(reflect.ValueOf(responseBody))

// And set the byte slice in the Result field.
detailedResponse.Result = responseBody
} else {
// At this point, we don't know how to set the result field, so we have to return an error
// At this point, we don't know how to set the result field, so we have to return an error.
// But make sure we save the bytes we read in the DetailedResponse for debugging purposes
detailedResponse.Result = responseBody
err = fmt.Errorf(ERRORMSG_UNEXPECTED_RESPONSE)
err = fmt.Errorf(ERRORMSG_UNEXPECTED_RESPONSE, contentType, resultType)
return
}
}
Expand Down
45 changes: 45 additions & 0 deletions v5/core/base_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,51 @@ func TestRequestGoodResponseJSON(t *testing.T) {
assert.Equal(t, "wonder woman", *(result.Name))
}

// Test a normal JSON-based response using a vendor-specific Content-Type
func TestRequestGoodResponseCustomJSONContentType(t *testing.T) {
customContentType := "application/vnd.sdksquad.custom.semantics+json;charset=UTF8"
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-type", customContentType)
w.WriteHeader(http.StatusCreated)
fmt.Fprint(w, `{"name": "wonder woman"}`)
}))
defer server.Close()

builder := NewRequestBuilder("POST")
_, err := builder.ResolveRequestURL(server.URL, "", nil)
assert.Nil(t, err)
req, _ := builder.Build()

authenticator, err := NewBasicAuthenticator("xxx", "yyy")
assert.Nil(t, err)
assert.NotNil(t, authenticator)

options := &ServiceOptions{
URL: server.URL,
Authenticator: authenticator,
}
service, err := NewBaseService(options)
assert.Nil(t, err)
assert.NotNil(t, service.Options.Authenticator)
assert.Equal(t, AUTHTYPE_BASIC, service.Options.Authenticator.AuthenticationType())

// Use a cloned service to verify it works ok.
service = service.Clone()

var foo *Foo
detailedResponse, err := service.Request(req, &foo)
assert.Nil(t, err)
assert.NotNil(t, detailedResponse)
assert.Equal(t, http.StatusCreated, detailedResponse.StatusCode)
assert.Equal(t, customContentType, detailedResponse.Headers.Get("Content-Type"))

result, ok := detailedResponse.Result.(*Foo)
assert.Equal(t, true, ok)
assert.NotNil(t, result)
assert.NotNil(t, foo)
assert.Equal(t, "wonder woman", *(result.Name))
}

// Test a JSON-based response that should be returned as a stream (io.ReadCloser).
func TestRequestGoodResponseJSONStream(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
Expand Down
4 changes: 2 additions & 2 deletions v5/core/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ const (
"and/or use the DisableSSLVerification option of the authenticator."
ERRORMSG_AUTHENTICATE_ERROR = "An error occurred while performing the 'authenticate' step: %s"
ERRORMSG_READ_RESPONSE_BODY = "An error occurred while reading the response body: %s"
ERRORMSG_UNEXPECTED_RESPONSE = "The response contained unexpected content"
ERRORMSG_UNEXPECTED_RESPONSE = "The response contained unexpected content, Content-Type=%s, operation resultType=%s"
ERRORMSG_UNMARSHAL_RESPONSE_BODY = "An error occurred while unmarshalling the response body: %s"
ERRORMSG_NIL_SLICE = "The 'slice' parameter cannot be nil"
ERRORMSG_PARAM_NOT_SLICE = "The 'slice' parameter must be a slice"
Expand All @@ -75,6 +75,6 @@ const (
ERRORMSG_CREATE_RETRYABLE_REQ = "An error occurred while creating a retryable http Request: %s"
ERRORMSG_UNEXPECTED_STATUS_CODE = "Unexpected HTTP status code %d (%s)"
ERRORMSG_UNMARSHAL_AUTH_RESPONSE = "error unmarshalling authentication response: %s"
ERRORMSG_UNABLE_RETRIEVE_CRTOKEN = "unable to retrieve compute resource token value: %s" // #nosec G101
ERRORMSG_UNABLE_RETRIEVE_CRTOKEN = "unable to retrieve compute resource token value: %s" // #nosec G101
ERRORMSG_IAM_GETTOKEN_ERROR = "IAM 'get token' error, status code %d received from '%s': %s" // #nosec G101
)
2 changes: 1 addition & 1 deletion v5/core/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func init() {
}

const (
jsonMimePattern = "(?i)^application\\/((json)|(merge\\-patch\\+json))(;.*)?$"
jsonMimePattern = "(?i)^application\\/((json)|(merge\\-patch\\+json)|(vnd\\..*\\+json))(;.*)?$"
jsonPatchMimePattern = "(?i)^application\\/json\\-patch\\+json(;.*)?$"
)

Expand Down
4 changes: 4 additions & 0 deletions v5/core/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,13 @@ func TestIsJSONMimeType(t *testing.T) {
assert.True(t, IsJSONMimeType("application/json"))
assert.True(t, IsJSONMimeType("APPlication/json"))
assert.True(t, IsJSONMimeType("application/json;blah"))
assert.True(t, IsJSONMimeType("application/vnd.docker.distribution.manifest.v2+json"))
assert.True(t, IsJSONMimeType("application/vnd.anothervendor.custom.semantics+json"))
assert.True(t, IsJSONMimeType("application/vnd.yet.another.vendor.with.custom.semantics.blah.v3+json;charset=UTF8"))

assert.False(t, IsJSONMimeType("application/json-patch+patch"))
assert.False(t, IsJSONMimeType("YOapplication/jsonYO"))
assert.False(t, IsJSONMimeType("YOapplication/vnd.docker.distribution.manifest.v2+jsonYO"))
}

func TestIsJSONPatchMimeType(t *testing.T) {
Expand Down

0 comments on commit fb2c14a

Please sign in to comment.