Avoid storing Flickr secret-bearing URLs (#46)

Author: glenn-jocher

README.md

@@ -76,9 +76,9 @@ python3 flickr_scraper.py --search 'honeybees on flowers' --n 10 --download
 You should see output indicating the download progress:
 
 ```plaintext
-1/10 https://live.staticflickr.com/21/38596887_40df118fd9_o.jpg
+1/10 downloaded
 ...
-10/10 https://live.staticflickr.com/1770/43276172331_e779b8c161_o.jpg
+10/10 downloaded
 Done. (4.1s)
 All images saved to /Users/glennjocher/PycharmProjects/flickr_scraper/images/honeybees_on_flowers/
 ```
@@ -101,6 +101,8 @@ The script de-duplicates URLs within each run, but it does not keep a persistent
 
 This scraper uses Flickr's JSON `photos.search` API response directly. It does not call `flickrapi.walk()`, avoiding the older `getchildren()` compatibility error reported with `flickrapi==2.4.0` on Python 3.9 and newer.
 
+Photos missing Flickr's direct `url_o` field are skipped instead of constructing fallback URLs from Flickr metadata fields. This avoids propagating secret-bearing fields into download paths or logs.
+
 If Flickr returns a `500` or another API error, the script reports the Flickr error code and message. These errors usually come from Flickr or invalid credentials; retry the command after checking your API key and secret.
 
 ## 📜 Citation

flickr_scraper.py

@@ -16,12 +16,8 @@
 
 
 def build_photo_url(photo):
-    """Return the best available static image URL from a Flickr photo response."""
-    url = photo.get("url_o")  # original size
-    return url or (
-        f"https://farm{photo.get('farm')}.staticflickr.com/"
-        f"{photo.get('server')}/{photo.get('id')}_{photo.get('secret')}_b.jpg"
-    )
+    """Return the Flickr-provided original image URL when available."""
+    return photo.get("url_o")
 
 
 def resolve_credentials(api_key=None, api_secret=None):
@@ -79,14 +75,18 @@ def get_urls(search="honeybees on flowers", n=10, download=False, api_key=None,
 
         for photo in photo_list:
             url = build_photo_url(photo)
+            if url is None:
+                print("skipped (missing original URL)")
+                continue
+
             if url in urls:
                 continue
 
             if download:
                 download_uri(url, dir_path)
 
             urls.append(url)
-            print(f"{len(urls)}/{n} {url}")
+            print(f"{len(urls)}/{n} {'downloaded' if download else 'found'}")
 
             if len(urls) >= n:
                 break

tests/test_flickr_scraper.py

@@ -5,7 +5,7 @@
 import pytest
 
 import flickr_scraper
-from utils.general import download_uri
+from utils.general import download_uri, safe_filename_from_uri
 
 
 def test_build_photo_url_prefers_original_url():
@@ -21,11 +21,11 @@ def test_build_photo_url_prefers_original_url():
     assert flickr_scraper.build_photo_url(photo) == "https://live.staticflickr.com/photo_o.jpg"
 
 
-def test_build_photo_url_falls_back_to_large_static_url():
-    """Ensure a static Flickr URL is built when the original URL is unavailable."""
+def test_build_photo_url_skips_photos_without_original_url():
+    """Ensure secret-bearing fallback URLs are not built."""
     photo = {"farm": "1", "server": "2", "id": "3", "secret": "4"}
 
-    assert flickr_scraper.build_photo_url(photo) == "https://farm1.staticflickr.com/2/3_4_b.jpg"
+    assert flickr_scraper.build_photo_url(photo) is None
 
 
 def test_resolve_credentials_requires_key_and_secret(monkeypatch):
@@ -39,8 +39,8 @@ def test_resolve_credentials_requires_key_and_secret(monkeypatch):
         flickr_scraper.resolve_credentials()
 
 
-def test_get_urls_uses_json_search_and_honors_limit(monkeypatch):
-    """Ensure get_urls uses photos.search and returns exactly the requested number of URLs."""
+def test_get_urls_uses_json_search_and_redacts_output(monkeypatch, capsys):
+    """Ensure get_urls uses photos.search, skips secret fallback URLs, and redacts output."""
     calls = []
 
     class FakePhotos:
@@ -69,8 +69,12 @@ def __init__(self, api_key, api_secret, format):
     monkeypatch.setattr(flickr_scraper, "FlickrAPI", FakeFlickr)
 
     urls = flickr_scraper.get_urls("bees", n=2, api_key="key", api_secret="secret", page=3)
+    stdout = capsys.readouterr().out
 
-    assert urls == ["https://example.com/1.jpg", "https://farm1.staticflickr.com/2/3_4_b.jpg"]
+    assert urls == ["https://example.com/1.jpg", "https://example.com/extra.jpg"]
+    assert "https://example.com" not in stdout
+    assert "3_4_b.jpg" not in stdout
+    assert "skipped (missing original URL)" in stdout
     assert calls == [
         {
             "text": "bees",
@@ -132,3 +136,10 @@ def raise_for_status(self):
     download_uri("https://example.com/folder/photo%20name(1).jpg?size=o", tmp_path)
 
     assert (Path(tmp_path) / "photo_name_1_.jpg").read_bytes() == b"image"
+
+
+def test_safe_filename_from_uri_removes_query_parameters():
+    """Ensure query parameters are not persisted in temporary filenames."""
+    uri = "https://example.com/folder/photo name.jpg?secret=token&download=1"
+
+    assert safe_filename_from_uri(uri) == "photo_name.jpg"

utils/general.py

@@ -1,38 +1,35 @@
 # Ultralytics 🚀 AGPL-3.0 License - https://ultralytics.com/license
 
 from pathlib import Path
+from urllib.parse import unquote, urlsplit
 
 import requests
 from PIL import Image
 
 
-def download_uri(uri, dir="./"):
-    """Downloads file from URI, performing checks and renaming; supports timeout and image format suffix addition."""
-    # Download
-    dir = Path(dir)
-    f = dir / Path(uri).name  # filename
-    response = requests.get(uri, timeout=10)
-    response.raise_for_status()
-    with open(f, "wb") as file:
-        file.write(response.content)
-
-    # Rename (remove wildcard characters)
-    src = f  # original name
-    f = Path(
-        str(f)
-        .replace("%20", "_")
+def safe_filename_from_uri(uri):
+    """Return a sanitized filename from a URI path without query parameters."""
+    filename = unquote(Path(urlsplit(uri).path).name) or "download"
+    return (
+        filename.replace("%20", "_")
         .replace("%", "_")
         .replace("*", "_")
         .replace("~", "_")
         .replace("(", "_")
         .replace(")", "_")
+        .replace(" ", "_")
     )
 
-    if "?" in str(f):
-        f = Path(str(f)[: str(f).index("?")])
 
-    if src != f:
-        src.rename(f)  # rename
+def download_uri(uri, dir="./"):
+    """Downloads file from URI, performing checks and renaming; supports timeout and image format suffix addition."""
+    # Download
+    dir = Path(dir)
+    f = dir / safe_filename_from_uri(uri)  # filename
+    response = requests.get(uri, timeout=10)
+    response.raise_for_status()
+    with open(f, "wb") as file:
+        file.write(response.content)
 
     # Add suffix (if missing)
     if not f.suffix: