fix(index): never hand the rolodex a nil file with a nil error - #638
Merged
Merged
Conversation
A BaseURL without a scheme (e.g. "example.com/specs/" or "//example.com/specs/") made normalizeRemoteURL overwrite every remote ref's scheme with an empty one. RemoteFS.OpenWithContext then returned (nil, nil) for the scheme-less URL, and Rolodex.asRemoteFile passed the nil file to io.ReadAll, which panicked. When refs are extracted concurrently the panic happens inside singleflight, which re-raises it on a fresh goroutine, so the host process crashed with nothing able to recover. A local ref that merely starts with "http" (httpdocs/pet.yaml) reached the same (nil, nil) without any BaseURL. - normalizeRemoteURL only rewrites a remote URL when the base URL has both a scheme and a host; otherwise the ref is fetched as written. - OpenWithContext returns an error for a URL with no scheme, and hands that error to callers waiting on the same in-flight open. Client errors and empty responses are now passed to waiting callers too, instead of a nil file and no error. - openFile, which the rolodex uses for every file system including user-supplied ones, turns a (nil, nil) result into an error. Fixes #578 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #638 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 297 297
Lines 37662 37668 +6
=========================================
+ Hits 37662 37668 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #578. Supersedes #579.
Problem
RemoteFS.OpenWithContextcould return(nil, nil).Rolodex.asRemoteFilethen passed the nil file toconsumeAdaptedFile→io.ReadAll(nil)→ nil pointer panic. WithExtractRefsSequentially: falsethis runs insidesingleflight.Group.Do, which re-raises the panic on a new goroutine, so the host process crashes and the caller can'trecover.Reproduced on
main, both sequential (recoverable panic) and concurrent (process exit), via two triggers:normalizeRemoteURLcopies the base URL's scheme and host onto every remote ref unconditionally, soexample.com/specs/or//example.com/specs/stripshttpsfromhttps://example.com/schemas/pet.yaml.http, e.g.$ref: httpdocs/pet.yaml. No BaseURL is needed.Rolodex.OpenWithContextroutes anything with anhttpprefix to the remote FS, where it parses with an empty scheme.Fix
Three small layers:
normalizeRemoteURLhttps://host/...bases behave as before.RemoteFS.OpenWithContextremote URL '…' has no scheme, unable to fetch itinstead of(nil, nil), and passes that error to callers waiting on the same in-flight open. Client errors and empty responses are now passed to waiting callers too; before, they got a nil file with no error.openFile(rolodex)fs.FS, including user-supplied ones viaAddLocalFS/AddRemoteFS. It now converts(nil, nil)into an error, so a misbehaving custom FS can't crash callers either.Behavior change: a BaseURL with no scheme or no host is now ignored for normalization, the same as having no BaseURL. Absolute remote refs are fetched as written; previously they failed.
Why not #579
#579 has the right
normalizeRemoteURLidea. But it changesTestNewRemoteFS_BasicCheck_NoSchemeto fetch a non-existent host over the real network, and its new tests reachexample.com. It also leaves custom file systems and in-flight waiters able to hand back a nil file with no error.Tests
No test touches the network; they use canned
RemoteURLHandlers. Everything also passes withHTTP(S)_PROXYset to a dead port.issue578_test.go:TestIssue578SchemelessBaseURLDoesNotPaniccovers both base URL forms, sequential and concurrent. It asserts no panic and no error, that the handler is called once with the originalhttpsURL, and that the schema resolves.TestIssue578HttpPrefixedLocalRefDoesNotPaniccovers both modes. It asserts the exact build error and that the scheme error is logged.TestRemoteFS_NormalizeRemoteURL_SkipsBaseWithoutSchemeOrHostcovers no scheme or host, host without scheme, and scheme without host.TestRemoteFS_OpenWithContext_NoSchemeReleasesWaitersWithErrorruns 32 concurrent callers × 50 rounds.TestRemoteFS_OpenWithContext_FailedFetchReleasesWaitersWithError: waiters on a failed or empty fetch get its error. This fails onmain.TestRolodex_Open_FileSystemReturnsNoFileAndNoError: a customfs.FSreturning(nil, nil), local and remote.TestRolodex_Open_HttpPrefixedLocationWithoutScheme.TestNewRemoteFS_BasicCheck_NoSchemeis updated to the new contract with a canned handler.Each guard was switched off in turn to confirm its tests fail. Every changed line is covered:
openFile, bothOpenWithContextandnormalizeRemoteURLare at 100%. The new tests are race-clean at-count=3.🤖 Generated with Claude Code