Bug source: normalizeRemoteURL unconditionally overwrites the ref's scheme with the BaseURL's, even when BaseURL has an empty scheme. The downstream early-return at rolodex_remote_loader.go:588-591 should either fall back to the RemoteURLHandler or return an error rather than (nil, nil). Because of this (nil, nil) return, Rolodex.openRemoteLocation returns a nil reader to consumeAdaptedFile, which crashes at io.ReadAll(nil).
In some call chains, this can happen synchronously, from the goroutine that calls document.BuildV3Model. This is not ideal, but at least the caller could work-around potential issues like this by recovering from a panic.
However, there are other call chains where it happens from a separate goroutine related to the use of singleflight.Group.Do to de-duplicate fetches. In this case, there is nothing the calling program can do to recover from the panic.
While a malformed BaseURL could be viewed as user-error, it's bad when it causes unrecoverable panics in the calling program. The BaseURL could be eagerly validated and have the operation fail fast with a reasonable error when it is malformed. But having normalizeRemoteURL do something different is also appropriate IMO (such as only conditionally overwrite the ref's scheme if the BaseURL has a schema, or have it call the RemoteURLHandler even if the scheme is empty, or have it produce an error due to the missing scheme).
Here are example call chains for both paths:
# Synchronous/single goroutine
document.BuildV3Model
└─ v3.CreateDocumentFromConfig
└─ v3.createDocument (low/v3/create_document.go:287)
└─ Rolodex.CheckForCircularReferences (index/rolodex.go:561)
└─ Resolver.CheckForCircularReferences (index/resolver_entry.go:199)
└─ visitIndexWithoutDamagingIt (index/resolver_visit.go:22)
└─ Resolver.VisitReference (index/resolver_visit.go:162)
└─ Resolver.collectReferenceRelatives (index/resolver_mutation.go:36)
└─ Resolver.extractRelatives (...resolver_relatives.go:27)
└─ ... extractRelativesWithState
└─ Resolver.extractRelativeReference (resolver_relatives.go:150)
└─ Resolver.searchReferenceWithContext (resolver_visit.go:113)
└─ SpecIndex.SearchIndexForReferenceByReferenceWithContext (search_index.go:278)
└─ Rolodex.OpenWithContext (rolodex.go:720)
└─ Rolodex.openRemoteLocation (rolodex.go:828) ← returns nil reader
└─ Rolodex.asRemoteFile (rolodex.go:842)
└─ consumeAdaptedFile (rolodex.go:865)
└─ io.ReadAll(nil) ← PANIC
# Unrecoverable/child goroutine
SpecIndex.ExtractComponentsFromRefs (index/extract_refs_lookup.go:108)
└─ go func1 (extract_refs_lookup.go:113) ← spawned
└─ singleflight.Group.Do (singleflight.go:113)
└─ singleflight.Group.doCall (singleflight.go:200)
└─ goroutine, doCall.func2 (singleflight.go:198) ← second goroutine
└─ ExtractComponentsFromRefs.func1.2 (extract_refs_lookup.go:121)
└─ SpecIndex.locateRef (extract_refs_lookup.go:205)
└─ SpecIndex.FindComponent (find_component_entry.go:50)
└─ SpecIndex.lookupRolodex (find_component_external.go:44)
└─ Rolodex.OpenWithContext (rolodex.go:720)
└─ Rolodex.openRemoteLocation (rolodex.go:828) ← same nil-reader bug
└─ Rolodex.asRemoteFile (rolodex.go:842)
└─ consumeAdaptedFile (rolodex.go:865)
└─ io.ReadAll(nil) ← PANIC, on worker goroutine
singleflight.doCall.func1 (line 170) installs its own defer recover(), but it converts the recovered value into a *panicError and re-panics on a fresh goroutine at singleflight.go:205. That re-panic propagates to the Go runtime with no caller in scope to recover it — the program exits.
The simplest way to trigger the differing behaviors above appears to be by setting ExtractRefsSequentially to true or false. Though when we encountered the synchronous approach initially, we didn't change that value but instead were testing with the Square API and its spec seems to be just-right-shaped to somehow avoid the panic when extracting component refs only to have it blow up from the calling goroutine when checking for cycles.
Versions confirmed: github.com/pb33f/libopenapi v0.37.0, golang.org/x/sync v0.20.0, Go 1.26.3 (darwin/arm64).
Bug source:
normalizeRemoteURLunconditionally overwrites the ref's scheme with theBaseURL's, even whenBaseURLhas an empty scheme. The downstream early-return atrolodex_remote_loader.go:588-591should either fall back to theRemoteURLHandleror return an error rather than(nil, nil). Because of this(nil, nil)return,Rolodex.openRemoteLocationreturns a nil reader toconsumeAdaptedFile, which crashes atio.ReadAll(nil).In some call chains, this can happen synchronously, from the goroutine that calls
document.BuildV3Model. This is not ideal, but at least the caller could work-around potential issues like this byrecovering from a panic.However, there are other call chains where it happens from a separate goroutine related to the use of
singleflight.Group.Doto de-duplicate fetches. In this case, there is nothing the calling program can do to recover from the panic.While a malformed
BaseURLcould be viewed as user-error, it's bad when it causes unrecoverable panics in the calling program. TheBaseURLcould be eagerly validated and have the operation fail fast with a reasonable error when it is malformed. But havingnormalizeRemoteURLdo something different is also appropriate IMO (such as only conditionally overwrite the ref's scheme if theBaseURLhas a schema, or have it call theRemoteURLHandlereven if the scheme is empty, or have it produce an error due to the missing scheme).Here are example call chains for both paths:
singleflight.doCall.func1(line 170) installs its owndefer recover(), but it converts the recovered value into a*panicErrorand re-panics on a fresh goroutine atsingleflight.go:205. That re-panic propagates to the Go runtime with no caller in scope to recover it — the program exits.The simplest way to trigger the differing behaviors above appears to be by setting
ExtractRefsSequentiallyto true or false. Though when we encountered the synchronous approach initially, we didn't change that value but instead were testing with the Square API and its spec seems to be just-right-shaped to somehow avoid the panic when extracting component refs only to have it blow up from the calling goroutine when checking for cycles.Versions confirmed: github.com/pb33f/libopenapi v0.37.0, golang.org/x/sync v0.20.0, Go 1.26.3 (darwin/arm64).