diff --git a/CHANGELOG.md b/CHANGELOG.md index 0954f96..6796476 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,10 @@ cookie file keeps working. URL, so page and scripts always fit together. - **Fix: a partial pack after a login error reported the cookies as fine.** The cookie state now comes from the download, not from the pack. +- **Fix: Facebook posts with pictures failed.** A shared post resolves to `story.php`, which + gallery-dl took for a profile and failed on, after yt-dlp found no video in it. gallery-dl now gets + such a post (and a `permalink.php` link) as `/posts/` and downloads the post's own + pictures; yt-dlp keeps the original link for videos. - **Hardening:** a host that only ends with `instagram.com` or `facebook.com`, such as `evilinstagram.com`, no longer counts as Instagram or Facebook. The host allowlist already refused such links. diff --git a/app/fetch.py b/app/fetch.py index 6430bf9..57c2076 100644 --- a/app/fetch.py +++ b/app/fetch.py @@ -12,12 +12,14 @@ import json import logging +import re import shutil import subprocess import sys from dataclasses import dataclass, field from datetime import datetime, timezone from pathlib import Path +from urllib.parse import parse_qsl, urlsplit from .config import Settings from .cookies import for_download @@ -171,6 +173,28 @@ def _first(data: dict, *keys: str) -> str: # --------------------------------------------------------------------- gallery-dl +FB_POST_PATHS = {"/story.php", "/permalink.php"} +FB_STORY_ID = re.compile(r"[0-9]+|pfbid[0-9A-Za-z]+") +FB_OWNER_ID = re.compile(r"[0-9]+") + + +def gallery_dl_url(target: Target) -> str: + """The link gallery-dl gets. A Facebook post resolves to story.php?story_fbid=...&id=..., + which gallery-dl reads as a profile named "story.php" and fails on; a permalink.php link it + takes for the page's whole photo stream. As /posts/ it opens the post and + gets the post's own pictures. yt-dlp keeps the original link, which works for videos.""" + if target.platform != "facebook": + return target.url + parts = urlsplit(target.url) + if parts.path not in FB_POST_PATHS: + return target.url + query = dict(parse_qsl(parts.query)) + story, owner = query.get("story_fbid", ""), query.get("id", "") + if FB_STORY_ID.fullmatch(story) and FB_OWNER_ID.fullmatch(owner): + return f"https://www.facebook.com/{owner}/posts/{story}" + return target.url + + def run_gallery_dl(target: Target, raw: Path, cookies: Path | None, settings: Settings) -> FetchResult: # The tool runs inside raw, so every path it gets must be absolute. raw = raw.resolve() @@ -186,7 +210,7 @@ def run_gallery_dl(target: Target, raw: Path, cookies: Path | None, settings: Se cmd += ["--config", str(config)] if cookies: cmd += ["--cookies", str(cookies.resolve())] - cmd.append(target.url) + cmd.append(gallery_dl_url(target)) proc = _run(cmd, cwd=raw) files = _media_files(raw) diff --git a/tests/test_fetch.py b/tests/test_fetch.py index 6f7f61d..66fe7ed 100644 --- a/tests/test_fetch.py +++ b/tests/test_fetch.py @@ -154,6 +154,55 @@ def test_complete_first_tool_needs_no_fallback(tools, tmp_path, with_cookies): assert [c["tool"] for c in tools.calls()] == ["gallery-dl"] +# ------------------------------------------------------------- Facebook photo posts + +# A shared facebook.com/share/p/... link, resolved and normalised (app/urls.py). +FACEBOOK_POST = Target( + "https://www.facebook.com/story.php?story_fbid=1128847296531599&id=100082190168714" + "&post_id=100082190168714_1128847296531599", + "facebook", "post", "1128847296531599", +) + + +def test_a_facebook_photo_post_reaches_gallery_dl_as_a_post_link(tools, tmp_path, settings): + # yt-dlp finds no video in a post with a picture. gallery-dl reads story.php as a profile + # named "story.php" and fails; as /posts/ it gets the post's own pictures. + tools.set("yt-dlp", exit=1, stderr="ERROR: [facebook] 1128847296531599: No video formats found!\n") + tools.set("gallery-dl", files=1) + result = fetch(FACEBOOK_POST, tmp_path / "job", settings) + assert (result.tool, len(result.items)) == ("gallery-dl", 1) + calls = tools.calls() + assert [c["tool"] for c in calls] == ["yt-dlp", "gallery-dl"] + assert calls[0]["args"][-1] == FACEBOOK_POST.url # yt-dlp keeps the link that works for videos + assert calls[1]["args"][-1] == "https://www.facebook.com/100082190168714/posts/1128847296531599" + + +@pytest.mark.parametrize("url,expected", [ + ("https://www.facebook.com/story.php?story_fbid=1128847296531599&id=100082190168714", + "https://www.facebook.com/100082190168714/posts/1128847296531599"), + # permalink.php would give gallery-dl the page's whole photo stream instead of the post. + ("https://www.facebook.com/permalink.php?story_fbid=1705099180979984&id=100044397550724", + "https://www.facebook.com/100044397550724/posts/1705099180979984"), + ("https://www.facebook.com/story.php?story_fbid=pfbid02AbCdEf123&id=100082190168714", + "https://www.facebook.com/100082190168714/posts/pfbid02AbCdEf123"), + ("https://www.facebook.com/story.php?id=100082190168714&story_fbid=1128847296531599&post_id=x", + "https://www.facebook.com/100082190168714/posts/1128847296531599"), + # Anything else stays as it is. + ("https://www.facebook.com/story.php?story_fbid=1128847296531599", None), + ("https://www.facebook.com/story.php?story_fbid=1128847296531599&id=../../evil", None), + ("https://www.facebook.com/story.php?story_fbid=11%2F28&id=100082190168714", None), + ("https://www.facebook.com/reel/954162721072206/", None), + ("https://www.facebook.com/photo/?fbid=1128847296531599&set=a.1", None), +]) +def test_the_link_gallery_dl_gets_for_a_facebook_post(url, expected): + target = Target(url, "facebook", "post", "key") + assert fetch_module.gallery_dl_url(target) == (expected or url) + + +def test_instagram_links_reach_gallery_dl_unchanged(): + assert fetch_module.gallery_dl_url(INSTAGRAM) == INSTAGRAM.url + + # ------------------------------------------------------------------------ paths @pytest.mark.parametrize("target,tool", [(INSTAGRAM, "gallery-dl"), (FACEBOOK, "yt-dlp")])