Repository: yt-dlp/yt-dlp
Pull request #9031 (2024-01-20 01:44:39)

[rh:requests] Apply `remove_dot_segments` to absolute redirect locations

Fixes https://github.com/yt-dlp/yt-dlp/issues/9020#issuecomment-1901575385


See [comment](https://github.com/yt-dlp/yt-dlp/issues/9020#issuecomment-1901575385) for info

<details open><summary>Template</summary> <!-- OPEN is intentional -->

<!--

# PLEASE FOLLOW THE GUIDE BELOW

- You will be asked some questions, please read them **carefully** and answer honestly
- Put an `x` into all the boxes `[ ]` relevant to your *pull request* (like [x])
- Use *Preview* tab to see how your *pull request* will actually look like

-->

### Before submitting a *pull request* make sure you have:
- [X] At least skimmed through [contributing guidelines](https://github.com/yt-dlp/yt-dlp/blob/master/CONTRIBUTING.md#developer-instructions) including [yt-dlp coding conventions](https://github.com/yt-dlp/yt-dlp/blob/master/CONTRIBUTING.md#yt-dlp-coding-conventions)
- [X] [Searched](https://github.com/yt-dlp/yt-dlp/search?q=is%3Apr&type=Issues) the bugtracker for similar pull requests
- [X] Checked the code with [flake8](https://pypi.python.org/pypi/flake8) and [ran relevant tests](https://github.com/yt-dlp/yt-dlp/blob/master/CONTRIBUTING.md#developer-instructions)

### In order to be accepted and merged into yt-dlp each piece of code must be in public domain or released under [Unlicense](http://unlicense.org/). Check all of the following options that apply:
- [X] I am the original author of this code and I am willing to release it under [Unlicense](http://unlicense.org/)
- [ ] I am not the original author of this code but it is in public domain or released under [Unlicense](http://unlicense.org/) (provide reliable evidence)

### What is the purpose of your *pull request*?
- [ ] Fix or improvement to an extractor (Make sure to add/update tests)
- [ ] New extractor ([Piracy websites will not be accepted](https://github.com/yt-dlp/yt-dlp/blob/master/CONTRIBUTING.md#is-the-website-primarily-used-for-piracy))
- [X] Core bug fix/improvement
- [ ] New feature (It is strongly [recommended to open an issue first](https://github.com/yt-dlp/yt-dlp/blob/master/CONTRIBUTING.md#adding-new-feature-or-making-overarching-changes))

</details>



Patch:
diff --git a/yt_dlp/YoutubeDL.py b/yt_dlp/YoutubeDL.py
index 8d96498a6..5dcefb5b8 100644
--- a/yt_dlp/YoutubeDL.py
+++ b/yt_dlp/YoutubeDL.py
@@ -40,7 +40,6 @@
     NoSupportingHandlers,
     RequestError,
     SSLError,
-    _CompatHTTPError,
     network_exceptions,
 )
 from .plugins import directories as plugin_directories
@@ -4110,8 +4109,6 @@ def urlopen(self, req):
                     'SSLV3_ALERT_HANDSHAKE_FAILURE: The server may not support the current cipher list. '
                     'Try using --legacy-server-connect', cause=e) from e
             raise
-        except HTTPError as e:  # TODO: Remove in a future release
-            raise _CompatHTTPError(e) from e
 
     def build_request_director(self, handlers, preferences=None):
         logger = _YDLLogger(self)
diff --git a/yt_dlp/compat/_legacy.py b/yt_dlp/compat/_legacy.py
index 90ccf0f14..7ea5d0812 100644
--- a/yt_dlp/compat/_legacy.py
+++ b/yt_dlp/compat/_legacy.py
@@ -35,6 +35,7 @@
 from ..dependencies import brotli as compat_brotli  # noqa: F401
 from ..dependencies import websockets as compat_websockets  # noqa: F401
 from ..dependencies.Cryptodome import AES as compat_pycrypto_AES  # noqa: F401
+from ..networking.exceptions import HTTPError as compat_HTTPError  # noqa: F401
 
 passthrough_module(__name__, '...utils', ('WINDOWS_VT_MODE', 'windows_enable_vt_mode'))
 
@@ -70,7 +71,6 @@ def compat_setenv(key, value, env=os.environ):
 compat_HTMLParser = compat_html_parser_HTMLParser = html.parser.HTMLParser
 compat_http_client = http.client
 compat_http_server = http.server
-compat_HTTPError = urllib.error.HTTPError
 compat_input = input
 compat_integer_types = (int, )
 compat_itertools_count = itertools.count
@@ -88,7 +88,7 @@ def compat_setenv(key, value, env=os.environ):
 compat_subprocess_get_DEVNULL = lambda: subprocess.DEVNULL
 compat_tokenize_tokenize = tokenize.tokenize
 compat_urllib_error = urllib.error
-compat_urllib_HTTPError = urllib.error.HTTPError
+compat_urllib_HTTPError = compat_HTTPError
 compat_urllib_parse = urllib.parse
 compat_urllib_parse_parse_qs = urllib.parse.parse_qs
 compat_urllib_parse_quote = urllib.parse.quote
diff --git a/yt_dlp/downloader/hls.py b/yt_dlp/downloader/hls.py
index d4b3f0320..4ac5d99dc 100644
--- a/yt_dlp/downloader/hls.py
+++ b/yt_dlp/downloader/hls.py
@@ -369,7 +369,10 @@ def fin_fragments():
 
                 return output.getvalue().encode()
 
-            self.download_and_append_fragments(
-                ctx, fragments, info_dict, pack_func=pack_fragment, finish_func=fin_fragments)
+            if len(fragments) == 1:
+                self.download_and_append_fragments(ctx, fragments, info_dict)
+            else:
+                self.download_and_append_fragments(
+                    ctx, fragments, info_dict, pack_func=pack_fragment, finish_func=fin_fragments)
         else:
             return self.download_and_append_fragments(ctx, fragments, info_dict)
diff --git a/yt_dlp/networking/_requests.py b/yt_dlp/networking/_requests.py
index e129110ca..00e4bdb49 100644
--- a/yt_dlp/networking/_requests.py
+++ b/yt_dlp/networking/_requests.py
@@ -8,6 +8,7 @@
 
 from ..dependencies import brotli, requests, urllib3
 from ..utils import bug_reports_message, int_or_none, variadic
+from ..utils.networking import normalize_url
 
 if requests is None:
     raise ImportError('requests module is not installed')
@@ -199,6 +200,10 @@ def rebuild_method(self, prepared_request, response):
 
         prepared_request.method = new_method
 
+        # Requests fails to resolve dot segments on absolute redirect locations
+        # See: https://github.com/yt-dlp/yt-dlp/issues/9020
+        prepared_request.url = normalize_url(prepared_request.url)
+
     def rebuild_auth(self, prepared_request, response):
         # HACK: undo status code change from rebuild_method, if applicable.
         # rebuild_auth runs after requests would remove headers/body based on status code
diff --git a/yt_dlp/networking/exceptions.py b/yt_dlp/networking/exceptions.py
index 12441901c..9037f18e2 100644
--- a/yt_dlp/networking/exceptions.py
+++ b/yt_dlp/networking/exceptions.py
@@ -1,9 +1,8 @@
 from __future__ import annotations
 
 import typing
-import urllib.error
 
-from ..utils import YoutubeDLError, deprecation_warning
+from ..utils import YoutubeDLError
 
 if typing.TYPE_CHECKING:
     from .common import RequestHandler, Response
@@ -101,117 +100,4 @@ class ProxyError(TransportError):
     pass
 
 
-class _CompatHTTPError(urllib.error.HTTPError, HTTPError):
-    """
-    Provides backwards compatibility with urllib.error.HTTPError.
-    Do not use this class directly, use HTTPError instead.
-    """
-
-    def __init__(self, http_error: HTTPError):
-        super().__init__(
-            url=http_error.response.url,
-            code=http_error.status,
-            msg=http_error.msg,
-            hdrs=http_error.response.headers,
-            fp=http_error.response
-        )
-        self._closer.close_called = True  # Disable auto close
-        self._http_error = http_error
-        HTTPError.__init__(self, http_error.response, redirect_loop=http_error.redirect_loop)
-
-    @property
-    def status(self):
-        return self._http_error.status
-
-    @status.setter
-    def status(self, value):
-        return
-
-    @property
-    def reason(self):
-        return self._http_error.reason
-
-    @reason.setter
-    def reason(self, value):
-        return
-
-    @property
-    def headers(self):
-        deprecation_warning('HTTPError.headers is deprecated, use HTTPError.response.headers instead')
-        return self._http_error.response.headers
-
-    @headers.setter
-    def headers(self, value):
-        return
-
-    def info(self):
-        deprecation_warning('HTTPError.info() is deprecated, use HTTPError.response.headers instead')
-        return self.response.headers
-
-    def getcode(self):
-        deprecation_warning('HTTPError.getcode is deprecated, use HTTPError.status instead')
-        return self.status
-
-    def geturl(self):
-        deprecation_warning('HTTPError.geturl is deprecated, use HTTPError.response.url instead')
-        return self.response.url
-
-    @property
-    def code(self):
-        deprecation_warning('HTTPError.code is deprecated, use HTTPError.status instead')
-        return self.status
-
-    @code.setter
-    def code(self, value):
-        return
-
-    @property
-    def url(self):
-        deprecation_warning('HTTPError.url is deprecated, use HTTPError.response.url instead')
-        return self.response.url
-
-    @url.setter
-    def url(self, value):
-        return
-
-    @property
-    def hdrs(self):
-        deprecation_warning('HTTPError.hdrs is deprecated, use HTTPError.response.headers instead')
-        return self.response.headers
-
-    @hdrs.setter
-    def hdrs(self, value):
-        return
-
-    @property
-    def filename(self):
-        deprecation_warning('HTTPError.filename is deprecated, use HTTPError.response.url instead')
-        return self.response.url
-
-    @filename.setter
-    def filename(self, value):
-        return
-
-    def __getattr__(self, name):
-        # File operations are passed through the response.
-        # Warn for some commonly used ones
-        passthrough_warnings = {
-            'read': 'response.read()',
-            # technically possibly due to passthrough, but we should discourage this
-            'get_header': 'response.get_header()',
-            'readable': 'response.readable()',
-            'closed': 'response.closed',
-            'tell': 'response.tell()',
-        }
-        if name in passthrough_warnings:
-            deprecation_warning(f'HTTPError.{name} is deprecated, use HTTPError.{passthrough_warnings[name]} instead')
-        return super().__getattr__(name)
-
-    def __str__(self):
-        return str(self._http_error)
-
-    def __repr__(self):
-        return repr(self._http_error)
-
-
 network_exceptions = (HTTPError, TransportError)


Test patch:
diff --git a/test/test_networking.py b/test/test_networking.py
index 62325aa8e..8cadd86f5 100644
--- a/test/test_networking.py
+++ b/test/test_networking.py
@@ -180,6 +180,12 @@ def do_GET(self):
             self.send_header('Location', '/a/b/./../../headers')
             self.send_header('Content-Length', '0')
             self.end_headers()
+        elif self.path == '/redirect_dotsegments_absolute':
+            self.send_response(301)
+            # redirect to /headers but with dot segments before - absolute url
+            self.send_header('Location', f'http://127.0.0.1:{http_server_port(self.server)}/a/b/./../../headers')
+            self.send_header('Content-Length', '0')
+            self.end_headers()
         elif self.path.startswith('/redirect_'):
             self._redirect()
         elif self.path.startswith('/method'):
@@ -345,16 +351,17 @@ def test_percent_encode(self, handler):
             res.close()
 
     @pytest.mark.parametrize('handler', ['Urllib', 'Requests'], indirect=True)
-    def test_remove_dot_segments(self, handler):
-        with handler() as rh:
+    @pytest.mark.parametrize('path', [
+        '/a/b/./../../headers',
+        '/redirect_dotsegments',
+        # https://github.com/yt-dlp/yt-dlp/issues/9020
+        '/redirect_dotsegments_absolute',
+    ])
+    def test_remove_dot_segments(self, handler, path):
+        with handler(verbose=True) as rh:
             # This isn't a comprehensive test,
-            # but it should be enough to check whether the handler is removing dot segments
-            res = validate_and_send(rh, Request(f'http://127.0.0.1:{self.http_port}/a/b/./../../headers'))
-            assert res.status == 200
-            assert res.url == f'http://127.0.0.1:{self.http_port}/headers'
-            res.close()
-
-            res = validate_and_send(rh, Request(f'http://127.0.0.1:{self.http_port}/redirect_dotsegments'))
+            # but it should be enough to check whether the handler is removing dot segments in required scenarios
+            res = validate_and_send(rh, Request(f'http://127.0.0.1:{self.http_port}{path}'))
             assert res.status == 200
             assert res.url == f'http://127.0.0.1:{self.http_port}/headers'
             res.close()
diff --git a/test/test_networking_utils.py b/test/test_networking_utils.py
index 419aae1e4..b7b71430e 100644
--- a/test/test_networking_utils.py
+++ b/test/test_networking_utils.py
@@ -8,13 +8,9 @@
 
 sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
 
-import contextlib
 import io
-import platform
 import random
 import ssl
-import urllib.error
-import warnings
 
 from yt_dlp.cookies import YoutubeDLCookieJar
 from yt_dlp.dependencies import certifi
@@ -30,7 +26,6 @@
 from yt_dlp.networking.exceptions import (
     HTTPError,
     IncompleteRead,
-    _CompatHTTPError,
 )
 from yt_dlp.socks import ProxyType
 from yt_dlp.utils.networking import HTTPHeaderDict
@@ -179,11 +174,10 @@ class TestNetworkingExceptions:
     def create_response(status):
         return Response(fp=io.BytesIO(b'test'), url='http://example.com', headers={'tesT': 'test'}, status=status)
 
-    @pytest.mark.parametrize('http_error_class', [HTTPError, lambda r: _CompatHTTPError(HTTPError(r))])
-    def test_http_error(self, http_error_class):
+    def test_http_error(self):
 
         response = self.create_response(403)
-        error = http_error_class(response)
+        error = HTTPError(response)
 
         assert error.status == 403
         assert str(error) == error.msg == 'HTTP Error 403: Forbidden'
@@ -194,80 +188,12 @@ def test_http_error(self, http_error_class):
         assert data == b'test'
         assert repr(error) == '<HTTPError 403: Forbidden>'
 
-    @pytest.mark.parametrize('http_error_class', [HTTPError, lambda *args, **kwargs: _CompatHTTPError(HTTPError(*args, **kwargs))])
-    def test_redirect_http_error(self, http_error_class):
+    def test_redirect_http_error(self):
         response = self.create_response(301)
-        error = http_error_class(response, redirect_loop=True)
+        error = HTTPError(response, redirect_loop=True)
         assert str(error) == error.msg == 'HTTP Error 301: Moved Permanently (redirect loop detected)'
         assert error.reason == 'Moved Permanently'
 
-    def test_compat_http_error(self):
-        response = self.create_response(403)
-        error = _CompatHTTPError(HTTPError(response))
-        assert isinstance(error, HTTPError)
-        assert isinstance(error, urllib.error.HTTPError)
-
-        @contextlib.contextmanager
-        def raises_deprecation_warning():
-            with warnings.catch_warnings(record=True) as w:
-                warnings.simplefilter('always')
-                yield
-
-                if len(w) == 0:
-                    pytest.fail('Did not raise DeprecationWarning')
-                if len(w) > 1:
-                    pytest.fail(f'Raised multiple warnings: {w}')
-
-                if not issubclass(w[-1].category, DeprecationWarning):
-                    pytest.fail(f'Expected DeprecationWarning, got {w[-1].category}')
-                w.clear()
-
-        with raises_deprecation_warning():
-            assert error.code == 403
-
-        with raises_deprecation_warning():
-            assert error.getcode() == 403
-
-        with raises_deprecation_warning():
-            assert error.hdrs is error.response.headers
-
-        with raises_deprecation_warning():
-            assert error.info() is error.response.headers
-
-        with raises_deprecation_warning():
-            assert error.headers is error.response.headers
-
-        with raises_deprecation_warning():
-            assert error.filename == error.response.url
-
-        with raises_deprecation_warning():
-            assert error.url == error.response.url
-
-        with raises_deprecation_warning():
-            assert error.geturl() == error.response.url
-
-        # Passthrough file operations
-        with raises_deprecation_warning():
-            assert error.read() == b'test'
-
-        with raises_deprecation_warning():
-            assert not error.closed
-
-        with raises_deprecation_warning():
-            # Technically Response operations are also passed through, which should not be used.
-            assert error.get_header('test') == 'test'
-
-        # Should not raise a warning
-        error.close()
-
-    @pytest.mark.skipif(
-        platform.python_implementation() == 'PyPy', reason='garbage collector works differently in pypy')
-    def test_compat_http_error_autoclose(self):
-        # Compat HTTPError should not autoclose response
-        response = self.create_response(403)
-        _CompatHTTPError(HTTPError(response))
-        assert not response.closed
-
     def test_incomplete_read_error(self):
         error = IncompleteRead(4, 3, cause='test')
         assert isinstance(error, IncompleteRead)