Skip to content

fix: prevent panic when BaseURL has empty scheme in normalizeRemoteURL#579

Open
toller892 wants to merge 1 commit into
pb33f:mainfrom
toller892:fix/normalize-remote-url-empty-scheme
Open

fix: prevent panic when BaseURL has empty scheme in normalizeRemoteURL#579
toller892 wants to merge 1 commit into
pb33f:mainfrom
toller892:fix/normalize-remote-url-empty-scheme

Conversation

@toller892
Copy link
Copy Markdown

Description

normalizeRemoteURL unconditionally overwrites the remote URL's scheme with the BaseURL's scheme, even when BaseURL has an empty scheme. This clears the remote URL's scheme, leading to a (nil, nil) return at the empty-scheme check in OpenWithContext (line 588-590). Callers like consumeAdaptedFile then pass nil to io.ReadAll, causing an unrecoverable panic.

In some call chains this happens from a singleflight.Group.Do goroutine, making it impossible for the calling program to recover even with recover.

Changes

  1. normalizeRemoteURL: Skip normalization entirely when BaseURL lacks a host or scheme. This prevents the remote URL's scheme from being cleared by a malformed base URL.

  2. OpenWithContext: Return an error instead of (nil, nil) when the scheme is empty after normalization (defense-in-depth). This prevents io.ReadAll(nil) panics even if some other code path produces an empty scheme.

  3. Regression tests: Added TestNormalizeRemoteURL_PreservesSchemeWhenBaseURLHasEmptyScheme, TestNormalizeRemoteURL_OverwritesSchemeWhenBaseURLHasScheme, TestNormalizeRemoteURL_NoOpWhenBaseURLIsEmpty, and TestOpenWithContext_ReturnsErrorOnEmptyScheme.

Testing

All existing tests pass, including TestNewRemoteFS_BasicCheck_NoScheme (updated to expect an error instead of nil when the root URL is empty, since normalization is now a no-op and the URL is fetched normally).

ok  github.com/pb33f/libopenapi/index   71.390s

Fixes #578

normalizeRemoteURL unconditionally overwrote the remote URL's scheme with
the BaseURL's scheme, even when the BaseURL had an empty scheme. This caused
the remote URL's scheme to be cleared, leading to a (nil, nil) return at the
empty-scheme check. Callers like consumeAdaptedFile then passed nil to
io.ReadAll, causing an unrecoverable panic.

Changes:
- normalizeRemoteURL now skips normalization when BaseURL lacks host or scheme
- OpenWithContext returns an error instead of (nil, nil) when scheme is empty
  (defense-in-depth)
- Added regression tests for empty-scheme BaseURL and empty root URL scenarios

Fixes pb33f#578
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 2, 2026

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 99.71%. Comparing base (5957faa) to head (3698b1b).

Files with missing lines Patch % Lines
index/rolodex_remote_loader.go 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #579      +/-   ##
==========================================
- Coverage   99.72%   99.71%   -0.01%     
==========================================
  Files         280      280              
  Lines       33853    33855       +2     
==========================================
- Hits        33761    33760       -1     
- Misses         55       57       +2     
- Partials       37       38       +1     
Flag Coverage Δ
unittests 99.71% <66.66%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@daveshanley
Copy link
Copy Markdown
Member

One single line.. you can do it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

a malformed BaseURI can lead to unrecoverable panics

2 participants