Skip to content

Commit 7776499

Browse files
committed
tune URL handling and refine tests
1 parent 0ebd59d commit 7776499

6 files changed

Lines changed: 44 additions & 22 deletions

File tree

cmd/html_report.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,8 @@ func generateHTMLReport(commits []*model.Commit, breakingConfig *whatChangedMode
9393
History: history,
9494
}
9595

96-
// For left/right comparisons, preserve explicit git-ref and URL labels while
97-
// keeping plain local files compact.
96+
// For left/right comparisons, preserve explicit git-ref labels, sanitize URL
97+
// labels, and keep plain local files compact.
9898
if len(args) == 2 && len(items) == 1 {
9999
payload.OriginalPath = displayLabelForHTML(args[0])
100100
payload.ModifiedPath = displayLabelForHTML(args[1])

cmd/html_report_test.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ func TestGenerateHTMLReport_LeftRightPreservesGitRefPaths(t *testing.T) {
7777
assert.Contains(t, content, `"modifiedPath":"HEAD:openapi.yaml"`)
7878
}
7979

80-
func TestGenerateHTMLReport_LeftRightPreservesURLPaths(t *testing.T) {
80+
func TestGenerateHTMLReport_LeftRightSanitizesURLPaths(t *testing.T) {
8181
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
8282
if r.URL.Path == "/left.yaml" {
8383
_, _ = w.Write([]byte("openapi: 3.0.3\ninfo:\n title: Left\n version: '1.0'\npaths: {}\n"))
@@ -87,8 +87,8 @@ func TestGenerateHTMLReport_LeftRightPreservesURLPaths(t *testing.T) {
8787
}))
8888
defer server.Close()
8989

90-
leftURL := server.URL + "/left.yaml"
91-
rightURL := server.URL + "/right.yaml"
90+
leftURL := server.URL + "/left.yaml?token=left-secret#frag"
91+
rightURL := server.URL + "/right.yaml?token=right-secret#frag"
9292

9393
commits, err := loadLeftRightCommits(leftURL, rightURL, summaryOpts{noColor: true})
9494
require.NoError(t, err)
@@ -98,8 +98,10 @@ func TestGenerateHTMLReport_LeftRightPreservesURLPaths(t *testing.T) {
9898
require.NotNil(t, report)
9999

100100
content := string(report)
101-
assert.Contains(t, content, `"originalPath":"`+leftURL+`"`)
102-
assert.Contains(t, content, `"modifiedPath":"`+rightURL+`"`)
101+
assert.Contains(t, content, `"originalPath":"`+server.URL+`/left.yaml"`)
102+
assert.Contains(t, content, `"modifiedPath":"`+server.URL+`/right.yaml"`)
103+
assert.NotContains(t, content, "left-secret")
104+
assert.NotContains(t, content, "right-secret")
103105
}
104106

105107
func TestBuildHTMLReportItems_AllCommitsFail(t *testing.T) {

cmd/left_right_sources.go

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -183,27 +183,29 @@ func resolveGitRefSource(raw, revision, filePath string, opts summaryOpts) (comp
183183
}
184184

185185
func resolveURLSource(raw string, opts summaryOpts) (comparisonSource, error) {
186+
display := sanitizeURLLabel(raw)
187+
186188
resp, err := httpGet(raw)
187189
if err != nil {
188-
return comparisonSource{}, fmt.Errorf("error downloading file '%s': %w", raw, err)
190+
return comparisonSource{}, fmt.Errorf("error downloading file '%s': %w", display, err)
189191
}
190192
defer resp.Body.Close()
191193
if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices {
192-
return comparisonSource{}, fmt.Errorf("error downloading file '%s': unexpected status %s", raw, resp.Status)
194+
return comparisonSource{}, fmt.Errorf("error downloading file '%s': unexpected status %s", display, resp.Status)
193195
}
194196

195197
const maxDownloadSize = 100 << 20 // 100 MB
196198
bits, err := io.ReadAll(io.LimitReader(resp.Body, maxDownloadSize))
197199
if err != nil {
198-
return comparisonSource{}, fmt.Errorf("error reading downloaded file '%s': %w", raw, err)
200+
return comparisonSource{}, fmt.Errorf("error reading downloaded file '%s': %w", display, err)
199201
}
200202
if len(bits) == 0 {
201-
return comparisonSource{}, fmt.Errorf("downloaded file '%s' is empty", raw)
203+
return comparisonSource{}, fmt.Errorf("downloaded file '%s' is empty", display)
202204
}
203205

204206
baseURL, err := url.Parse(raw)
205207
if err != nil {
206-
return comparisonSource{}, fmt.Errorf("invalid URL '%s': %w", raw, err)
208+
return comparisonSource{}, fmt.Errorf("invalid URL '%s': %w", display, err)
207209
}
208210
baseURL.Path = path.Dir(baseURL.Path)
209211
baseURL.RawQuery = ""
@@ -230,13 +232,28 @@ func resolveURLSource(raw string, opts summaryOpts) (comparisonSource, error) {
230232
}
231233

232234
return comparisonSource{
233-
Display: raw,
235+
Display: display,
234236
RootBytes: bits,
235237
DocConfig: docConfig,
236238
Cleanup: func() {},
237239
}, nil
238240
}
239241

242+
func sanitizeURLLabel(raw string) string {
243+
if !isHTTPURL(raw) {
244+
return raw
245+
}
246+
247+
parsed, err := url.Parse(raw)
248+
if err != nil || parsed == nil {
249+
return raw
250+
}
251+
parsed.User = nil
252+
parsed.RawQuery = ""
253+
parsed.Fragment = ""
254+
return parsed.String()
255+
}
256+
240257
func attachLazyLocalFS(docConfig *datamodel.DocumentConfiguration) error {
241258
if docConfig == nil || docConfig.BasePath == "" || !docConfig.AllowFileReferences || docConfig.LocalFS != nil {
242259
return nil

cmd/loaders.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ func displayLabelForHTML(raw string) string {
327327
return raw
328328
}
329329
if isHTTPURL(raw) {
330-
return raw
330+
return sanitizeURLLabel(raw)
331331
}
332332
return filepath.Base(raw)
333333
}

cmd/loaders_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -185,13 +185,13 @@ func TestResolveGitRefSource_RequiresGitRepo(t *testing.T) {
185185
assert.Contains(t, err.Error(), "requires the current working directory to be inside a git repository")
186186
}
187187

188-
func TestResolveComparisonSource_URLPreservesDisplay(t *testing.T) {
188+
func TestResolveComparisonSource_URLSanitizesDisplay(t *testing.T) {
189189
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
190190
_, _ = w.Write([]byte("openapi: 3.0.3\ninfo:\n title: test\n version: '1.0'\npaths: {}\n"))
191191
}))
192192
defer server.Close()
193193

194-
source, err := resolveComparisonSource(server.URL+"/spec.yaml", summaryOpts{})
194+
source, err := resolveComparisonSource(server.URL+"/spec.yaml?token=secret#section", summaryOpts{})
195195
require.NoError(t, err)
196196

197197
assert.Equal(t, server.URL+"/spec.yaml", source.Display)
@@ -223,7 +223,7 @@ func TestResolveComparisonSource_URLReturnsStatusErrors(t *testing.T) {
223223
assert.Contains(t, err.Error(), "unexpected status 404 Not Found")
224224
}
225225

226-
func TestLoadLeftRightCommits_PreservesRawDisplayLabels(t *testing.T) {
226+
func TestLoadLeftRightCommits_UsesSafeDisplayLabels(t *testing.T) {
227227
wd, err := os.Getwd()
228228
require.NoError(t, err)
229229
repoRoot := filepath.Clean(filepath.Join(wd, ".."))
@@ -257,7 +257,7 @@ func TestLoadLeftRightCommits_PreservesRawDisplayLabels(t *testing.T) {
257257
},
258258
{
259259
name: "url and local file",
260-
left: server.URL + "/spec.yaml",
260+
left: server.URL + "/spec.yaml?token=secret#section",
261261
right: "sample-specs/petstorev3.json",
262262
originalLabel: server.URL + "/spec.yaml",
263263
modifiedLabel: "sample-specs/petstorev3.json",

cmd/summary_test.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@ paths: {}
351351
assert.True(t, hasChanges)
352352
}
353353

354-
func TestLoadLeftRightCommits_DownloadedFilesPreserveRawLabels(t *testing.T) {
354+
func TestLoadLeftRightCommits_DownloadedFilesSanitizeURLLabels(t *testing.T) {
355355
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
356356
_, _ = w.Write([]byte(`openapi: 3.0.3
357357
info:
@@ -362,14 +362,17 @@ paths: {}
362362
}))
363363
defer server.Close()
364364

365-
leftURL := server.URL + "/left.yaml"
366-
rightURL := server.URL + "/right.yaml"
365+
leftURL := server.URL + "/left.yaml?token=left-secret#frag"
366+
rightURL := server.URL + "/right.yaml?token=right-secret#frag"
367367

368368
commits, err := loadLeftRightCommits(leftURL, rightURL, summaryOpts{})
369369

370370
require.NoError(t, err)
371371
require.Len(t, commits, 1)
372-
assert.Equal(t, "Original: "+leftURL+", Modified: "+rightURL, commits[0].Message)
372+
assert.Equal(t,
373+
"Original: "+server.URL+"/left.yaml, Modified: "+server.URL+"/right.yaml",
374+
commits[0].Message,
375+
)
373376
}
374377

375378
func TestRenderSummary_GitRefExplodedSpecDetectsSiblingChanges(t *testing.T) {

0 commit comments

Comments
 (0)