Address #236

better handling of encoding issues pre validation.

Author: daveshanley

schema_validation/validate_document.go

@@ -8,6 +8,8 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"strconv"
+	"strings"
 
 	"github.com/pb33f/libopenapi"
 	"github.com/santhosh-tekuri/jsonschema/v6"
@@ -20,11 +22,200 @@ import (
 	"github.com/pb33f/libopenapi-validator/helpers"
 )
 
-func normalizeJSON(data any) any {
-	d, _ := json.Marshal(data)
+type nonStringMappingKey struct {
+	Value    string
+	Tag      string
+	Path     []string
+	Line     int
+	Column   int
+	Sequence bool
+}
+
+func normalizeJSON(data any) (any, error) {
+	d, err := json.Marshal(data)
+	if err != nil {
+		return nil, err
+	}
+
 	var normalized any
 	_ = json.Unmarshal(d, &normalized)
-	return normalized
+	return normalized, nil
+}
+
+func findNonStringMappingKey(rootNode *yaml.Node) *nonStringMappingKey {
+	if rootNode == nil {
+		return nil
+	}
+	return findNonStringMappingKeyInNode(rootNode, nil)
+}
+
+func findNonStringMappingKeyInNode(node *yaml.Node, path []string) *nonStringMappingKey {
+	if node == nil {
+		return nil
+	}
+
+	switch node.Kind {
+	case yaml.DocumentNode:
+		for _, child := range node.Content {
+			if found := findNonStringMappingKeyInNode(child, path); found != nil {
+				return found
+			}
+		}
+	case yaml.MappingNode:
+		for i := 0; i+1 < len(node.Content); i += 2 {
+			keyNode := node.Content[i]
+			valueNode := node.Content[i+1]
+			if isMergeMappingKey(keyNode) {
+				if found := findNonStringMappingKeyInMergeValue(valueNode, path); found != nil {
+					return found
+				}
+				continue
+			}
+			nextPath := appendPathSegment(path, keyNode.Value)
+			if !isStringMappingKey(keyNode) {
+				return &nonStringMappingKey{
+					Value:    keyNode.Value,
+					Tag:      keyNode.ShortTag(),
+					Path:     nextPath,
+					Line:     keyNode.Line,
+					Column:   keyNode.Column,
+					Sequence: keyNode.Kind == yaml.SequenceNode,
+				}
+			}
+			if found := findNonStringMappingKeyInNode(valueNode, nextPath); found != nil {
+				return found
+			}
+		}
+	case yaml.SequenceNode:
+		for i, child := range node.Content {
+			if found := findNonStringMappingKeyInNode(child, appendPathSegment(path, strconv.Itoa(i))); found != nil {
+				return found
+			}
+		}
+	}
+
+	return nil
+}
+
+func findNonStringMappingKeyInMergeValue(node *yaml.Node, path []string) *nonStringMappingKey {
+	if node == nil {
+		return nil
+	}
+
+	switch node.Kind {
+	case yaml.AliasNode:
+		return findNonStringMappingKeyInMergeValue(node.Alias, path)
+	case yaml.SequenceNode:
+		for _, child := range node.Content {
+			if found := findNonStringMappingKeyInMergeValue(child, path); found != nil {
+				return found
+			}
+		}
+		return nil
+	default:
+		return findNonStringMappingKeyInNode(node, path)
+	}
+}
+
+func isStringMappingKey(keyNode *yaml.Node) bool {
+	if keyNode == nil || keyNode.Kind != yaml.ScalarNode {
+		return false
+	}
+	return keyNode.ShortTag() == "!!str"
+}
+
+func isMergeMappingKey(keyNode *yaml.Node) bool {
+	if keyNode == nil || keyNode.Kind != yaml.ScalarNode {
+		return false
+	}
+	return keyNode.ShortTag() == "!!merge" && keyNode.Value == "<<"
+}
+
+func appendPathSegment(path []string, segment string) []string {
+	next := make([]string, 0, len(path)+1)
+	next = append(next, path...)
+	return append(next, segment)
+}
+
+func buildJSONPointer(path []string) string {
+	if len(path) == 0 {
+		return ""
+	}
+	var builder strings.Builder
+	for _, segment := range path {
+		builder.WriteByte('/')
+		builder.WriteString(helpers.EscapeJSONPointerSegment(segment))
+	}
+	return builder.String()
+}
+
+func buildNonStringMappingKeyError(key *nonStringMappingKey) *liberrors.ValidationError {
+	pointer := buildJSONPointer(key.Path)
+	reason := fmt.Sprintf("OpenAPI documents require string mapping keys, but found %s key %q at %s",
+		yamlKeyType(key), key.Value, pointer)
+	howToFix := "Quote YAML mapping keys that should be strings, because OpenAPI documents must be representable as JSON objects"
+
+	if isOperationResponseStatusCodeKey(key.Path) {
+		reason = fmt.Sprintf("Response status code keys must be strings, quote %s as %q at %s",
+			key.Value, key.Value, pointer)
+		howToFix = fmt.Sprintf("Quote the response status code key, for example use %q instead of %s",
+			key.Value, key.Value)
+	}
+
+	return &liberrors.ValidationError{
+		ValidationType:    helpers.Schema,
+		ValidationSubType: "document",
+		Message:           "OpenAPI document validation failed",
+		Reason:            reason,
+		SpecLine:          key.Line,
+		SpecCol:           key.Column,
+		HowToFix:          howToFix,
+		Context:           pointer,
+	}
+}
+
+func yamlKeyType(key *nonStringMappingKey) string {
+	if key == nil {
+		return "non-string"
+	}
+	if key.Sequence {
+		return "sequence"
+	}
+	return strings.TrimPrefix(key.Tag, "!!")
+}
+
+func isOperationResponseStatusCodeKey(path []string) bool {
+	if len(path) < 5 || path[0] != "paths" || path[len(path)-2] != "responses" {
+		return false
+	}
+	for _, segment := range path[2 : len(path)-2] {
+		if isHTTPMethod(segment) {
+			return true
+		}
+	}
+	return false
+}
+
+func isHTTPMethod(segment string) bool {
+	switch strings.ToLower(segment) {
+	case "get", "put", "post", "delete", "options", "head", "patch", "trace":
+		return true
+	default:
+		return false
+	}
+}
+
+func buildDocumentDecodeError(reason, context string) *liberrors.ValidationError {
+	return &liberrors.ValidationError{
+		ValidationType:    helpers.Schema,
+		ValidationSubType: "document",
+		Message:           "OpenAPI document validation failed",
+		Reason:            reason,
+		SpecLine:          1,
+		SpecCol:           0,
+		HowToFix:          "ensure the OpenAPI document is valid YAML/JSON and can be represented as JSON",
+		Context:           context,
+	}
 }
 
 // ValidateOpenAPIDocument will validate an OpenAPI document against the OpenAPI 2, 3.0 and 3.1 schemas (depending on version)
@@ -59,6 +250,12 @@ func ValidateOpenAPIDocumentWithPrecompiled(doc libopenapi.Document, compiledSch
 		return false, validationErrors
 	}
 
+	if info.RootNode != nil {
+		if invalidKey := findNonStringMappingKey(info.RootNode); invalidKey != nil {
+			return false, []*liberrors.ValidationError{buildNonStringMappingKeyError(invalidKey)}
+		}
+	}
+
 	// Use the precompiled schema if provided, otherwise compile it
 	jsch := compiledSchema
 	if jsch == nil {
@@ -88,11 +285,29 @@ func ValidateOpenAPIDocumentWithPrecompiled(doc libopenapi.Document, compiledSch
 		if err != nil {
 			// Fall back to normalizeJSON if UnmarshalJSON fails
 			if info.SpecJSON != nil {
-				normalized = normalizeJSON(*info.SpecJSON)
+				normalized, err = normalizeJSON(*info.SpecJSON)
+				if err != nil {
+					return false, []*liberrors.ValidationError{buildDocumentDecodeError(
+						fmt.Sprintf("The OpenAPI document cannot be converted to JSON: %s", err.Error()),
+						"SpecJSON",
+					)}
+				}
+			} else {
+				return false, []*liberrors.ValidationError{buildDocumentDecodeError(
+					fmt.Sprintf("The document's SpecJSONBytes cannot be decoded as JSON: %s", err.Error()),
+					"SpecJSONBytes",
+				)}
 			}
 		}
 	} else if info.SpecJSON != nil {
-		normalized = normalizeJSON(*info.SpecJSON)
+		var err error
+		normalized, err = normalizeJSON(*info.SpecJSON)
+		if err != nil {
+			return false, []*liberrors.ValidationError{buildDocumentDecodeError(
+				fmt.Sprintf("The OpenAPI document cannot be converted to JSON: %s", err.Error()),
+				"SpecJSON",
+			)}
+		}
 	}
 
 	// Validate the document