Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions index/rolodex.go
Original file line number Diff line number Diff line change
Expand Up @@ -905,6 +905,11 @@ func openFile(ctx context.Context, location string, v fs.FS) (fs.File, error) {
} else {
f, err = v.Open(location)
}
// a file system returning neither a file nor an error breaks the fs.FS contract,
// reading from the nil file would panic, so treat it as a failed open.
if f == nil && err == nil {
return nil, fmt.Errorf("file system returned no file and no error when opening '%s'", location)
}
return f, err
}

Expand Down
17 changes: 12 additions & 5 deletions index/rolodex_remote_loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -586,16 +586,17 @@ func (i *RemoteFS) OpenWithContext(ctx context.Context, remoteURL string) (fs.Fi
}

if remoteParsedURL.Scheme == "" {
i.releaseRemoteProcessingWaiter(processingWaiter, cacheKey, nil, nil)
return nil, nil // not a remote file — scheme is empty, skip processing.
schemeErr := fmt.Errorf("remote URL '%s' has no scheme, unable to fetch it", remoteURL)
i.releaseRemoteProcessingWaiter(processingWaiter, cacheKey, nil, schemeErr)
return nil, schemeErr
}

i.logger.Debug("[rolodex remote loader] loading remote file", "file", remoteURL, "remoteURL", remoteParsedURL.String())

