diff --git a/datamodel/spec_info.go b/datamodel/spec_info.go index f28126ed..3e1c6fa5 100644 --- a/datamodel/spec_info.go +++ b/datamodel/spec_info.go @@ -88,18 +88,25 @@ func ExtractSpecInfoWithDocumentCheck(spec []byte, bypass bool) (*SpecInfo, erro _, openAPI2 := utils.FindKeyNode(utils.OpenApi2, parsedSpec.Content) _, asyncAPI := utils.FindKeyNode(utils.AsyncApi, parsedSpec.Content) - parseJSON := func(bytes []byte, spec *SpecInfo, parsedNode *yaml.Node) { + parseJSON := func(bytes []byte, spec *SpecInfo, parsedNode *yaml.Node) error { var jsonSpec map[string]interface{} + var parseErr error + if utils.IsYAML(string(bytes)) { - _ = parsedNode.Decode(&jsonSpec) + parseErr = parsedNode.Decode(&jsonSpec) + // NOTE: Even if Decoding results in an error, `jsonSpec` can still contain partial or empty data. + // This subsequent Marshalling should succeed unless `jsonSpec` contains unsupported types, + // which isn't possible as Decoding will only decode valid data. b, _ := json.Marshal(&jsonSpec) spec.SpecJSONBytes = &b spec.SpecJSON = &jsonSpec } else { - _ = json.Unmarshal(bytes, &jsonSpec) + parseErr = json.Unmarshal(bytes, &jsonSpec) spec.SpecJSONBytes = &bytes spec.SpecJSON = &jsonSpec } + + return parseErr } //if !bypass { @@ -133,7 +140,9 @@ func ExtractSpecInfoWithDocumentCheck(spec []byte, bypass bool) (*SpecInfo, erro } // parse JSON - parseJSON(spec, specInfo, &parsedSpec) + if err := parseJSON(spec, specInfo, &parsedSpec); err != nil && !bypass { + return nil, err + } parsed = true // double check for the right version, people mix this up. @@ -160,7 +169,9 @@ func ExtractSpecInfoWithDocumentCheck(spec []byte, bypass bool) (*SpecInfo, erro specInfo.APISchema = OpenAPI2SchemaData // parse JSON - parseJSON(spec, specInfo, &parsedSpec) + if err := parseJSON(spec, specInfo, &parsedSpec); err != nil && !bypass { + return nil, err + } parsed = true // I am not certain this edge-case is very frequent, but let's make sure we handle it anyway. @@ -184,7 +195,9 @@ func ExtractSpecInfoWithDocumentCheck(spec []byte, bypass bool) (*SpecInfo, erro // TODO: format for AsyncAPI. // parse JSON - parseJSON(spec, specInfo, &parsedSpec) + if err := parseJSON(spec, specInfo, &parsedSpec); err != nil && !bypass { + return nil, err + } parsed = true // so far there is only 2 as a major release of AsyncAPI @@ -199,7 +212,9 @@ func ExtractSpecInfoWithDocumentCheck(spec []byte, bypass bool) (*SpecInfo, erro if specInfo.SpecType == "" { // parse JSON if !bypass { - parseJSON(spec, specInfo, &parsedSpec) + if err := parseJSON(spec, specInfo, &parsedSpec); err != nil { + return nil, err + } parsed = true specInfo.Error = errors.New("spec type not supported by libopenapi, sorry") return specInfo, specInfo.Error @@ -207,11 +222,13 @@ func ExtractSpecInfoWithDocumentCheck(spec []byte, bypass bool) (*SpecInfo, erro } //} else { // // parse JSON - // parseJSON(spec, specInfo, &parsedSpec) + // _ = parseJSON(spec, specInfo, &parsedSpec) //} if !parsed { - parseJSON(spec, specInfo, &parsedSpec) + if err := parseJSON(spec, specInfo, &parsedSpec); err != nil && !bypass { + return nil, err + } } // detect the original whitespace indentation diff --git a/datamodel/spec_info_test.go b/datamodel/spec_info_test.go index 5ef6ec03..e6a74a63 100644 --- a/datamodel/spec_info_test.go +++ b/datamodel/spec_info_test.go @@ -5,10 +5,10 @@ package datamodel import ( "fmt" + "github.com/pb33f/libopenapi/utils" "os" "testing" - "github.com/pb33f/libopenapi/utils" "github.com/stretchr/testify/assert" ) @@ -24,9 +24,10 @@ const ( ) var ( - goodJSON = `{"name":"kitty", "noises":["meow","purrrr","gggrrraaaaaooooww"]}` - badJSON = `{"name":"kitty, "noises":[{"meow","purrrr","gggrrraaaaaooooww"]}}` - goodYAML = `name: kitty + goodJSON = `{"name":"kitty", "noises":["meow","purrrr","gggrrraaaaaooooww"]}` + badJSON = `{"name":"kitty, "noises":[{"meow","purrrr","gggrrraaaaaooooww"]}}` + badJSONMissingQuote = `{"openapi": "3.0.1", "name":"kitty", "name":["kitty2",kitty3"]}` + goodYAML = `name: kitty noises: - meow - purrr @@ -74,6 +75,23 @@ tags: servers: - url: https://quobix.com/api` +// This is a bad yaml file, where the second openapi path is indented incorrectly, +// causing its 'get' operation to be on the same indent level as the first path's 'get' operation, so it's a duplicate key and would fail decoding. +var BadYAMLRepeatedPathKey = `openapi: 3.0.1 +info: + title: Test API +paths: + /test: + get: + responses: + '200': + description: OK + /test2: + get: + responses: + '200': + description: OK` + var OpenApi2Spec = `swagger: 2.0.1 info: title: Test API @@ -118,6 +136,11 @@ func TestExtractSpecInfo_InvalidJSON(t *testing.T) { assert.Error(t, e) } +func TestExtractSpecInfo_InvalidJSON_MissingQuote(t *testing.T) { + _, e := ExtractSpecInfo([]byte(badJSONMissingQuote)) + assert.Error(t, e) +} + func TestExtractSpecInfo_Nothing(t *testing.T) { _, e := ExtractSpecInfo([]byte("")) assert.Error(t, e) @@ -134,6 +157,11 @@ func TestExtractSpecInfo_InvalidYAML(t *testing.T) { assert.Error(t, e) } +func TestExtractSpecInfo_InvalidYAML_RepeatedPathKey(t *testing.T) { + _, e := ExtractSpecInfo([]byte(BadYAMLRepeatedPathKey)) + assert.Error(t, e) +} + func TestExtractSpecInfo_InvalidOpenAPIVersion(t *testing.T) { _, e := ExtractSpecInfo([]byte(OpenApiOne)) assert.Error(t, e)