diff --git a/internal/reader/subscription/finder.go b/internal/reader/subscription/finder.go index 8e16be64..0434f71d 100644 --- a/internal/reader/subscription/finder.go +++ b/internal/reader/subscription/finder.go @@ -74,7 +74,7 @@ func (f *subscriptionFinder) FindSubscriptions(websiteURL, rssBridgeURL string, if subscriptions, localizedError := f.findSubscriptionsFromYouTube(websiteURL); localizedError != nil { return nil, localizedError } else if len(subscriptions) > 0 { - slog.Debug("Subscriptions found from YouTube channel page", slog.String("website_url", websiteURL), slog.Any("subscriptions", subscriptions)) + slog.Debug("Subscriptions found from YouTube page", slog.String("website_url", websiteURL), slog.Any("subscriptions", subscriptions)) return subscriptions, nil } @@ -275,23 +275,25 @@ func (f *subscriptionFinder) findSubscriptionsFromRSSBridge(websiteURL, rssBridg } func (f *subscriptionFinder) findSubscriptionsFromYouTube(websiteURL string) (Subscriptions, *locale.LocalizedErrorWrapper) { - u, err := url.Parse(websiteURL) + decodedURL, err := url.Parse(websiteURL) if err != nil { return nil, locale.NewLocalizedErrorWrapper(err, "error.invalid_site_url", err) } - if !strings.HasSuffix(u.Host, "youtube.com") { + if !strings.HasSuffix(decodedURL.Host, "youtube.com") { slog.Debug("This website isn't on the youtube domain.", slog.String("website_url", websiteURL)) return nil, nil } - if _, channelID, found := strings.Cut(u.Path, "channel/"); found { + if _, channelID, found := strings.Cut(decodedURL.Path, "channel/"); found { feedURL := "https://www.youtube.com/feeds/videos.xml?channel_id=" + channelID - return Subscriptions{NewSubscription(u.String(), feedURL, parser.FormatAtom)}, nil + return Subscriptions{NewSubscription(decodedURL.String(), feedURL, parser.FormatAtom)}, nil } - if (strings.HasPrefix(u.Path, "/watch") && u.Query().Has("list")) || strings.HasPrefix(u.Path, "/playlist") { - playlistID := u.Query().Get("list") - feedURL := "https://www.youtube.com/feeds/videos.xml?playlist_id=" + playlistID - return Subscriptions{NewSubscription(u.String(), feedURL, parser.FormatAtom)}, nil + + if strings.HasPrefix(decodedURL.Path, "/watch") || strings.HasPrefix(decodedURL.Path, "/playlist") { + if playlistID := decodedURL.Query().Get("list"); playlistID != "" { + feedURL := "https://www.youtube.com/feeds/videos.xml?playlist_id=" + playlistID + return Subscriptions{NewSubscription(decodedURL.String(), feedURL, parser.FormatAtom)}, nil + } } return nil, nil diff --git a/internal/reader/subscription/finder_test.go b/internal/reader/subscription/finder_test.go index bc83645d..05b61992 100644 --- a/internal/reader/subscription/finder_test.go +++ b/internal/reader/subscription/finder_test.go @@ -8,7 +8,7 @@ import ( "testing" ) -func TestFindYoutubePlaylistFeed(t *testing.T) { +func TestFindYoutubeFeed(t *testing.T) { type testResult struct { websiteURL string feedURL string @@ -56,6 +56,11 @@ func TestFindYoutubePlaylistFeed(t *testing.T) { websiteURL: "https://www.youtube.com/watch?v=6IutBmRJNLk&list=PLOOwEPgFWm_NHcQd9aCi5JXWASHO_n5uR&index=4", feedURL: "https://www.youtube.com/feeds/videos.xml?playlist_id=PLOOwEPgFWm_NHcQd9aCi5JXWASHO_n5uR", }, + // Empty playlist ID parameter + { + websiteURL: "https://www.youtube.com/playlist?list=", + feedURL: "", + }, // Non-Youtube URL { websiteURL: "https://www.example.com/channel/UC-Qj80avWItNRjkZ41rzHyw", @@ -79,11 +84,11 @@ func TestFindYoutubePlaylistFeed(t *testing.T) { if scenario.feedURL == "" { if len(subscriptions) > 0 { - t.Fatalf(`Parsing a non-playlist URL should not return any subscription: %q`, scenario.websiteURL) + t.Fatalf(`Parsing an invalid URL should not return any subscription: %q -> %v`, scenario.websiteURL, subscriptions) } } else { if localizedError != nil { - t.Fatalf(`Parsing a correctly formatted YouTube playlist page should not return any error: %v`, localizedError) + t.Fatalf(`Parsing a correctly formatted YouTube playlist or channel page should not return any error: %v`, localizedError) } if len(subscriptions) != 1 { @@ -91,7 +96,7 @@ func TestFindYoutubePlaylistFeed(t *testing.T) { } if subscriptions[0].URL != scenario.feedURL { - t.Errorf(`Unexpected Feed, got %s, instead of %s`, subscriptions[0].URL, scenario.feedURL) + t.Errorf(`Unexpected feed, got %s, instead of %s`, subscriptions[0].URL, scenario.feedURL) } } }