Fix scheme handling in v2->v3 conversion (getkin#441)

go.mod

@@ -2,10 +2,14 @@ module github.com/getkin/kin-openapi
 
 go 1.14
 
+replace github.com/ghodss/yaml/v2 => github.com/diamondburned/yaml/v2 v2.0.0-20240812065612-baf990d70122
+
 require (
 	github.com/ghodss/yaml v1.0.0
+	github.com/ghodss/yaml/v2 v2.0.0-00010101000000-000000000000
 	github.com/go-openapi/jsonpointer v0.19.5
 	github.com/gorilla/mux v1.8.0
 	github.com/stretchr/testify v1.5.1
 	gopkg.in/yaml.v2 v2.3.0
+	gopkg.in/yaml.v3 v3.0.1 // indirect
 )

go.sum

@@ -1,6 +1,8 @@
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/diamondburned/yaml/v2 v2.0.0-20240812065612-baf990d70122 h1:hOA7Z6xhY5sn50zMsuY9JhA0A1QMiO0z/Ltx7ZcqUCM=
+github.com/diamondburned/yaml/v2 v2.0.0-20240812065612-baf990d70122/go.mod h1:KkR1H6NtyEqVsGChMAaRwn4BkIX0dG683i7NgqX947Y=
 github.com/ghodss/yaml v1.0.0 h1:wQHKEahhL6wmXdzwWG11gIVCkOv05bNOh+Rxn0yngAk=
 github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04=
 github.com/go-openapi/jsonpointer v0.19.5 h1:gZr+CIYByUqjcgeLXnQu2gHYQC9o73G2XUeOFYEICuY=
@@ -29,3 +31,6 @@ gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8
 gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.3.0 h1:clyUAQHOM3G0M3f5vQj7LuJrETvjVot3Z5el9nffUtU=
 gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
+gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
+gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
+gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

jsoninfo/orderedmap.go

@@ -0,0 +1,46 @@
+package jsoninfo
+
+import (
+	"bytes"
+	"encoding/json"
+	"fmt"
+	"log"
+)
+
+// ExtractObjectKeys extracts the keys of an object in a JSON string. The keys
+// are returned in the order they appear in the JSON string.
+func ExtractObjectKeys(b []byte) ([]string, error) {
+	if !bytes.HasPrefix(b, []byte{'{'}) {
+		return nil, fmt.Errorf("expected '{' at start of JSON object")
+	}
+
+	dec := json.NewDecoder(bytes.NewReader(b))
+	var keys []string
+
+	for dec.More() {
+		// Read prop name
+		t, err := dec.Token()
+		if err != nil {
+			log.Printf("Err: %v", err)
+			break
+		}
+
+		name, ok := t.(string)
+		if !ok {
+			continue // May be a delimeter
+		}
+
+		keys = append(keys, name)
+
+		var whatever nullMessage
+		dec.Decode(&whatever)
+	}
+
+	return keys, nil
+}
+
+// nullMessage implements json.Unmarshaler and does nothing with the given
+// value.
+type nullMessage struct{}
+
+func (*nullMessage) UnmarshalJSON(data []byte) error { return nil }

jsoninfo/orderedmap_test.go

@@ -0,0 +1,23 @@
+package jsoninfo
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestExtractObjectKeys(t *testing.T) {
+	const j = `{
+		"foo": {"bar": 1},
+		"baz": "qux",
+		"quux": "quuz"
+	}`
+
+	keys, err := ExtractObjectKeys([]byte(j))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if !reflect.DeepEqual(keys, []string{"foo", "baz", "quux"}) {
+		t.Fatalf("expected %v, got %v", []string{"foo", "baz", "quux"}, keys)
+	}
+}

openapi2conv/issue440_test.go

@@ -0,0 +1,48 @@
+package openapi2conv
+
+import (
+	"context"
+	"encoding/json"
+	"os"
+	"testing"
+
+	"github.com/getkin/kin-openapi/openapi2"
+	"github.com/getkin/kin-openapi/openapi3"
+	"github.com/stretchr/testify/require"
+)
+
+func TestIssue440(t *testing.T) {
+	doc2file, err := os.Open("testdata/swagger.json")
+	require.NoError(t, err)
+	defer doc2file.Close()
+	var doc2 openapi2.T
+	err = json.NewDecoder(doc2file).Decode(&doc2)
+	require.NoError(t, err)
+
+	doc3, err := ToV3(&doc2)
+	require.NoError(t, err)
+	err = doc3.Validate(context.Background())
+	require.NoError(t, err)
+	require.Equal(t, openapi3.Servers{
+		{URL: "https://petstore.swagger.io/v2"},
+		{URL: "http://petstore.swagger.io/v2"},
+	}, doc3.Servers)
+
+	doc2.Host = "your-bot-domain.de"
+	doc2.Schemes = nil
+	doc2.BasePath = ""
+	doc3, err = ToV3(&doc2)
+	require.NoError(t, err)
+	err = doc3.Validate(context.Background())
+	require.NoError(t, err)
+	require.Equal(t, openapi3.Servers{
+		{URL: "https://your-bot-domain.de/"},
+	}, doc3.Servers)
+
+	doc2.Host = "https://your-bot-domain.de"
+	doc2.Schemes = nil
+	doc2.BasePath = ""
+	doc3, err = ToV3(&doc2)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), `invalid host`)
+}

