Fix/align url normalization

expander.go

@@ -583,7 +583,7 @@ func expandPathItem(pathItem *PathItem, resolver *schemaLoader, basePath string)
 
 	pathItem.Ref = Ref{}
 	for i := range pathItem.Parameters {
-		if err := expandParameterOrResponse(&(pathItem.Parameters[i]), resolver, basePath); resolver.shouldStopOnError(err) {
+		if err := expandParameterOrResponse(&pathItem.Parameters[i], resolver, basePath); resolver.shouldStopOnError(err) {
 			return err
 		}
 	}

go.mod

@@ -2,19 +2,19 @@ module github.com/go-openapi/spec
 
 require (
 	github.com/go-openapi/jsonpointer v1.0.0
-	github.com/go-openapi/jsonreference v1.0.0
-	github.com/go-openapi/swag/conv v0.29.0
-	github.com/go-openapi/swag/jsonutils v0.29.0
-	github.com/go-openapi/swag/loading v0.29.0
-	github.com/go-openapi/swag/stringutils v0.29.0
+	github.com/go-openapi/jsonreference v1.0.1
+	github.com/go-openapi/swag/conv v0.29.1
+	github.com/go-openapi/swag/jsonutils v0.29.1
+	github.com/go-openapi/swag/loading v0.29.1
+	github.com/go-openapi/swag/stringutils v0.29.1
 	github.com/go-openapi/testify/enable/yaml/v2 v2.6.1
 	github.com/go-openapi/testify/v2 v2.6.1
 )
 
 require (
-	github.com/go-openapi/swag/pools v0.29.0 // indirect
-	github.com/go-openapi/swag/typeutils v0.29.0 // indirect
-	github.com/go-openapi/swag/yamlutils v0.29.0 // indirect
+	github.com/go-openapi/swag/pools v0.29.1 // indirect
+	github.com/go-openapi/swag/typeutils v0.29.1 // indirect
+	github.com/go-openapi/swag/yamlutils v0.29.1 // indirect
 	go.yaml.in/yaml/v3 v3.0.5 // indirect
 )
 

go.sum

@@ -1,23 +1,23 @@
 github.com/go-openapi/jsonpointer v1.0.0 h1:kR9tHqY0CtZaOPVFm622dPVNhrvYpwr4uCxgL3h1H8s=
 github.com/go-openapi/jsonpointer v1.0.0/go.mod h1:Z3rw7dWu1p9IgitXCFamSlA5lmDiklEB6vkaxcNZW5Y=
-github.com/go-openapi/jsonreference v1.0.0 h1:jlmTr6torcd1YgDQvSfNmRtKzYDO4FGBkrAdlAVWnpY=
-github.com/go-openapi/jsonreference v1.0.0/go.mod h1:jtwdyGbJk0Xhe5Y+rwtglQP6Sb1WZST4rT32LWB+sv0=
-github.com/go-openapi/swag/conv v0.29.0 h1:4+1TogWpOIzMPzVKrvx1BfqBYlApB7D7DW3EAWpwmp4=
-github.com/go-openapi/swag/conv v0.29.0/go.mod h1:ch1l7V87F6zQXuLs5s0RFvrro6aFvrVcfVXn2PTZnu8=
-github.com/go-openapi/swag/jsonutils v0.29.0 h1:Xgnf9g32ycQjQUnDxkhqLraH2FhitcSE3w7ayQB3TgA=
-github.com/go-openapi/swag/jsonutils v0.29.0/go.mod h1:5WYmjf6hJcBve+ArzBaUsYy4M1GXsgjIQTmwJKfZHrA=
-github.com/go-openapi/swag/jsonutils/fixtures_test v0.29.0 h1:bpSF6LFkJJVtaRtJCzbZADVPVHQYKPwPdKthOQA2/5o=
-github.com/go-openapi/swag/jsonutils/fixtures_test v0.29.0/go.mod h1:julgTUKZ9/D0j6O7GKajmRs+812FWxQg/mMpGunWSjg=
-github.com/go-openapi/swag/loading v0.29.0 h1:r1lg2DQbT1VgBwgiPYXBM059RNswFI6r36CC0QCcRGw=
-github.com/go-openapi/swag/loading v0.29.0/go.mod h1:l/Z4MNbom0jSqzvWJqK2VUUWEceBknGEuVbLHLq4KN0=
-github.com/go-openapi/swag/pools v0.29.0 h1:uMQcoJeHJ8fWkdfEXJZMMpqk6hpfW8qTL5Q/IoRFFII=
-github.com/go-openapi/swag/pools v0.29.0/go.mod h1:leDcaghjkRAhCuCRv9NfJU5f0mjoU3cT/XZObhMk3pc=
-github.com/go-openapi/swag/stringutils v0.29.0 h1:/IEOuZ7PGJi6lqgH83dVt7/A9eHsDGEH1459lm+gpEo=
-github.com/go-openapi/swag/stringutils v0.29.0/go.mod h1:7fSqZ+z8Qc0tOfAAK0jVa5qFGrnIlRi6n7NeGGrr1vc=
-github.com/go-openapi/swag/typeutils v0.29.0 h1:HrWCYZeXVVNDo/7QQPRaYk33XeIDxksbxpalID3bWR8=
-github.com/go-openapi/swag/typeutils v0.29.0/go.mod h1:hxpgDZJVBkBsi/d3MIUosafoFdE5exaQRmVp0zwu3YE=
-github.com/go-openapi/swag/yamlutils v0.29.0 h1:JOKKuhMnBx4HYTM+kPEYw8S5YKKU9PnC4Mwb+c69BBA=
-github.com/go-openapi/swag/yamlutils v0.29.0/go.mod h1:/+FVozjFWZzku6mRz5U/Qmq5Yk8PLFxBLLWA/jHaxYE=
+github.com/go-openapi/jsonreference v1.0.1 h1:4zJ7AmYDKNmD3aSpfPnFNCFA5E80/xMHUNKgydaLh38=
+github.com/go-openapi/jsonreference v1.0.1/go.mod h1:dYplQXa6p5lXprLcJ8LE2iU7vNpXsAHDQ5ZAgL+Qx3A=
+github.com/go-openapi/swag/conv v0.29.1 h1:AC4Eh/5c/eUDOUCzzsRC9ghmFgOSBHeRMGIngY0ZUGA=
+github.com/go-openapi/swag/conv v0.29.1/go.mod h1:S1X7/ZrBEZOC0Wc8AGxjbcGS92l3WEjA7aPtpl+RaqM=
+github.com/go-openapi/swag/jsonutils v0.29.1 h1:AFCxs0eQZ24/QyfhVHM2t49rMz7Vv3XCsZQI6yrNy+c=
+github.com/go-openapi/swag/jsonutils v0.29.1/go.mod h1:u3+sCfJpttDpcmS5kpm0yxL6GK0eWgODsx8Yw8fcqNM=
+github.com/go-openapi/swag/jsonutils/fixtures_test v0.29.1 h1:BiiXE31Bx9SfpsMmOQj5KYpUhTZBpLVriVhJDuLuY2o=
+github.com/go-openapi/swag/jsonutils/fixtures_test v0.29.1/go.mod h1:julgTUKZ9/D0j6O7GKajmRs+812FWxQg/mMpGunWSjg=
+github.com/go-openapi/swag/loading v0.29.1 h1:FCv5fG8UhTdDJa2R7w+5O9Ekpcbw7tt0nFWvmDKGBjc=
+github.com/go-openapi/swag/loading v0.29.1/go.mod h1:N0ESuem4p2oedKal8EJhciqnJ9Q9Wmt83L1CRB3Fouw=
+github.com/go-openapi/swag/pools v0.29.1 h1:NRogYxdEW9SjRM4mkAOji9iefO4MRXq3p/ZJcoQbUKg=
+github.com/go-openapi/swag/pools v0.29.1/go.mod h1:leDcaghjkRAhCuCRv9NfJU5f0mjoU3cT/XZObhMk3pc=
+github.com/go-openapi/swag/stringutils v0.29.1 h1:1ykunK7iJQk1uOO7+oUH1ukbsK85fFCOiCFMOVSY+F0=
+github.com/go-openapi/swag/stringutils v0.29.1/go.mod h1:7fSqZ+z8Qc0tOfAAK0jVa5qFGrnIlRi6n7NeGGrr1vc=
+github.com/go-openapi/swag/typeutils v0.29.1 h1:Nzv9nhnlLCRBPQqfOX+7lB6Guju370or8StT+lIOf6M=
+github.com/go-openapi/swag/typeutils v0.29.1/go.mod h1:hxpgDZJVBkBsi/d3MIUosafoFdE5exaQRmVp0zwu3YE=
+github.com/go-openapi/swag/yamlutils v0.29.1 h1:69w3tsBajm7MR/fejLy7HD/3J68Ys1SeeZMEzZ3w2sk=
+github.com/go-openapi/swag/yamlutils v0.29.1/go.mod h1:rgsp3vT/QdWzKwn43CigDwjOGIenPyTZMKnxEM8jZOA=
 github.com/go-openapi/testify/enable/yaml/v2 v2.6.1 h1:Jm+/ze2rMtbD98yen92AhATGLGREDYXG56Xr4gMjEtE=
 github.com/go-openapi/testify/enable/yaml/v2 v2.6.1/go.mod h1:YDPnwCRDu38/oJBVMBVXOUDiJ9cIeBHWvfImHaXqnv4=
 github.com/go-openapi/testify/v2 v2.6.1 h1:6CNJhTjMzgaeaH8WhshcsZNPIvRemiOcFpU7seO/y7Q=

normalizer.go

@@ -41,10 +41,11 @@ func normalizeURI(refPath, base string) string {
 	if refURL.Path == "." {
 		refURL.Path = ""
 	}
+	forgetSpelling(refURL)
 
 	r := MustCreateRef(refURL.String())
 	if r.IsCanonical() {
-		return refURL.String()
+		return r.String()
 	}
 
 	baseURL, _ := parseURL(base)
@@ -252,11 +253,12 @@ func normalizeBase(in string) string {
 	if u.Path == "." { // empty after Clean()
 		u.Path = ""
 	}
+	forgetSpelling(u)
 
 	if u.Scheme != "" {
 		if path.IsAbs(u.Path) || u.Scheme != fileScheme {
 			// this is absolute or explicitly not a local file: we're good
-			return u.String()
+			return canonicalString(u)
 		}
 	}
 
@@ -269,5 +271,41 @@ func normalizeBase(in string) string {
 	u.Scheme = fileScheme
 	u.Path = absPath(u.Path) // platform-dependent
 	u.RawQuery = ""          // any query component is irrelevant for a base
-	return u.String()
+	return canonicalString(u)
+}
+
+// forgetSpelling drops the escaped spelling url.Parse recorded, so that rendering the URL
+// derives it from Path and Fragment again.
+//
+// url.URL.String returns RawPath whenever it still decodes to Path, and the normalizer rewrites
+// Path - path.Clean, then a join onto the base. A RawPath describing the path as it was written
+// no longer describes the path we now hold, and rendering it can produce a URI that does not
+// parse: normalizeBase("%2F") used to return "file://%2F", where the escaped form lost the
+// leading slash that Path carries and what is left reads as an authority.
+func forgetSpelling(u *url.URL) {
+	u.RawPath = ""
+	u.RawFragment = ""
+}
+
+// canonicalString renders a URL the way jsonreference does.
+//
+// The normalizer and jsonreference each canonicalize a URI, and they used to disagree: the
+// normalizer kept the spelling it was handed while a Ref lower-cases the host, drops a default
+// port and re-escapes path and fragment. Everything the expander stores - cache keys, the
+// parentRefs chain, a $ref written back into the document - goes through both, so the two must
+// agree or one document splits into two spellings of itself.
+//
+// A URI that jsonreference cannot parse is returned as it stands, rather than panicking through
+// MustCreateRef. Such a URI is a defect in the normalizer, and the fuzz target reports it as one.
+func canonicalString(u *url.URL) string {
+	rendered := u.String()
+
+	r, err := NewRef(rendered)
+	if err != nil {
+		specLogger.Printf("warning: the normalizer produced an unparseable URI %q: %v", rendered, err)
+
+		return rendered
+	}
+
+	return r.String()
 }

normalizer_canonical_test.go

@@ -0,0 +1,224 @@
+// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers
+// SPDX-License-Identifier: Apache-2.0
+
+package spec
+
+import (
+	"testing"
+
+	"github.com/go-openapi/testify/v2/assert"
+	"github.com/go-openapi/testify/v2/require"
+)
+
+// TestNormalizer_Canonicalization pins the rules the normalizer and jsonreference both apply,
+// on the edge cases where they used to part company.
+//
+// Two canonicalizers meet on every $ref. The normalizer builds a URI out of a $ref and a base;
+// jsonreference re-renders it whenever the string becomes a Ref, which happens as soon as the
+// document is unmarshalled. When the two disagreed, one document could be cached under two
+// spellings of itself, and a $ref written back by denormalizeRef did not normalize to what it
+// came from. normalizeURI and normalizeBase now end in canonicalString, so both sides answer
+// with jsonreference's spelling.
+func TestNormalizer_Canonicalization(t *testing.T) {
+	t.Parallel()
+
+	const base = "https://example.com/base/spec.json"
+
+	tests := []struct {
+		name     string
+		rule     string
+		refPath  string
+		expected string
+	}{
+		{
+			name:     "host case",
+			rule:     "the host is lower-cased, the path is not",
+			refPath:  "https://EXAMPLE.com/OTHER.json",
+			expected: "https://example.com/OTHER.json",
+		},
+		{
+			name:     "default port",
+			rule:     ":443 comes off an https URL",
+			refPath:  "https://example.com:443/other.json",
+			expected: "https://example.com/other.json",
+		},
+		{
+			name:     "non-default port",
+			rule:     "any other port stays",
+			refPath:  "https://example.com:8443/other.json",
+			expected: "https://example.com:8443/other.json",
+		},
+		{
+			name:     "IPv6 literal",
+			rule:     "a bracketed host is lower-cased and loses its default port",
+			refPath:  "https://[2001:DB8::1]:443/other.json",
+			expected: "https://[2001:db8::1]/other.json",
+		},
+		{ //nolint:gosec // test URLs carrying userinfo, not credentials
+			name:     "userinfo",
+			rule:     "the colon in userinfo is not a port",
+			refPath:  "https://user:pw@EXAMPLE.com:443/other.json",
+			expected: "https://user:pw@example.com/other.json",
+		},
+		{
+			name:     "degenerate authority, port kept",
+			rule:     "url.Parse reads the host as \":a\" on port 443, and dropping the port would leave a URI that no longer parses",
+			refPath:  "https://:a:443/other.json",
+			expected: "https://:a:443/other.json",
+		},
+		{
+			name:     "degenerate authority, port twice",
+			rule:     "the host \"0:443\" spells a default port of its own, so removal repeats",
+			refPath:  "https://0:443:443/other.json",
+			expected: "https://0/other.json",
+		},
+		{
+			name:     "duplicate slashes, relative",
+			rule:     "a run of slashes collapses to one, however long",
+			refPath:  "a//b///c////d.json",
+			expected: "https://example.com/base/a/b/c/d.json",
+		},
+		{
+			name:     "duplicate slashes, absolute",
+			rule:     "same, on a $ref that carries its own host",
+			refPath:  "https://example.com/a//b///c////d.json",
+			expected: "https://example.com/a/b/c/d.json",
+		},
+		{
+			name:     "unreserved character, escaped",
+			rule:     "%41 is decoded: RFC 3986 §6.2.2.2 allows decoding an unreserved character",
+			refPath:  "https://example.com/%41.json",
+			expected: "https://example.com/A.json",
+		},
+		{
+			name:     "reserved character, escaped",
+			rule:     "%2F is decoded too, which changes what the URI denotes - see the note below",
+			refPath:  "https://example.com/a%2Fb.json",
+			expected: "https://example.com/a/b.json",
+		},
+		{
+			name:     "fragment, unescaped",
+			rule:     "a quote in a JSON pointer is escaped on the way out",
+			refPath:  "other.json#/definitions/it's",
+			expected: "https://example.com/base/other.json#/definitions/it%27s",
+		},
+		{
+			name:     "fragment, escaped",
+			rule:     "and the escaped spelling is left as it is, so the two agree",
+			refPath:  "other.json#/definitions/it%27s",
+			expected: "https://example.com/base/other.json#/definitions/it%27s",
+		},
+		{
+			name:     "fragment, brackets",
+			rule:     "same for a definition named o[k]",
+			refPath:  "other.json#/definitions/o[k]",
+			expected: "https://example.com/base/other.json#/definitions/o%5Bk%5D",
+		},
+		{
+			name:     "fragment, space",
+			rule:     "and for one named \"a b\"",
+			refPath:  "other.json#/definitions/a b",
+			expected: "https://example.com/base/other.json#/definitions/a%20b",
+		},
+	}
+
+	canonicalBase := normalizeBase(base)
+	require.EqualT(t, base, canonicalBase, "the base of these cases is already canonical")
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+
+			normalized := normalizeURI(test.refPath, canonicalBase)
+			assert.EqualTf(t, test.expected, normalized, "%s: %s", test.name, test.rule)
+
+			// the rule that matters: what the normalizer returns is what jsonreference would
+			// have made of it, so turning it into a Ref changes nothing
+			ref := MustCreateRef(normalized)
+			assert.EqualTf(t, normalized, ref.String(),
+				"%q is not a fixpoint of jsonreference's canonicalization", normalized)
+
+			assert.EqualTf(t, normalized, normalizeURI(normalized, canonicalBase),
+				"normalizing %q again changed it", normalized)
+		})
+	}
+}
+
+// TestNormalizer_CanonicalBase pins the same rules on the base, which normalizeBase canonicalizes
+// too, and covers the rendering defect that produced an unparseable base.
+func TestNormalizer_CanonicalBase(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name     string
+		rule     string
+		base     string
+		expected string
+	}{
+		{
+			name:     "host case and default port",
+			rule:     "a base is canonicalized like a $ref, so two spellings key one document",
+			base:     "https://EXAMPLE.com:443/base/spec.json",
+			expected: "https://example.com/base/spec.json",
+		},
+		{
+			name:     "duplicate slashes",
+			rule:     "path.Clean already collapsed these; the rule is pinned here too",
+			base:     "https://example.com/base//sub///spec.json",
+			expected: "https://example.com/base/sub/spec.json",
+		},
+		{
+			name:     "fragment dropped",
+			rule:     "a fragment in the base names nothing",
+			base:     "https://example.com/base/spec.json#/definitions/x",
+			expected: "https://example.com/base/spec.json",
+		},
+		{
+			name: "escaped path rendering",
+			rule: `"%2F" decodes to "/", and the escaped form has no leading slash to render, ` +
+				`so it used to come out as the authority of "file://%2F"`,
+			base:     "%2F",
+			expected: "file:///",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+
+			canonical := normalizeBase(test.base)
+			assert.EqualTf(t, test.expected, canonical, "%s: %s", test.name, test.rule)
+
+			ref := MustCreateRef(canonical)
+			assert.EqualTf(t, canonical, ref.String(),
+				"%q is not a fixpoint of jsonreference's canonicalization", canonical)
+
+			assert.EqualTf(t, canonical, normalizeBase(canonical),
+				"normalizing %q again changed it", canonical)
+		})
+	}
+}
+
+// TestNormalizer_EscapedSlashIsDecoded records a rule that is not ours to change here.
+//
+// jsonreference clears url.URL.RawPath, so "%2F" comes back as a separator. RFC 3986 §6.2.2.2
+// allows decoding an unreserved character and forbids decoding a reserved one, so this is a
+// change of meaning rather than of spelling: the GitLab API requires the project path
+// percent-encoded, and the decoded URL names a different endpoint.
+//
+// It is pinned rather than fixed because a $ref loses the escape anyway - the document model
+// turns every $ref into a Ref, and jsonreference decodes it there. Making the normalizer keep
+// the escape would only hide that. The fix belongs upstream.
+func TestNormalizer_EscapedSlashIsDecoded(t *testing.T) {
+	t.Parallel()
+
+	const gitlab = "https://gitlab.com/api/v4/projects/mygroup%2Fmyproject/repository/files/swagger.json/raw"
+	const decoded = "https://gitlab.com/api/v4/projects/mygroup/myproject/repository/files/swagger.json/raw"
+
+	var ref Ref
+	require.NoError(t, ref.UnmarshalJSON([]byte(`{"$ref":"`+gitlab+`"}`)))
+	assert.EqualT(t, decoded, ref.String(), "the document model decodes the escape on its own")
+
+	assert.EqualT(t, decoded, normalizeBase(gitlab))
+	assert.EqualT(t, decoded, normalizeURI(gitlab, normalizeBase("https://example.com/spec.json")))
+}