[core] Fix HTTP redirect handling

[coletdjnz]

IMPORTANT: PRs without the template will be CLOSED

Description of your pull request and other information

ADD DESCRIPTION HERE

Backport from #2861

afac4ca never properly fixed the issue (and was not tested, which was part of the problem):

• Did not handle 307/308 for POST (HTTPRedirectHandler fails on POST for 307 and 308 python/cpython#91306)
• Removed payload regardless if method changed or not (so POST to POST would strip payload. It should only remove if method changed i.e. POST to GET)
• Did not support PUT and other methods for redirect.

This PR fixes the above issues. It aligns the redirect handling with the latest recommended RFC standard/what common browsers tend to do (see #2861 for more full description of this).

I have also back-ported some of the networking tests, including the HTTP redirect test.

Template

Before submitting a pull request make sure you have:

• At least skimmed through contributing guidelines including yt-dlp coding conventions
• Searched the bugtracker for similar pull requests
• Checked the code with flake8 and ran relevant tests

In order to be accepted and merged into yt-dlp each piece of code must be in public domain or released under Unlicense. Check all of the following options that apply:

• I am the original author of this code and I am willing to release it under Unlicense
• I am not the original author of this code but it is in public domain or released under Unlicense (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)
• Core bug fix/improvement
• New feature (It is strongly recommended to open an issue first)

Copilot Summary

🤖 Generated by Copilot at f7ce007

Summary

🧹🐛🎨

Improve redirection handling and code style in yt_dlp/utils/_utils.py and remove obsolete code and tests. This pull request enhances the functionality, readability, and maintainability of the codebase.

yt_dlp cleans up
Redirection bug is fixed
Autumn leaves unused code

Walkthrough

• Simplify and fix redirection logic in YoutubeDLRedirectHandler class ([link](https://github.com/yt-dlp/yt-dlp/pull/7094/files?diff=unified&w=0#diff-feda8f56946dc048e754111850baaef0eec4b9f8bbc2d3f04b1a785626ea5c0eL1682-R1683), [link](https://github.com/yt-dlp/yt-dlp/pull/7094/files?diff=unified&w=0#diff-feda8f56946dc048e754111850baaef0eec4b9f8bbc2d3f04b1a785626ea5c0eL1696-R1717))

[Grub4K]

with FakeYDL(...) as ydl:
    ...

Can be rewritten as ydl = FakeYDL(...), saving one indentation across the file, since we are currently only using it to setup/teardown console title.

[pukkandan]

I haven't compared the actual logic to the RFC - let me know if I should or you are confident enough to merge without.

While cleaning up legacy code certainly would be nice, I don't want to hold this PR up for that, so I'm not making any suggestions for it.

PS: It's difficult to see what's moved vs new code in test_networking. In future, try to do rename and code change in separate commits.

[dirkf]

In Py3.9, this quashed the resource warnings on shutdown (not seen in 3.5 or 2.7):

• ensure daemon_threads = False in ThreadingHTTPServer (default)
• add TestHTTP.tearDown() method

    def tearDown(self):

        def closer(svr):
            def _closer():
                svr.shutdown()
                svr.server_close()
            return _closer

        shutdown_thread = threading.Thread(target=closer(self.http_httpd))
        shutdown_thread.start()
        self.http_server_thread.join(2.0)

        shutdown_thread = threading.Thread(target=closer(self.https_httpd))
        shutdown_thread.start()
        self.https_server_thread.join(2.0)

threading needs a threading.join_all() like JS Promise.all() so that the caller can wait for several threads to finish in one call.

[dirkf]

In ytdl-org/youtube-dl@c047270c02, yt-dl started to remove each Content-Encoding header after processing the encoding. The rationale was that the handler routine could be called twice with a geo-verification proxy set. If this still needs to be done, the tests TestHTT.test_{encoding} won't work as written. But shouldn't it be done anyway?

RFC9110 s8.4

The "Content-Encoding" header field indicates what content codings have been applied to the representation, beyond those inherent in the media type, and thus what decoding mechanisms have to be applied in order to obtain data in the media type referenced by the Content-Type header field. ...

Obviously RFC9110 applies to what's sent over the network rather than the processing at either end, but it seems reasonable to apply the same semantics in those contexts, so that the header only lists the encodings that haven't yet been decoded. Work-arounds with a custom header or Request attribute seem unnecessarily painful.

[coletdjnz]

In ytdl-org/youtube-dl@c047270c02, yt-dl started to remove each Content-Encoding header after processing the encoding. The rationale was that the handler routine could be called twice with a geo-verification proxy set. If this still needs to be done, the tests TestHTT.test_{encoding} won't work as written. But shouldn't it be done anyway?

RFC9110 s8.4

The "Content-Encoding" header field indicates what content codings have been applied to the representation, beyond those inherent in the media type, and thus what decoding mechanisms have to be applied in order to obtain data in the media type referenced by the Content-Type header field. ...

Obviously RFC9110 applies to what's sent over the network rather than the processing at either end, but it seems reasonable to apply the same semantics in those contexts, so that the header only lists the encodings that haven't yet been decoded. Work-arounds with a custom header or Request attribute seem unnecessarily painful.

This was changed in 65e5c02.

Funny enough, even urllib3/requests don't remove the Content-Encoding header, hence why I gave the go ahead.

I'll have to test the geo verification/per request proxy. I completely changed it in the network rework as the implementation is flawed/buggy.

[dirkf]

And I actually commented on that PR too. But I hadn't yet tracked down the commit where the removal was added.

Looking at the MDN page for content-encoding (tl;dr for the RFCs), the decoding logic is indeed essentially for encoding in reversed(headers['Content-Encoding'].split()): data = decode(data, encoding). However nested encoding would appear to have little benefit with the standard set of encodings, and presumably would only happen accidentally in some proxy configuration.

[dirkf]

Reading the HTTP specs again, it seems that a header (like Content-encoding) that can have multiple (comma-separated) values can also appear as multiple lines each containing one or more of the same set of values, so these should be equivalent:

Multi-valued-hdr: val1, val2, val3

Multi-valued-hdr: val1, val2
...
Multi-valued-hdr: val3

So my code for this is (where the decompression methods return io.BytesIO()) and txt_or_none() is lambda x: strip_or_none(x) or None:

        encodings = list(filter(txt_or_none, ', '.join(resp.headers.get_all('Content-encoding', [])).split(',')))
        for enc in encodings[::-1]:
            old_resp = resp
            if enc in ('gzip', 'x-gzip'):
                uncompressed = self.gzip_d(resp.read())
                encodings.pop()
            elif enc == 'deflate':
                uncompressed = self.reflate(resp.read())
                encodings.pop()
            elif enc == 'br' and brotli:
                uncompressed = self.brotli(resp.read())
                encodings.pop()
            else:
                break
            resp = compat_urllib_request.addinfourl(
                uncompressed, old_resp.headers, old_resp.url, old_resp.code)
            resp.msg = old_resp.msg

        if encodings:
            # remaining encodings
            resp.headers['Content-encoding'] = ' '.join(encodings)
        elif 'Content-encoding' in resp.headers:
            del resp.headers['Content-encoding']