Skip to content

Commit 187d114

Browse files
author
Paul Schwabauer
authored
Remove unnecessary URL joins (gocsaf#676)
This should avoid bugs for more complex scenarios.
1 parent 1a2a8fa commit 187d114

File tree

5 files changed

+41
-70
lines changed

5 files changed

+41
-70
lines changed

‎cmd/csaf_aggregator/mirror.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,17 +67,16 @@ func (w *worker) mirrorInternal() (*csaf.AggregatorCSAFProvider, error) {
6767
// Collecting the categories per label.
6868
w.categories = map[string]util.Set[string]{}
6969

70-
base, err := url.Parse(w.loc)
70+
pmdURL, err := url.Parse(w.loc)
7171
if err != nil {
7272
return nil, err
7373
}
74-
base.Path = ""
7574

7675
afp := csaf.NewAdvisoryFileProcessor(
7776
w.client,
7877
w.expr,
7978
w.metadataProvider,
80-
base)
79+
pmdURL)
8180

8281
afp.AgeAccept = w.provider.ageAccept(w.processor.cfg)
8382

‎cmd/csaf_checker/processor.go‎

Lines changed: 18 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -628,14 +628,9 @@ var yearFromURL = regexp.MustCompile(`.*/(\d{4})/[^/]+$`)
628628
// mistakes, from conforming filenames to invalid advisories.
629629
func (p *processor) integrity(
630630
files []csaf.AdvisoryFile,
631-
base string,
632631
mask whereType,
633632
lg func(MessageType, string, ...any),
634633
) error {
635-
b, err := url.Parse(base)
636-
if err != nil {
637-
return err
638-
}
639634
client := p.httpClient()
640635

641636
var data bytes.Buffer
@@ -647,7 +642,7 @@ func (p *processor) integrity(
647642
continue
648643
}
649644

650-
u := misc.JoinURL(b, fp).String()
645+
u := fp.String()
651646

652647
// Should this URL be ignored?
653648
if p.cfg.ignoreURL(u) {
@@ -779,7 +774,7 @@ func (p *processor) integrity(
779774
lg(ErrorType, "Bad URL %s: %v", x.url(), err)
780775
continue
781776
}
782-
hashFile := misc.JoinURL(b, hu).String()
777+
hashFile := hu.String()
783778

784779
p.checkTLS(hashFile)
785780
if res, err = client.Get(hashFile); err != nil {
@@ -828,7 +823,7 @@ func (p *processor) integrity(
828823
lg(ErrorType, "Bad URL %s: %v", f.SignURL(), err)
829824
continue
830825
}
831-
sigFile := misc.JoinURL(b, su).String()
826+
sigFile := su.String()
832827
p.checkTLS(sigFile)
833828

834829
p.badSignatures.use()
@@ -948,12 +943,13 @@ func (p *processor) checkIndex(base string, mask whereType) error {
948943
scanner := bufio.NewScanner(res.Body)
949944
for line := 1; scanner.Scan(); line++ {
950945
u := scanner.Text()
951-
if _, err := url.Parse(u); err != nil {
946+
up, err := url.Parse(u)
947+
if err != nil {
952948
p.badIntegrities.error("index.txt contains invalid URL %q in line %d", u, line)
953949
continue
954950
}
955951

956-
files = append(files, csaf.DirectoryAdvisoryFile{Path: u})
952+
files = append(files, csaf.DirectoryAdvisoryFile{Path: misc.JoinURL(bu, up).String()})
957953
}
958954
return files, scanner.Err()
959955
}()
@@ -968,7 +964,7 @@ func (p *processor) checkIndex(base string, mask whereType) error {
968964
// Block rolie checks.
969965
p.labelChecker.feedLabel = ""
970966

971-
return p.integrity(files, base, mask, p.badIndices.add)
967+
return p.integrity(files, mask, p.badIndices.add)
972968
}
973969

974970
// checkChanges fetches the "changes.csv" and calls the "checkTLS" method for HTTPs checks.
@@ -1035,8 +1031,13 @@ func (p *processor) checkChanges(base string, mask whereType) error {
10351031
}
10361032
path := r[pathColumn]
10371033

1034+
pathURL, err := url.Parse(path)
1035+
if err != nil {
1036+
return nil, nil, err
1037+
}
1038+
10381039
times, files = append(times, t),
1039-
append(files, csaf.DirectoryAdvisoryFile{Path: path})
1040+
append(files, csaf.DirectoryAdvisoryFile{Path: misc.JoinURL(bu, pathURL).String()})
10401041
p.timesChanges[path] = t
10411042
}
10421043
return times, files, nil
@@ -1063,7 +1064,7 @@ func (p *processor) checkChanges(base string, mask whereType) error {
10631064
// Block rolie checks.
10641065
p.labelChecker.feedLabel = ""
10651066

1066-
return p.integrity(files, base, mask, p.badChanges.add)
1067+
return p.integrity(files, mask, p.badChanges.add)
10671068
}
10681069

10691070
// empty checks if list of strings contains at least one none empty string.
@@ -1364,18 +1365,11 @@ func (p *processor) checkSecurityFolder(folder string) string {
13641365
}
13651366

13661367
// Try to load
1367-
up, err := url.Parse(u)
1368+
_, err = url.Parse(u)
13681369
if err != nil {
13691370
return fmt.Sprintf("CSAF URL '%s' invalid: %v", u, err)
13701371
}
13711372

1372-
base, err := url.Parse(folder)
1373-
if err != nil {
1374-
return err.Error()
1375-
}
1376-
base.Path = ""
1377-
1378-
u = misc.JoinURL(base, up).String()
13791373
p.checkTLS(u)
13801374
if res, err = client.Get(u); err != nil {
13811375
return fmt.Sprintf("Cannot fetch %s from security.txt: %v", u, err)
@@ -1523,12 +1517,6 @@ func (p *processor) checkPGPKeys(_ string) error {
15231517

15241518
client := p.httpClient()
15251519

1526-
base, err := url.Parse(p.pmdURL)
1527-
if err != nil {
1528-
return err
1529-
}
1530-
base.Path = ""
1531-
15321520
for i := range keys {
15331521
key := &keys[i]
15341522
if key.URL == nil {
@@ -1541,10 +1529,11 @@ func (p *processor) checkPGPKeys(_ string) error {
15411529
continue
15421530
}
15431531

1544-
u := misc.JoinURL(base, up).String()
1532+
// Todo: refactor all methods to directly accept *url.URL
1533+
u := up.String()
15451534
p.checkTLS(u)
15461535

1547-
res, err := client.Get(u)
1536+
res, err := client.Get(*key.URL)
15481537
if err != nil {
15491538
p.badPGPs.error("Fetching public OpenPGP key %s failed: %v.", u, err)
15501539
continue

‎cmd/csaf_checker/roliecheck.go‎

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ package main
1010

1111
import (
1212
"errors"
13-
"github.com/gocsaf/csaf/v3/internal/misc"
1413
"net/http"
1514
"net/url"
1615
"sort"
@@ -217,12 +216,6 @@ func defaults[T any](p *T, def T) T {
217216
// processROLIEFeeds goes through all ROLIE feeds and checks their
218217
// integrity and completeness.
219218
func (p *processor) processROLIEFeeds(feeds [][]csaf.Feed) error {
220-
221-
base, err := url.Parse(p.pmdURL)
222-
if err != nil {
223-
return err
224-
}
225-
base.Path = ""
226219
p.badROLIEFeed.use()
227220

228221
advisories := map[*csaf.Feed][]csaf.AdvisoryFile{}
@@ -234,12 +227,11 @@ func (p *processor) processROLIEFeeds(feeds [][]csaf.Feed) error {
234227
if feed.URL == nil {
235228
continue
236229
}
237-
up, err := url.Parse(string(*feed.URL))
230+
feedBase, err := url.Parse(string(*feed.URL))
238231
if err != nil {
239232
p.badProviderMetadata.error("Invalid URL %s in feed: %v.", *feed.URL, err)
240233
continue
241234
}
242-
feedBase := misc.JoinURL(base, up)
243235
feedURL := feedBase.String()
244236
p.checkTLS(feedURL)
245237

@@ -266,13 +258,12 @@ func (p *processor) processROLIEFeeds(feeds [][]csaf.Feed) error {
266258
continue
267259
}
268260

269-
up, err := url.Parse(string(*feed.URL))
261+
feedURL, err := url.Parse(string(*feed.URL))
270262
if err != nil {
271263
p.badProviderMetadata.error("Invalid URL %s in feed: %v.", *feed.URL, err)
272264
continue
273265
}
274266

275-
feedURL := misc.JoinURL(base, up)
276267
feedBase, err := util.BaseURL(feedURL)
277268
if err != nil {
278269
p.badProviderMetadata.error("Bad base path: %v", err)
@@ -292,7 +283,7 @@ func (p *processor) processROLIEFeeds(feeds [][]csaf.Feed) error {
292283
// TODO: Issue a warning if we want check AMBER+ without an
293284
// authorizing client.
294285

295-
if err := p.integrity(files, base.String(), rolieMask, p.badProviderMetadata.add); err != nil {
286+
if err := p.integrity(files, rolieMask, p.badProviderMetadata.add); err != nil {
296287
if err != errContinue {
297288
return err
298289
}
@@ -321,13 +312,12 @@ func (p *processor) processROLIEFeeds(feeds [][]csaf.Feed) error {
321312
continue
322313
}
323314

324-
up, err := url.Parse(string(*feed.URL))
315+
feedBase, err := url.Parse(string(*feed.URL))
325316
if err != nil {
326317
p.badProviderMetadata.error("Invalid URL %s in feed: %v.", *feed.URL, err)
327318
continue
328319
}
329320

330-
feedBase := misc.JoinURL(base, up)
331321
makeAbs := makeAbsolute(feedBase)
332322
label := defaults(feed.TLPLabel, csaf.TLPLabelUnlabeled)
333323

‎cmd/csaf_downloader/downloader.go‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -226,18 +226,16 @@ func (d *downloader) download(ctx context.Context, domain string) error {
226226
}
227227
}
228228

229-
base, err := url.Parse(lpmd.URL)
229+
pmdURL, err := url.Parse(lpmd.URL)
230230
if err != nil {
231231
return fmt.Errorf("invalid URL '%s': %v", lpmd.URL, err)
232232
}
233-
base.Path = ""
234233

235234
expr := util.NewPathEval()
236235

237236
if err := d.loadOpenPGPKeys(
238237
client,
239238
lpmd.Document,
240-
base,
241239
expr,
242240
); err != nil {
243241
return err
@@ -247,7 +245,7 @@ func (d *downloader) download(ctx context.Context, domain string) error {
247245
client,
248246
expr,
249247
lpmd.Document,
250-
base)
248+
pmdURL)
251249

252250
// Do we need time range based filtering?
253251
if d.cfg.Range != nil {
@@ -312,7 +310,6 @@ allFiles:
312310
func (d *downloader) loadOpenPGPKeys(
313311
client util.Client,
314312
doc any,
315-
base *url.URL,
316313
expr *util.PathEval,
317314
) error {
318315
src, err := expr.Eval("$.public_openpgp_keys", doc)
@@ -337,17 +334,15 @@ func (d *downloader) loadOpenPGPKeys(
337334
if key.URL == nil {
338335
continue
339336
}
340-
up, err := url.Parse(*key.URL)
337+
u, err := url.Parse(*key.URL)
341338
if err != nil {
342339
slog.Warn("Invalid URL",
343340
"url", *key.URL,
344341
"error", err)
345342
continue
346343
}
347344

348-
u := base.JoinPath(up.Path).String()
349-
350-
res, err := client.Get(u)
345+
res, err := client.Get(u.String())
351346
if err != nil {
352347
slog.Warn(
353348
"Fetching public OpenPGP key failed",

‎csaf/advisories.go‎

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,14 @@ import (
1212
"context"
1313
"encoding/csv"
1414
"fmt"
15-
"github.com/gocsaf/csaf/v3/internal/misc"
1615
"io"
1716
"log/slog"
1817
"net/http"
1918
"net/url"
2019
"strings"
2120
"time"
2221

22+
"github.com/gocsaf/csaf/v3/internal/misc"
2323
"github.com/gocsaf/csaf/v3/util"
2424
)
2525

@@ -96,7 +96,7 @@ type AdvisoryFileProcessor struct {
9696
client util.Client
9797
expr *util.PathEval
9898
doc any
99-
base *url.URL
99+
pmdURL *url.URL
100100
}
101101

102102
// NewAdvisoryFileProcessor constructs a filename extractor
@@ -105,13 +105,13 @@ func NewAdvisoryFileProcessor(
105105
client util.Client,
106106
expr *util.PathEval,
107107
doc any,
108-
base *url.URL,
108+
pmdURL *url.URL,
109109
) *AdvisoryFileProcessor {
110110
return &AdvisoryFileProcessor{
111111
client: client,
112112
expr: expr,
113113
doc: doc,
114-
base: base,
114+
pmdURL: pmdURL,
115115
}
116116
}
117117

@@ -180,7 +180,7 @@ func (afp *AdvisoryFileProcessor) Process(
180180

181181
// Not found -> fall back to PMD url
182182
if empty(dirURLs) {
183-
baseURL, err := util.BaseURL(afp.base)
183+
baseURL, err := util.BaseURL(afp.pmdURL)
184184
if err != nil {
185185
return err
186186
}
@@ -262,8 +262,13 @@ func (afp *AdvisoryFileProcessor) loadChanges(
262262
continue
263263
}
264264

265+
pathURL, err := url.Parse(path)
266+
if err != nil {
267+
return nil, err
268+
}
269+
265270
files = append(files,
266-
DirectoryAdvisoryFile{Path: base.JoinPath(path).String()})
271+
DirectoryAdvisoryFile{Path: misc.JoinURL(base, pathURL).String()})
267272
}
268273
return files, nil
269274
}
@@ -277,25 +282,18 @@ func (afp *AdvisoryFileProcessor) processROLIE(
277282
if feed.URL == nil {
278283
continue
279284
}
280-
up, err := url.Parse(string(*feed.URL))
285+
feedURL, err := url.Parse(string(*feed.URL))
281286
if err != nil {
282287
slog.Error("Invalid URL in feed", "feed", *feed.URL, "err", err)
283288
continue
284289
}
285-
feedURL := misc.JoinURL(afp.base, up)
286290
slog.Info("Got feed URL", "feed", feedURL)
287291

288292
fb, err := util.BaseURL(feedURL)
289293
if err != nil {
290294
slog.Error("Invalid feed base URL", "url", fb, "err", err)
291295
continue
292296
}
293-
feedBaseURL, err := url.Parse(fb)
294-
if err != nil {
295-
slog.Error("Cannot parse feed base URL", "url", fb, "err", err)
296-
continue
297-
}
298-
feedBaseURL.Path = ""
299297

300298
res, err := afp.client.Get(feedURL.String())
301299
if err != nil {
@@ -327,7 +325,7 @@ func (afp *AdvisoryFileProcessor) processROLIE(
327325
slog.Error("Invalid URL", "url", u, "err", err)
328326
return ""
329327
}
330-
return misc.JoinURL(feedBaseURL, p).String()
328+
return p.String()
331329
}
332330

333331
rfeed.Entries(func(entry *Entry) {

0 commit comments

Comments
 (0)