fix: Add error handling when parsing JSON/YAML spec

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,19 +212,23 @@ 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
 		}
 	}
 	//} 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

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)