diff --git a/internal/reader/handler/handler.go b/internal/reader/handler/handler.go index 320643b1..43ccfe47 100644 --- a/internal/reader/handler/handler.go +++ b/internal/reader/handler/handler.go @@ -220,9 +220,9 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool return nil } -func checkFeedIcon(store *storage.Storage, feedID int64, websiteURL, iconURL, userAgent string, fetchViaProxy, allowSelfSignedCertificates bool) { +func checkFeedIcon(store *storage.Storage, feedID int64, websiteURL, feedIconURL, userAgent string, fetchViaProxy, allowSelfSignedCertificates bool) { if !store.HasIcon(feedID) { - icon, err := icon.FindIcon(websiteURL, iconURL, userAgent, fetchViaProxy, allowSelfSignedCertificates) + icon, err := icon.FindIcon(websiteURL, feedIconURL, userAgent, fetchViaProxy, allowSelfSignedCertificates) if err != nil { logger.Debug(`[CheckFeedIcon] %v (feedID=%d websiteURL=%s)`, err, feedID, websiteURL) } else if icon == nil { diff --git a/internal/reader/icon/finder.go b/internal/reader/icon/finder.go index bb412615..afc032e3 100644 --- a/internal/reader/icon/finder.go +++ b/internal/reader/icon/finder.go @@ -21,48 +21,73 @@ import ( ) // FindIcon try to find the website's icon. -func FindIcon(websiteURL, iconURL, userAgent string, fetchViaProxy, allowSelfSignedCertificates bool) (*model.Icon, error) { - if iconURL == "" { - rootURL := urllib.RootURL(websiteURL) - logger.Debug("[FindIcon] Trying to find an icon: rootURL=%q websiteURL=%q userAgent=%q", rootURL, websiteURL, userAgent) - - clt := client.NewClientWithConfig(rootURL, config.Opts) - clt.WithUserAgent(userAgent) - clt.AllowSelfSignedCertificates = allowSelfSignedCertificates - - if fetchViaProxy { - clt.WithProxy() - } - - response, err := clt.Get() - if err != nil { - return nil, fmt.Errorf("icon: unable to download website index page: %v", err) - } - - if response.HasServerFailure() { - return nil, fmt.Errorf("icon: unable to download website index page: status=%d", response.StatusCode) - } - - iconURL, err = parseDocument(rootURL, response.Body) +func FindIcon(websiteURL, feedIconURL, userAgent string, fetchViaProxy, allowSelfSignedCertificates bool) (icon *model.Icon, err error) { + if feedIconURL == "" { + feedIconURL, err = fetchHTMLDocumentAndFindIconURL(websiteURL, userAgent, fetchViaProxy, allowSelfSignedCertificates) if err != nil { return nil, err } } - if strings.HasPrefix(iconURL, "data:") { - return parseImageDataURL(iconURL) + if strings.HasPrefix(feedIconURL, "data:") { + return parseImageDataURL(feedIconURL) } - logger.Debug("[FindIcon] Fetching icon => %s", iconURL) - icon, err := downloadIcon(iconURL, userAgent, fetchViaProxy, allowSelfSignedCertificates) + feedIconURL, err = generateIconURL(websiteURL, feedIconURL) if err != nil { return nil, err } + if icon, err = downloadIcon(feedIconURL, userAgent, fetchViaProxy, allowSelfSignedCertificates); err != nil { + return nil, err + } + return icon, nil } -func parseDocument(websiteURL string, data io.Reader) (string, error) { +func generateIconURL(websiteURL, feedIconURL string) (iconURL string, err error) { + feedIconURL = strings.TrimSpace(feedIconURL) + + if feedIconURL == "" { + iconURL, err = urllib.JoinBaseURLAndPath(urllib.RootURL(websiteURL), "favicon.ico") + if err != nil { + return "", fmt.Errorf(`icon: unable to join base URL and path: %w`, err) + } + } else { + iconURL, err = urllib.AbsoluteURL(websiteURL, feedIconURL) + if err != nil { + return "", fmt.Errorf(`icon: unable to convert icon URL to absolute URL: %w`, err) + } + } + + return iconURL, nil +} + +func fetchHTMLDocumentAndFindIconURL(websiteURL, userAgent string, fetchViaProxy, allowSelfSignedCertificates bool) (string, error) { + rootURL := urllib.RootURL(websiteURL) + logger.Debug("[FindIcon] Find icon from HTML webpage: %s", rootURL) + + clt := client.NewClientWithConfig(rootURL, config.Opts) + clt.WithUserAgent(userAgent) + clt.AllowSelfSignedCertificates = allowSelfSignedCertificates + + if fetchViaProxy { + clt.WithProxy() + } + + response, err := clt.Get() + if err != nil { + return "", fmt.Errorf("icon: unable to download website index page: %v", err) + } + + if response.HasServerFailure() { + return "", fmt.Errorf("icon: unable to download website index page: status=%d", response.StatusCode) + } + + return findIconURLFromHTMLDocument(response.Body) +} + +func findIconURLFromHTMLDocument(body io.Reader) (string, error) { queries := []string{ "link[rel='shortcut icon']", "link[rel='Shortcut Icon']", @@ -70,7 +95,7 @@ func parseDocument(websiteURL string, data io.Reader) (string, error) { "link[rel='icon']", } - doc, err := goquery.NewDocumentFromReader(data) + doc, err := goquery.NewDocumentFromReader(body) if err != nil { return "", fmt.Errorf("icon: unable to read document: %v", err) } @@ -88,12 +113,6 @@ func parseDocument(websiteURL string, data io.Reader) (string, error) { } } - if iconURL == "" { - iconURL = urllib.RootURL(websiteURL) + "favicon.ico" - } else { - iconURL, _ = urllib.AbsoluteURL(websiteURL, iconURL) - } - return iconURL, nil } diff --git a/internal/reader/icon/finder_test.go b/internal/reader/icon/finder_test.go index 93769194..7bc8c3e8 100644 --- a/internal/reader/icon/finder_test.go +++ b/internal/reader/icon/finder_test.go @@ -92,12 +92,59 @@ func TestParseDocumentWithWhitespaceIconURL(t *testing.T) { /static/img/favicon.ico ">` - iconURL, err := parseDocument("http://www.example.org/", strings.NewReader(html)) + iconURL, err := findIconURLFromHTMLDocument(strings.NewReader(html)) if err != nil { t.Fatal(err) } - if iconURL != "http://www.example.org/static/img/favicon.ico" { + if iconURL != "/static/img/favicon.ico" { + t.Errorf(`Invalid icon URL, got %q`, iconURL) + } +} + +func TestGenerateIconURL(t *testing.T) { + iconURL, err := generateIconURL("https://example.org/", "/favicon.png") + if err != nil { + t.Fatal(err) + } + + if iconURL != "https://example.org/favicon.png" { + t.Errorf(`Invalid icon URL, got %q`, iconURL) + } + + iconURL, err = generateIconURL("https://example.org/", "img/favicon.png") + if err != nil { + t.Fatal(err) + } + + if iconURL != "https://example.org/img/favicon.png" { + t.Errorf(`Invalid icon URL, got %q`, iconURL) + } + + iconURL, err = generateIconURL("https://example.org/", "https://example.org/img/favicon.png") + if err != nil { + t.Fatal(err) + } + + if iconURL != "https://example.org/img/favicon.png" { + t.Errorf(`Invalid icon URL, got %q`, iconURL) + } + + iconURL, err = generateIconURL("https://example.org/", "//example.org/img/favicon.png") + if err != nil { + t.Fatal(err) + } + + if iconURL != "https://example.org/img/favicon.png" { + t.Errorf(`Invalid icon URL, got %q`, iconURL) + } + + iconURL, err = generateIconURL("https://example.org/", " ") + if err != nil { + t.Fatal(err) + } + + if iconURL != "https://example.org/favicon.ico" { t.Errorf(`Invalid icon URL, got %q`, iconURL) } } diff --git a/internal/storage/icon.go b/internal/storage/icon.go index dc24ff89..dc04f657 100644 --- a/internal/storage/icon.go +++ b/internal/storage/icon.go @@ -27,7 +27,7 @@ func (s *Storage) IconByID(iconID int64) (*model.Icon, error) { if err == sql.ErrNoRows { return nil, nil } else if err != nil { - return nil, fmt.Errorf("Unable to fetch icon by hash: %v", err) + return nil, fmt.Errorf("store: unable to fetch icon by hash: %v", err) } return &icon, nil