response, clientErr := i.RemoteHandlerFunc(remoteParsedURL.String())
if clientErr != nil {
i.appendRemoteError(clientErr)
i.releaseRemoteProcessingWaiter(processingWaiter, cacheKey, nil, nil)
i.releaseRemoteProcessingWaiter(processingWaiter, cacheKey, nil, clientErr)
if response != nil && response.Body != nil {
_ = response.Body.Close()
}
Expand All @@ -607,8 +608,9 @@ func (i *RemoteFS) OpenWithContext(ctx context.Context, remoteURL string) (fs.Fi
return nil, clientErr
}
if response == nil {
i.releaseRemoteProcessingWaiter(processingWaiter, cacheKey, nil, nil)
return nil, fmt.Errorf("empty response from remote URL: %s", remoteParsedURL.String())
emptyErr := fmt.Errorf("empty response from remote URL: %s", remoteParsedURL.String())
i.releaseRemoteProcessingWaiter(processingWaiter, cacheKey, nil, emptyErr)
return nil, emptyErr
}
defer func() {
if response.Body != nil {
Expand Down Expand Up @@ -650,6 +652,11 @@ func (i *RemoteFS) normalizeRemoteURL(remoteParsedURL *url.URL) {
if i.rootURLParsed == nil || remoteParsedURL == nil {
return
}
// a base URL without a scheme or host (e.g. 'example.com/specs/') cannot be fetched from,
// rewriting with it would strip the scheme from every absolute remote reference.
if i.rootURLParsed.Scheme == "" || i.rootURLParsed.Host == "" {
return
}
remoteParsedURL.Host = i.rootURLParsed.Host
remoteParsedURL.Scheme = i.rootURLParsed.Scheme
}
Expand Down
108 changes: 103 additions & 5 deletions index/rolodex_remote_loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,16 +129,90 @@ func TestNewRemoteFS_BasicCheck_Valid(t *testing.T) {
}

func TestNewRemoteFS_BasicCheck_NoScheme(t *testing.T) {
server := test_buildServer()
defer server.Close()

remoteFS, _ := NewRemoteFSWithRootURL("")
remoteFS.RemoteHandlerFunc = test_httpClient.Get
var requested []string
remoteFS.RemoteHandlerFunc = func(u string) (*http.Response, error) {
requested = append(requested, u)
return &http.Response{
StatusCode: http.StatusOK,
Body: io.NopCloser(strings.NewReader("type: object")),
}, nil
}

// an empty root URL has no scheme or host, so the remote URL is fetched untouched.
file, err := remoteFS.Open("https://ding-dong-bing-bong.com/file1.yaml")

assert.NoError(t, err)
assert.NotNil(t, file)
assert.Equal(t, "https://ding-dong-bing-bong.com/file1.yaml", file.(*RemoteFile).GetFullPath())

// a location without a scheme cannot be fetched, it must fail instead of returning no file and no error.
file, err = remoteFS.Open("httpdocs/file1.yaml")
assert.Nil(t, file)
assert.EqualError(t, err, "remote URL 'httpdocs/file1.yaml' has no scheme, unable to fetch it")
assert.Equal(t, []string{"https://ding-dong-bing-bong.com/file1.yaml"}, requested)
}

func TestRemoteFS_OpenWithContext_NoSchemeReleasesWaitersWithError(t *testing.T) {
remoteFS, _ := NewRemoteFSWithRootURL("")
remoteFS.RemoteHandlerFunc = func(u string) (*http.Response, error) {
t.Errorf("a location without a scheme must never be fetched: %s", u)
return nil, errors.New("unexpected fetch")
}

// callers that wait on an in-flight open of the same location must receive the same error.
const workers = 32
for round := 0; round < 50; round++ {
start := make(chan struct{})
errs := make(chan error, workers)
for w := 0; w < workers; w++ {
go func() {
<-start
_, err := remoteFS.OpenWithContext(context.Background(), "httpdocs/shared.yaml")
errs <- err
}()
}
close(start)
for w := 0; w < workers; w++ {
assert.EqualError(t, <-errs, "remote URL 'httpdocs/shared.yaml' has no scheme, unable to fetch it")
}
}
}

func TestRemoteFS_OpenWithContext_FailedFetchReleasesWaitersWithError(t *testing.T) {
tests := []struct {
name string
response *http.Response
err error
expected string
}{
{name: "client error", err: errors.New("connection refused"), expected: "connection refused"},
{name: "no response", expected: "empty response from remote URL: https://example.com/shared.yaml"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
remoteFS, _ := NewRemoteFSWithRootURL("")
release := make(chan struct{})
remoteFS.RemoteHandlerFunc = func(string) (*http.Response, error) {
<-release
return tt.response, tt.err
}

// callers that wait on the in-flight fetch must receive its error, not a nil file and no error.
const workers = 16
errs := make(chan error, workers)
for w := 0; w < workers; w++ {
go func() {
_, err := remoteFS.OpenWithContext(context.Background(), "https://example.com/shared.yaml")
errs <- err
}()
}
time.Sleep(50 * time.Millisecond)
close(release)
for w := 0; w < workers; w++ {
assert.EqualError(t, <-errs, tt.expected)
}
})
}
}

func TestNewRemoteFS_BasicCheck_Relative(t *testing.T) {
Expand Down Expand Up @@ -559,6 +633,30 @@ func TestRemoteFS_NormalizeAndLoadCachedHelpers(t *testing.T) {
assert.Same(t, legacyFile, rfs.loadCachedRemoteFile("https://root.example/spec.yaml", "/spec.yaml"))
}

func TestRemoteFS_NormalizeRemoteURL_SkipsBaseWithoutSchemeOrHost(t *testing.T) {
tests := []struct {
name string
base string
}{
{name: "no scheme or host", base: "example.com/specs/"},
{name: "host without scheme", base: "//example.com/specs/"},
{name: "scheme without host", base: "file:///specs/"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
config := CreateOpenAPIIndexConfig()
config.BaseURL, _ = url.Parse(tt.base)
rfs, err := NewRemoteFSWithConfig(config)
assert.NoError(t, err)

target, err := url.Parse("https://other.example/schemas/pet.yaml")
assert.NoError(t, err)
rfs.normalizeRemoteURL(target)
assert.Equal(t, "https://other.example/schemas/pet.yaml", target.String())
})
}
}

func TestRemoteFS_CreateRemoteHelpers(t *testing.T) {
config := CreateOpenAPIIndexConfig()
rfs, err := NewRemoteFSWithConfig(config)
Expand Down
47 changes: 47 additions & 0 deletions index/rolodex_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3002,3 +3002,50 @@ func TestRolodex_Release_ConcurrentSafe(t *testing.T) {
_ = rolodex.GetIndexes()
<-done
}

// nilFileFS breaks the fs.FS contract by returning neither a file nor an error.
type nilFileFS struct{}

func (nilFileFS) Open(string) (fs.File, error) {
return nil, nil
}

func TestRolodex_Open_FileSystemReturnsNoFileAndNoError(t *testing.T) {
rolo := NewRolodex(CreateOpenAPIIndexConfig())
rolo.AddLocalFS(t.TempDir(), nilFileFS{})
rolo.AddRemoteFS("", nilFileFS{})

var rf RolodexFile
var err error
assert.NotPanics(t, func() {
rf, err = rolo.Open("https://example.com/spec.yaml")
})
assert.Nil(t, rf)
assert.EqualError(t, err, "file system returned no file and no error when opening 'https://example.com/spec.yaml'")

assert.NotPanics(t, func() {
rf, err = rolo.Open("spec.yaml")
})
assert.Nil(t, rf)
assert.EqualError(t, err, "file system returned no file and no error when opening 'spec.yaml'")
}

func TestRolodex_Open_HttpPrefixedLocationWithoutScheme(t *testing.T) {
cfg := CreateOpenAPIIndexConfig()
remoteFS, err := NewRemoteFSWithConfig(cfg)
assert.NoError(t, err)
remoteFS.RemoteHandlerFunc = func(u string) (*http.Response, error) {
t.Errorf("a location without a scheme must never be fetched: %s", u)
return nil, errors.New("unexpected fetch")
}
rolo := NewRolodex(cfg)
rolo.AddRemoteFS("", remoteFS)

// 'httpdocs/pet.yaml' is routed to the remote file system because it starts with 'http'.
var rf RolodexFile
assert.NotPanics(t, func() {
rf, err = rolo.Open("httpdocs/pet.yaml")
})
assert.Nil(t, rf)
assert.EqualError(t, err, "remote URL 'httpdocs/pet.yaml' has no scheme, unable to fetch it")
}
134 changes: 134 additions & 0 deletions issue578_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
// Copyright 2026 Princess Beef Heavy Industries / Dave Shanley
// https://pb33f.io
// SPDX-License-Identifier: MIT
package libopenapi

import (
"bytes"
"fmt"
"io"
"log/slog"
"net/http"
"net/url"
"sync"
"testing"

"github.com/pb33f/libopenapi/datamodel"
v3high "github.com/pb33f/libopenapi/datamodel/high/v3"
"github.com/pb33f/testify/require"
)

const issue578Spec = `openapi: 3.1.0
info:
title: t
version: 1.0.0
paths:
/x:
get:
responses:
"200":
description: ok
content:
application/json:
schema:
$ref: "%s"
`

func issue578Config(sequential bool, requested *[]string, mu *sync.Mutex) *datamodel.DocumentConfiguration {
cfg := datamodel.NewDocumentConfiguration()
cfg.AllowRemoteReferences = true
cfg.ExtractRefsSequentially = sequential
cfg.RemoteURLHandler = func(u string) (*http.Response, error) {
mu.Lock()
*requested = append(*requested, u)
mu.Unlock()
return &http.Response{
StatusCode: http.StatusOK,
Body: io.NopCloser(bytes.NewBufferString("type: object\n")),
}, nil
}
return cfg
}

// A BaseURL without a scheme must not strip the scheme from absolute remote refs. The rolodex
// used to receive no file and no error, then panic reading it; when refs were extracted
// concurrently the panic was re-raised on a fresh goroutine and killed the process.
func TestIssue578SchemelessBaseURLDoesNotPanic(t *testing.T) {
tests := []struct {
name string
base string
}{
{name: "no scheme or host", base: "example.com/specs/"},
{name: "host without scheme", base: "//example.com/specs/"},
}
for _, tt := range tests {
for _, sequential := range []bool{true, false} {
name := tt.name + " concurrent"
if sequential {
name = tt.name + " sequential"
}
t.Run(name, func(t *testing.T) {
var mu sync.Mutex
var requested []string
cfg := issue578Config(sequential, &requested, &mu)
baseURL, err := url.Parse(tt.base)
require.NoError(t, err)
cfg.BaseURL = baseURL

spec := []byte(fmt.Sprintf(issue578Spec, "https://example.com/schemas/pet.yaml"))
doc, err := NewDocumentWithConfiguration(spec, cfg)
require.NoError(t, err)

var model *DocumentModel[v3high.Document]
require.NotPanics(t, func() {
model, err = doc.BuildV3Model()
})
require.NoError(t, err)
require.NotNil(t, model)

mu.Lock()
require.Equal(t, []string{"https://example.com/schemas/pet.yaml"}, requested)
mu.Unlock()

pathItem, ok := model.Model.Paths.PathItems.Get("/x")
require.True(t, ok)
mediaType, ok := pathItem.Get.Responses.Codes.GetOrZero("200").Content.Get("application/json")
require.True(t, ok)
schema := mediaType.Schema.Schema()
require.NotNil(t, schema)
require.Equal(t, []string{"object"}, schema.Type)
})
}
}
}

// A local ref that merely starts with 'http' is routed to the remote file system, where it has no
// scheme. It must fail to resolve with an error rather than panic.
func TestIssue578HttpPrefixedLocalRefDoesNotPanic(t *testing.T) {
for _, sequential := range []bool{true, false} {
name := "concurrent"
if sequential {
name = "sequential"
}
t.Run(name, func(t *testing.T) {
var mu sync.Mutex
var requested []string
var logs bytes.Buffer
cfg := issue578Config(sequential, &requested, &mu)
cfg.Logger = slog.New(slog.NewTextHandler(&logs, &slog.HandlerOptions{Level: slog.LevelError}))

doc, err := NewDocumentWithConfiguration([]byte(fmt.Sprintf(issue578Spec, "httpdocs/pet.yaml")), cfg)
require.NoError(t, err)

require.NotPanics(t, func() {
_, err = doc.BuildV3Model()
})
require.EqualError(t, err, "component `httpdocs/pet.yaml` does not exist in the specification")
require.Contains(t, logs.String(), "remote URL 'httpdocs/pet.yaml' has no scheme, unable to fetch it")

mu.Lock()
require.Empty(t, requested)
mu.Unlock()
})
}
}
Loading