Merge pull request #100 from packit/url-fragments

Add support for filenames in source URL fragments

Fixes this Sentry issue. Even though the issue is actually caused by a typo in Source0 URL, the typo goes unnoticed by standard tooling because the correct filename is specified in URL fragment.

RELEASE NOTES BEGIN
Added support for filenames specified in source URL fragments, for example: https://example.com/foo/1.0/download.cgi#/%{name}-%{version}.tar.gz
RELEASE NOTES END

Reviewed-by: None <None>

specfile/rpm.py

@@ -8,14 +8,14 @@
 import re
 import sys
 import tempfile
-import urllib.parse
 from enum import IntEnum
 from pathlib import Path
 from typing import Iterator, List, Optional, Tuple
 
 import rpm
 
 from specfile.exceptions import MacroRemovalException, RPMException
+from specfile.utils import get_filename_from_location
 
 MAX_REMOVAL_RETRIES = 20
 
@@ -267,7 +267,7 @@ def make_dummy_sources(sources: List[str], sourcedir: Path) -> Iterator[List[Pat
         MAGIC_LENGTH = 13
         dummy_sources = []
         for source in sources:
-            filename = Path(urllib.parse.urlsplit(source).path).name
+            filename = get_filename_from_location(source)
             if not filename:
                 continue
             path = sourcedir / filename

specfile/sources.py

@@ -5,13 +5,13 @@
 import re
 import urllib.parse
 from abc import ABC, abstractmethod
-from pathlib import Path
 from typing import Iterable, List, Optional, Tuple, Union, cast, overload
 
 from specfile.exceptions import DuplicateSourceException
 from specfile.rpm import Macros
 from specfile.sourcelist import Sourcelist, SourcelistEntry
 from specfile.tags import Comments, Tag, Tags
+from specfile.utils import get_filename_from_location
 
 
 class Source(ABC):
@@ -137,12 +137,12 @@ def expanded_location(self) -> str:
     @property
     def filename(self) -> str:
         """Literal filename of the source."""
-        return Path(urllib.parse.urlsplit(self._tag.value).path).name
+        return get_filename_from_location(self._tag.value)
 
     @property
     def expanded_filename(self) -> str:
         """Filename of the source after expanding macros."""
-        return Path(urllib.parse.urlsplit(self._tag.expanded_value).path).name
+        return get_filename_from_location(self._tag.expanded_value)
 
     @property
     def comments(self) -> Comments:
@@ -193,12 +193,12 @@ def expanded_location(self) -> str:
     @property
     def filename(self) -> str:
         """Literal filename of the source."""
-        return Path(urllib.parse.urlsplit(self._source.location).path).name
+        return get_filename_from_location(self._source.location)
 
     @property
     def expanded_filename(self) -> str:
         """Filename of the source after expanding macros."""
-        return Path(urllib.parse.urlsplit(self._source.expanded_location).path).name
+        return get_filename_from_location(self._source.expanded_location)
 
     @property
     def comments(self) -> Comments:

specfile/utils.py

@@ -0,0 +1,25 @@
+# Copyright Contributors to the Packit project.
+# SPDX-License-Identifier: MIT
+
+import urllib.parse
+from pathlib import Path
+
+
+def get_filename_from_location(location: str) -> str:
+    """
+    Extracts filename from given source location.
+
+    Follows RPM logic - target filename can be specified in URL fragment.
+
+    Args:
+        location: Location to extract filename from.
+
+    Returns:
+        Extracted filename that can be empty if there is none.
+    """
+    url = urllib.parse.urlsplit(location)
+    if url.fragment:
+        if "/" in url.fragment:
+            return Path(url.fragment).name.split("=")[-1]
+        return Path(f"{url.path}#{url.fragment}").name
+    return Path(url.path).name

tests/unit/test_utils.py

@@ -0,0 +1,31 @@
+# Copyright Contributors to the Packit project.
+# SPDX-License-Identifier: MIT
+
+import pytest
+
+from specfile.utils import get_filename_from_location
+
+
+@pytest.mark.parametrize(
+    "location, filename",
+    [
+        ("", ""),
+        ("tarball-0.1.tar.gz", "tarball-0.1.tar.gz"),
+        ("https://example.com", ""),
+        ("https://example.com/archive/tarball-0.1.tar.gz", "tarball-0.1.tar.gz"),
+        (
+            "https://example.com/archive/tarball-0.1.tar.gz#fragment",
+            "tarball-0.1.tar.gz#fragment",
+        ),
+        (
+            "https://example.com/download_tarball.cgi#/tarball-0.1.tar.gz",
+            "tarball-0.1.tar.gz",
+        ),
+        (
+            "https://example.com/tarball-latest.tar.gz#/file=tarball-0.1.tar.gz",
+            "tarball-0.1.tar.gz",
+        ),
+    ],
+)
+def test_get_filename_from_location(location, filename):
+    assert get_filename_from_location(location) == filename