openapi2conv/openapi2_conv.go

@@ -26,11 +26,18 @@ func ToV3(doc2 *openapi2.T) (*openapi3.T, error) {
 	}
 
 	if host := doc2.Host; host != "" {
+		if strings.Contains(host, "/") {
+			err := fmt.Errorf("invalid host %q. This MUST be the host only and does not include the scheme nor sub-paths.", host)
+			return nil, err
+		}
 		schemes := doc2.Schemes
 		if len(schemes) == 0 {
-			schemes = []string{"https://"}
+			schemes = []string{"https"}
 		}
 		basePath := doc2.BasePath
+		if basePath == "" {
+			basePath = "/"
+		}
 		for _, scheme := range schemes {
 			u := url.URL{
 				Scheme: scheme,

openapi2conv/openapi2_conv_test.go

@@ -23,9 +23,9 @@ func TestConvOpenAPIV3ToV2(t *testing.T) {
 		require.NoError(t, err)
 	}
 
-	spec2, err := FromV3(&doc3)
+	doc2, err := FromV3(&doc3)
 	require.NoError(t, err)
-	data, err := json.Marshal(spec2)
+	data, err := json.Marshal(doc2)
 	require.NoError(t, err)
 	require.JSONEq(t, exampleV2, string(data))
 }
@@ -35,11 +35,11 @@ func TestConvOpenAPIV2ToV3(t *testing.T) {
 	err := json.Unmarshal([]byte(exampleV2), &doc2)
 	require.NoError(t, err)
 
-	spec3, err := ToV3(&doc2)
+	doc3, err := ToV3(&doc2)
 	require.NoError(t, err)
-	err = spec3.Validate(context.Background())
+	err = doc3.Validate(context.Background())
 	require.NoError(t, err)
-	data, err := json.Marshal(spec3)
+	data, err := json.Marshal(doc3)
 	require.NoError(t, err)
 	require.JSONEq(t, exampleV3, string(data))
 }

openapi2conv/testdata/swagger.json

@@ -0,0 +1 @@
+../../openapi2/testdata/swagger.json

openapi3/loader.go

@@ -14,7 +14,7 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/ghodss/yaml"
+	"github.com/ghodss/yaml/v2"
 )
 
 func foundUnresolvedRef(ref string) error {

openapi3/schema.go

@@ -9,6 +9,7 @@ import (
 	"math"
 	"math/big"
 	"regexp"
+	"sort"
 	"strconv"
 	"unicode/utf16"
 
@@ -150,14 +151,18 @@ type Schema struct {
 	// Object
 	Required                    []string       `json:"required,omitempty" yaml:"required,omitempty"`
 	Properties                  Schemas        `json:"properties,omitempty" yaml:"properties,omitempty"`
+	propertyKeys                []string       // order kept
 	MinProps                    uint64         `json:"minProperties,omitempty" yaml:"minProperties,omitempty"`
 	MaxProps                    *uint64        `json:"maxProperties,omitempty" yaml:"maxProperties,omitempty"`
 	AdditionalPropertiesAllowed *bool          `multijson:"additionalProperties,omitempty" json:"-" yaml:"-"` // In this order...
 	AdditionalProperties        *SchemaRef     `multijson:"additionalProperties,omitempty" json:"-" yaml:"-"` // ...for multijson
 	Discriminator               *Discriminator `json:"discriminator,omitempty" yaml:"discriminator,omitempty"`
 }
 
-var _ jsonpointer.JSONPointable = (*Schema)(nil)
+var (
+	_ jsonpointer.JSONPointable = (*Schema)(nil)
+	_ json.Unmarshaler          = (*Schema)(nil)
+)
 
 func NewSchema() *Schema {
 	return &Schema{}
@@ -168,7 +173,42 @@ func (schema *Schema) MarshalJSON() ([]byte, error) {
 }
 
 func (schema *Schema) UnmarshalJSON(data []byte) error {
-	return jsoninfo.UnmarshalStrictStruct(data, schema)
+	if err := jsoninfo.UnmarshalStrictStruct(data, schema); err != nil {
+		return err
+	}
+
+	var rawProperties struct {
+		Properties json.RawMessage `json:"properties"`
+	}
+
+	if err := json.Unmarshal(data, &rawProperties); err != nil {
+		return fmt.Errorf("failed to extract raw schema properties: %w", err)
+	}
+
+	if schema.Type == "object" && rawProperties.Properties != nil {
+		keys, _ := jsoninfo.ExtractObjectKeys(rawProperties.Properties)
+		schema.propertyKeys = keys
+	}
+
+	return nil
+}
+
+// OrderedPropertyKeys returns the keys of the properties in the order they were
+// defined. This is useful for generating code that needs to iterate over the
+// properties in a consistent order. If the keys could not be extracted for some
+// reason, then this method automatically sorts the keys to be deterministic.
+func (schema Schema) OrderedPropertyKeys() []string {
+	if schema.propertyKeys != nil {
+		return schema.propertyKeys
+	}
+
+	keys := make([]string, 0, len(schema.Properties))
+	for k := range schema.Properties {
+		keys = append(keys, k)
+	}
+
+	sort.Strings(keys)
+	return keys
 }
 
 func (schema Schema) JSONLookup(token string) (interface{}, error) {
@@ -1176,7 +1216,7 @@ func (schema *Schema) visitJSONString(settings *schemaValidationSettings, value
 			Value:       value,
 			Schema:      schema,
 			SchemaField: "pattern",
-			Reason:      fmt.Sprintf("string doesn't match the regular expression \"%s\"", schema.Pattern),
+			Reason:      fmt.Sprintf(`string doesn't match the regular expression "%s"`, schema.Pattern),
 		}
 		if !settings.multiError {
 			return err
@@ -1191,7 +1231,7 @@ func (schema *Schema) visitJSONString(settings *schemaValidationSettings, value
 			switch {
 			case f.regexp != nil && f.callback == nil:
 				if cp := f.regexp; !cp.MatchString(value) {
-					formatErr = fmt.Sprintf("string doesn't match the format %q (regular expression \"%s\")", format, cp.String())
+					formatErr = fmt.Sprintf(`string doesn't match the format %q (regular expression "%s")`, format, cp.String())
 				}
 			case f.regexp == nil && f.callback != nil:
 				if err := f.callback(value); err != nil {

openapi3/schema_test.go

@@ -1220,6 +1220,26 @@ components:
 	require.Contains(t, err.Error(), `Error at "/ownerName": Doesn't match schema "not"`)
 }
 
+func TestSchemaOrderedProperties(t *testing.T) {
+	const api = `
+openapi: "3.0.1"
+components:
+  schemas:
+    Pet:
+      properties:
+        z_name:
+          type: string
+        a_ownerName:
+          not:
+            type: boolean
+      type: object
+`
+	s, err := NewLoader().LoadFromData([]byte(api))
+	require.NoError(t, err)
+	require.NotNil(t, s)
+	require.Equal(t, []string{"z_name", "a_ownerName"}, s.Components.Schemas["Pet"].Value.propertyKeys)
+}
+
 func TestValidationFailsOnInvalidPattern(t *testing.T) {
 	schema := Schema{
 		Pattern: "[",