From fe77d496a20d51038dbcc8a84b4008b995f00fcf Mon Sep 17 00:00:00 2001 From: koplas Date: Tue, 26 Aug 2025 18:47:53 +0200 Subject: [PATCH] Remove unnecessary URL joins This should avoid bugs for more complex scenarios. --- cmd/csaf_aggregator/mirror.go | 5 ++-- cmd/csaf_checker/processor.go | 47 ++++++++++++------------------- cmd/csaf_checker/roliecheck.go | 18 +++--------- cmd/csaf_downloader/downloader.go | 13 +++------ csaf/advisories.go | 28 +++++++++--------- 5 files changed, 41 insertions(+), 70 deletions(-) diff --git a/cmd/csaf_aggregator/mirror.go b/cmd/csaf_aggregator/mirror.go index a013553..9653ea9 100644 --- a/cmd/csaf_aggregator/mirror.go +++ b/cmd/csaf_aggregator/mirror.go @@ -67,17 +67,16 @@ func (w *worker) mirrorInternal() (*csaf.AggregatorCSAFProvider, error) { // Collecting the categories per label. w.categories = map[string]util.Set[string]{} - base, err := url.Parse(w.loc) + pmdURL, err := url.Parse(w.loc) if err != nil { return nil, err } - base.Path = "" afp := csaf.NewAdvisoryFileProcessor( w.client, w.expr, w.metadataProvider, - base) + pmdURL) afp.AgeAccept = w.provider.ageAccept(w.processor.cfg) diff --git a/cmd/csaf_checker/processor.go b/cmd/csaf_checker/processor.go index 18ef49e..6e780ca 100644 --- a/cmd/csaf_checker/processor.go +++ b/cmd/csaf_checker/processor.go @@ -628,14 +628,9 @@ var yearFromURL = regexp.MustCompile(`.*/(\d{4})/[^/]+$`) // mistakes, from conforming filenames to invalid advisories. func (p *processor) integrity( files []csaf.AdvisoryFile, - base string, mask whereType, lg func(MessageType, string, ...any), ) error { - b, err := url.Parse(base) - if err != nil { - return err - } client := p.httpClient() var data bytes.Buffer @@ -647,7 +642,7 @@ func (p *processor) integrity( continue } - u := misc.JoinURL(b, fp).String() + u := fp.String() // Should this URL be ignored? if p.cfg.ignoreURL(u) { @@ -779,7 +774,7 @@ func (p *processor) integrity( lg(ErrorType, "Bad URL %s: %v", x.url(), err) continue } - hashFile := misc.JoinURL(b, hu).String() + hashFile := hu.String() p.checkTLS(hashFile) if res, err = client.Get(hashFile); err != nil { @@ -828,7 +823,7 @@ func (p *processor) integrity( lg(ErrorType, "Bad URL %s: %v", f.SignURL(), err) continue } - sigFile := misc.JoinURL(b, su).String() + sigFile := su.String() p.checkTLS(sigFile) p.badSignatures.use() @@ -948,12 +943,13 @@ func (p *processor) checkIndex(base string, mask whereType) error { scanner := bufio.NewScanner(res.Body) for line := 1; scanner.Scan(); line++ { u := scanner.Text() - if _, err := url.Parse(u); err != nil { + up, err := url.Parse(u) + if err != nil { p.badIntegrities.error("index.txt contains invalid URL %q in line %d", u, line) continue } - files = append(files, csaf.DirectoryAdvisoryFile{Path: u}) + files = append(files, csaf.DirectoryAdvisoryFile{Path: misc.JoinURL(bu, up).String()}) } return files, scanner.Err() }() @@ -968,7 +964,7 @@ func (p *processor) checkIndex(base string, mask whereType) error { // Block rolie checks. p.labelChecker.feedLabel = "" - return p.integrity(files, base, mask, p.badIndices.add) + return p.integrity(files, mask, p.badIndices.add) } // 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 { } path := r[pathColumn] + pathURL, err := url.Parse(path) + if err != nil { + return nil, nil, err + } + times, files = append(times, t), - append(files, csaf.DirectoryAdvisoryFile{Path: path}) + append(files, csaf.DirectoryAdvisoryFile{Path: misc.JoinURL(bu, pathURL).String()}) p.timesChanges[path] = t } return times, files, nil @@ -1063,7 +1064,7 @@ func (p *processor) checkChanges(base string, mask whereType) error { // Block rolie checks. p.labelChecker.feedLabel = "" - return p.integrity(files, base, mask, p.badChanges.add) + return p.integrity(files, mask, p.badChanges.add) } // empty checks if list of strings contains at least one none empty string. @@ -1364,18 +1365,11 @@ func (p *processor) checkSecurityFolder(folder string) string { } // Try to load - up, err := url.Parse(u) + _, err = url.Parse(u) if err != nil { return fmt.Sprintf("CSAF URL '%s' invalid: %v", u, err) } - base, err := url.Parse(folder) - if err != nil { - return err.Error() - } - base.Path = "" - - u = misc.JoinURL(base, up).String() p.checkTLS(u) if res, err = client.Get(u); err != nil { return fmt.Sprintf("Cannot fetch %s from security.txt: %v", u, err) @@ -1523,12 +1517,6 @@ func (p *processor) checkPGPKeys(_ string) error { client := p.httpClient() - base, err := url.Parse(p.pmdURL) - if err != nil { - return err - } - base.Path = "" - for i := range keys { key := &keys[i] if key.URL == nil { @@ -1541,10 +1529,11 @@ func (p *processor) checkPGPKeys(_ string) error { continue } - u := misc.JoinURL(base, up).String() + // Todo: refactor all methods to directly accept *url.URL + u := up.String() p.checkTLS(u) - res, err := client.Get(u) + res, err := client.Get(*key.URL) if err != nil { p.badPGPs.error("Fetching public OpenPGP key %s failed: %v.", u, err) continue diff --git a/cmd/csaf_checker/roliecheck.go b/cmd/csaf_checker/roliecheck.go index ace4d0d..f510992 100644 --- a/cmd/csaf_checker/roliecheck.go +++ b/cmd/csaf_checker/roliecheck.go @@ -10,7 +10,6 @@ package main import ( "errors" - "github.com/gocsaf/csaf/v3/internal/misc" "net/http" "net/url" "sort" @@ -217,12 +216,6 @@ func defaults[T any](p *T, def T) T { // processROLIEFeeds goes through all ROLIE feeds and checks their // integrity and completeness. func (p *processor) processROLIEFeeds(feeds [][]csaf.Feed) error { - - base, err := url.Parse(p.pmdURL) - if err != nil { - return err - } - base.Path = "" p.badROLIEFeed.use() advisories := map[*csaf.Feed][]csaf.AdvisoryFile{} @@ -234,12 +227,11 @@ func (p *processor) processROLIEFeeds(feeds [][]csaf.Feed) error { if feed.URL == nil { continue } - up, err := url.Parse(string(*feed.URL)) + feedBase, err := url.Parse(string(*feed.URL)) if err != nil { p.badProviderMetadata.error("Invalid URL %s in feed: %v.", *feed.URL, err) continue } - feedBase := misc.JoinURL(base, up) feedURL := feedBase.String() p.checkTLS(feedURL) @@ -266,13 +258,12 @@ func (p *processor) processROLIEFeeds(feeds [][]csaf.Feed) error { continue } - up, err := url.Parse(string(*feed.URL)) + feedURL, err := url.Parse(string(*feed.URL)) if err != nil { p.badProviderMetadata.error("Invalid URL %s in feed: %v.", *feed.URL, err) continue } - feedURL := misc.JoinURL(base, up) feedBase, err := util.BaseURL(feedURL) if err != nil { p.badProviderMetadata.error("Bad base path: %v", err) @@ -292,7 +283,7 @@ func (p *processor) processROLIEFeeds(feeds [][]csaf.Feed) error { // TODO: Issue a warning if we want check AMBER+ without an // authorizing client. - if err := p.integrity(files, base.String(), rolieMask, p.badProviderMetadata.add); err != nil { + if err := p.integrity(files, rolieMask, p.badProviderMetadata.add); err != nil { if err != errContinue { return err } @@ -321,13 +312,12 @@ func (p *processor) processROLIEFeeds(feeds [][]csaf.Feed) error { continue } - up, err := url.Parse(string(*feed.URL)) + feedBase, err := url.Parse(string(*feed.URL)) if err != nil { p.badProviderMetadata.error("Invalid URL %s in feed: %v.", *feed.URL, err) continue } - feedBase := misc.JoinURL(base, up) makeAbs := makeAbsolute(feedBase) label := defaults(feed.TLPLabel, csaf.TLPLabelUnlabeled) diff --git a/cmd/csaf_downloader/downloader.go b/cmd/csaf_downloader/downloader.go index 4890593..4edd724 100644 --- a/cmd/csaf_downloader/downloader.go +++ b/cmd/csaf_downloader/downloader.go @@ -226,18 +226,16 @@ func (d *downloader) download(ctx context.Context, domain string) error { } } - base, err := url.Parse(lpmd.URL) + pmdURL, err := url.Parse(lpmd.URL) if err != nil { return fmt.Errorf("invalid URL '%s': %v", lpmd.URL, err) } - base.Path = "" expr := util.NewPathEval() if err := d.loadOpenPGPKeys( client, lpmd.Document, - base, expr, ); err != nil { return err @@ -247,7 +245,7 @@ func (d *downloader) download(ctx context.Context, domain string) error { client, expr, lpmd.Document, - base) + pmdURL) // Do we need time range based filtering? if d.cfg.Range != nil { @@ -312,7 +310,6 @@ allFiles: func (d *downloader) loadOpenPGPKeys( client util.Client, doc any, - base *url.URL, expr *util.PathEval, ) error { src, err := expr.Eval("$.public_openpgp_keys", doc) @@ -337,7 +334,7 @@ func (d *downloader) loadOpenPGPKeys( if key.URL == nil { continue } - up, err := url.Parse(*key.URL) + u, err := url.Parse(*key.URL) if err != nil { slog.Warn("Invalid URL", "url", *key.URL, @@ -345,9 +342,7 @@ func (d *downloader) loadOpenPGPKeys( continue } - u := base.JoinPath(up.Path).String() - - res, err := client.Get(u) + res, err := client.Get(u.String()) if err != nil { slog.Warn( "Fetching public OpenPGP key failed", diff --git a/csaf/advisories.go b/csaf/advisories.go index c5e4fea..33dfa03 100644 --- a/csaf/advisories.go +++ b/csaf/advisories.go @@ -12,7 +12,6 @@ import ( "context" "encoding/csv" "fmt" - "github.com/gocsaf/csaf/v3/internal/misc" "io" "log/slog" "net/http" @@ -20,6 +19,7 @@ import ( "strings" "time" + "github.com/gocsaf/csaf/v3/internal/misc" "github.com/gocsaf/csaf/v3/util" ) @@ -96,7 +96,7 @@ type AdvisoryFileProcessor struct { client util.Client expr *util.PathEval doc any - base *url.URL + pmdURL *url.URL } // NewAdvisoryFileProcessor constructs a filename extractor @@ -105,13 +105,13 @@ func NewAdvisoryFileProcessor( client util.Client, expr *util.PathEval, doc any, - base *url.URL, + pmdURL *url.URL, ) *AdvisoryFileProcessor { return &AdvisoryFileProcessor{ client: client, expr: expr, doc: doc, - base: base, + pmdURL: pmdURL, } } @@ -180,7 +180,7 @@ func (afp *AdvisoryFileProcessor) Process( // Not found -> fall back to PMD url if empty(dirURLs) { - baseURL, err := util.BaseURL(afp.base) + baseURL, err := util.BaseURL(afp.pmdURL) if err != nil { return err } @@ -262,8 +262,13 @@ func (afp *AdvisoryFileProcessor) loadChanges( continue } + pathURL, err := url.Parse(path) + if err != nil { + return nil, err + } + files = append(files, - DirectoryAdvisoryFile{Path: base.JoinPath(path).String()}) + DirectoryAdvisoryFile{Path: misc.JoinURL(base, pathURL).String()}) } return files, nil } @@ -277,12 +282,11 @@ func (afp *AdvisoryFileProcessor) processROLIE( if feed.URL == nil { continue } - up, err := url.Parse(string(*feed.URL)) + feedURL, err := url.Parse(string(*feed.URL)) if err != nil { slog.Error("Invalid URL in feed", "feed", *feed.URL, "err", err) continue } - feedURL := misc.JoinURL(afp.base, up) slog.Info("Got feed URL", "feed", feedURL) fb, err := util.BaseURL(feedURL) @@ -290,12 +294,6 @@ func (afp *AdvisoryFileProcessor) processROLIE( slog.Error("Invalid feed base URL", "url", fb, "err", err) continue } - feedBaseURL, err := url.Parse(fb) - if err != nil { - slog.Error("Cannot parse feed base URL", "url", fb, "err", err) - continue - } - feedBaseURL.Path = "" res, err := afp.client.Get(feedURL.String()) if err != nil { @@ -327,7 +325,7 @@ func (afp *AdvisoryFileProcessor) processROLIE( slog.Error("Invalid URL", "url", u, "err", err) return "" } - return misc.JoinURL(feedBaseURL, p).String() + return p.String() } rfeed.Entries(func(entry *Entry) {