Overall response timeout.

[Granitosaurus]

trafficstars

• [x] The bug is reproducible against the latest release and/or master.
• [x] There are no similar issues or pull requests to fix it yet.

Currently both async and sync client requests are succeptible to low-and-slow "attacks" unless explicitly handled. In other words when request is being streamed as long as 1 byte every 2 seconds (or aprox.) is being generated the connection will not close or timeout with none of the current available settings.
It's possible for the server to serve single html page almost idefinitely and hang the client thread.

Maybe httpx should introduce some optional or even default handlers for this? Maybe it's already possible to hook something like this in easily?

To reproduce

resp = httpx.get(
    "http://httpbin.org/drip?duration=30&numbytes=30&code=200&delay=2",
    timeout=Timeout(connect=3, read=3, write=3, pool=3, timeout=3),
)

The above code snippet will take 30+ seconds to complete (though this could be practically infinity) disregarding any timeout settings. The only way to avoid this is to explitictly wrap everything in either asyncio.wait_for() for async code and for sync code seems to be much more complicated (?).

[florimondmanca]

@Granitosaurus Hi!

This is an interesting topic. To be honest, I'm not sure yet if/how we would be adding security knobs like these. Security folks tend to be paranoid 😄 and I'm not sure if they wouldn't just like to put things like slow lorry defense measures in place themselves, rather than relying on the underlying HTTP lib to do it for them. There's also a case to be made to include some minimal support built-in, but then we need to discuss opt-in vs opt-out; if opt-out then what is the default; etc.

Luckily, HTTPX already enables people to implement these measures themselves without having to do advanced concurrency. I was pinged on the tuf project recently to expose a bit more how HTTPX deals with timeouts, and discussed slow retrieval there as well: https://github.com/theupdateframework/tuf/issues/1213#issuecomment-736002497

From my POV, the best bet for folks might be to write a custom transport that wraps the stream and checks against slow sending of bytes. Something like this:

import time
from typing import Iterator
import httpcore
import httpx


class TooSlowError(Exception):
    pass


class SlowGuardTransport(httpcore.SyncHTTPTransport):
    def __init__(self, transport: httpcore.SyncHTTPTransport, min_rate: str) -> None:
        self._transport = transport
        value, unit = min_rate.split("/")
        self._min_rate = float(value) / {"second": 1, "minute": 60}[unit]

    def _wrap(self, stream: Iterator[bytes]) -> Iterator[bytes]:
        last_chunk_date = time.time()

        for chunk in stream:
            # Compute current recv rate.
            # TODO: Maybe use a rolling average to reduce false positives.
            now = time.time()
            elapsed = now - last_chunk_date
            rate = len(chunk) / elapsed

            if rate <= self._min_rate:
                raise TooSlowError(f"Server is sending data too slowly: {rate:.2f} < {self._min_rate:.2f}")

            last_chunk_date = now

            yield chunk

    def request(self, *args, **kwargs):
        status_code, headers, stream, ext = self._transport.request(*args, **kwargs)
        return status_code, headers, self._wrap(stream), ext


transport = httpcore.SyncConnectionPool()  # See docs for kwargs, or wait for upcoming `httpx.HTTPTransport()`.
transport = SlowGuardTransport(transport, min_rate="10/second")
with httpx.Client(transport=transport) as client:
    # 20/second: OK
    response = client.get("http://httpbin.org/drip?duration=5&numbytes=100&code=200")
    print(response)
    # 1/second: fails with `TooSlowError`
    # 'TooSlowError: Server is sending data too slowly: 0.93 < 10.00'
    response = client.get("http://httpbin.org/drip?duration=5&numbytes=5&code=200")

I'm not sure yet whether something like this would benefit from being built-in, with what API, or if it's okay for folks to just copy-paste and tweak a bit of this code using custom transports. (The Transport API is here exactly for enabling this kind of high-precision behaviors.)

[lovelydinosaur]

Heya, thanks for raising this.

I'd potentially be a bit cautious about labelling this under "attacks"/"security", since controlling the server and attacking the client isn't a very typical scenario, but yes I do think there's scope for a few more resource limits, which I've so far only noted in passing.

pause - I'm going to follow up on this more in a moment, but first lunch is up...

[florimondmanca]

@tomchristie As far as security scenarios go, slowloris seems like a typical one to take into account to me. Eg one is building a service that uses HTTPX to interact with some other server-side service that you can't trust to not be compromised and controlled by malicious agent. Clearly that's entering paranoia land, but one thing I've learnt is dealing with security is a lot about dealing with paranoia. 😆

Anyway, I've dropped the "security" label for now as I've realized it might come off as "here's a security breach we found in HTTPX" whereas that's not what this issue is about (which might be what you were getting at).

[lovelydinosaur]

It's just that slowlorris makes sense as a client-side attack on the server, allowing attackers to DDOS a server. Whereas in the converse case, where the server has been owned by the attacker, it's an odd scenario to envisage an "attack" that is "let's make the clients using this service really slow". Maybe that's a thing that's happened(?), but I've never heard of it.

You'd more typically see simply this kind of issue on the client side framed as "resource limits to ensure graceful degradation vs. overloaded servers".

Anyways, I think it's valid either ways around, and I think a useful guideline for considering resource limits is to think about it in the same way as services like Heroku treat resource limiting.

The cases that seem like likely candidates to me might be:

• Maximum total time allowed for an HTTP request. (Eg. 60 seconds)
• Maximum total size allowed for the body of an HTTP response. (Eg. 100MB)
• Maximum total number of concurrent requests on a client. (Eg. 100)

[Granitosaurus]

Yeah I did put "attack" in quotes because it's not really an attack but it serves a very similar function.
For example a way for a server to maliciously deal with web-scrapers (or any unwanted connections) would be to stream bytes slowly since many of http client libraries have no default handlers for that - so web-scrapers would hang indefinitely consuming resources and blocking flows which could be interpreted as a malicious action against the client.

I think those 3 proposals by @tomchristie would be more than enough to deal with this!

[simonw]

I think about this problem a lot. I've written a lot of code which accepts an untrusted URL from a user and attempts to fetch that URL - a few examples:

• OpenID and IndieAuth, where a user enters the URL to their identity provider and my backend needs to fetch it.
• A really common one these days is retrieving a URL in order to "unfurl" it - extract the title, any og:image tags etc in order to display a summary of the link on a page. Chat programs, comment systems etc all do this now.
• Features that allow users to provide a URL to a CSV file in order to import data.

For all of these I really appreciate being able to use an HTTP client that makes it easy to "safely" consume a malicious URL. I want to be able to set aggressive timeouts AND limit the number of bytes retrieved too (see #1392 for a previous question I've had around limiting the number of bytes).

The attacks I'm worried about here are deliberate denial-of-service attacks - users providing malicious URLs that exhaust my server resources.

The three limits Tom suggests above are exactly what I'm after.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[lovelydinosaur]

We need to reframe this issue more specifically around the types of resource limiting that we'd still like to add, but I think it's worth us continuing to track this.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[lovelydinosaur]

Okay, I've retitled this issue so that it's just about addressing the overall request timeout.

In the comment at https://github.com/encode/httpx/issues/1450 I also mentioned some other resource limiting that we might consider, but let's treat any conversations about that separately.

For this one particular case, I can see us adding the following...

timeout = httpx.Timeout(response=60.0)
client = httpx.Client(timeout=timeout)

I'd expect that we'd want to implement the timeout behaviour at the httpcore level, not at the httpx level.

Would we want a response timeout to be on or off by default?

[rthalley]

I like this proposal, it's good to be able to limit the resources you can expend on a task (no so much for security as for coping with the amazing variety of brokenness you encounter on the Internet). My sense is that the response timeout should probably be off by default as it's really hard to guess what a good timeout would be, especially for HTTP. Someone might be getting some massive document over a slow connection, and any "default to on" behavior might break existing code. I think giving people a mechanism they can activate and specify values appropriate to their situation is enough.

One default-related thing I do think is worth discussing is what is the meaning of

client = httpx.Client(timeout = 10)

You could argue that for backwards compatibility that this means "10 second idle timeout for all operations, no overall response timeout", or you could argue "least surprise" and say it means a 10 second value for all timeouts, and thus a max block time of 10 seconds. I'm not sure which way to go on this one, as I don't know how much code out there is expecting the timeout to be only an idle timeout because they read the documentation carefully, and how much code out there made the same mistake we did in dnspython by assuming it was an overall request timeout :).

[rthalley]

One other thing: I'm definitely not arguing for any removal of the existing idle timeouts. For most things I've written involving stream connections, I've ended up with both an idle timeout and a total lifetime, because they both have useful purposes.

[lovelydinosaur]

Oh that's a really good point that I'd not considered... I'm not sure either.

From an API perspective I'd assume it to include the response timeout yup. That's a moderately big behavioural change to introduce tho. (Probs need landing in a 1.0 release)

[codingpaula]

Hello everyone! I stumbled over this issue while looking for an option to timeout our requests to ensure we are not waiting longer for a response than absolutely necessary. For us something like timeout = httpx.Timeout(total=1.0) would also work to achieve this. Will something like this or like @rthalley described be implemented in an upcoming release?

[lovelydinosaur]

For us something like timeout = httpx.Timeout(total=1.0) would also work to achieve this.

That'd be a really neat feature, yep. I'd also like us to have this. ☺️⏳

Couple of different things here...

• The implementation for this would be in httpcore - I'd be happy to help guide someone through a PR there.
• We possibly also have some changes we'd want to make to our timeout API in order to support this cleanly.

Here's our recently updated not quite live yet timeout docs.

Currently we support timeout=[httpx.Timeout|int|None]. I think this is a bit awkward, because...

# This looks like a 'total' timeout, but it's actually a network timeout.
httpx.get('http://example.com/api/v1/example', timeout=10.0)

If we switched to enforcing an explicit style everywhere, then we could ensure it's always more clear. Eg...

timeout = httpx.Timeout(network=10.0)
httpx.get('http://example.com/api/v1/example', timeout=timeout)

timeout = httpx.Timeout(network=10.0, total=60.0)
httpx.get('http://example.com/api/v1/example', timeout=timeout)

timeout = httpx.Timeout(total=60.0)
httpx.get('http://example.com/api/v1/example', timeout=timeout)

[gsakkis]

Should the overall timeout include the pool timeout? If yes the implementation is easy, at least for the async client; just wrap AsyncClient.request in asyncio.wait_for(). However at least in my use case a more useful timeout is to bound the network time: the total time for connection, write and read, excluding the time waiting for an available connection. The reason is that the latter is a client limit while the others have to do with the server.

I took a stab at implementing this but got stuck because there is not a single method or code block to guard with asyncio.wait_for(). AsyncHTTPConnection.handle_async_request involves connecting to the server, sending the request and receiving the response headers but not the response body; the latter is wrapped in a lazy HTTP11ConnectionByteStream (or HTTP2ConnectionByteStream) and read in a totally different part of the stack, in AsyncClient.send. Any guidance or suggestions how to go about it would be great.

[falkoschindler]

Thanks for mentioning asyncio.wait_for(), @gsakkis! I've been searching for such a total timeout for quite a while now, but haven't thought of using asyncio to enforce it. This is a sufficient solution for my needs:

#!/usr/bin/env python3
import asyncio
import httpx
import time

async def download():
    t = time.perf_counter()
    try:
        async with httpx.AsyncClient(timeout=1.0) as client:
            response = await asyncio.wait_for(client.get('https://httpbin.org/delay/3'), timeout=1.0)
            print(response.content.decode())
    except httpx.TimeoutException as e:
        print(f'HTTPX Timeout: {e}')
    except asyncio.TimeoutError as e:
        print(f'Asyncio Timeout: {e}')
    print(f'{time.perf_counter() - t:.3f} s')

asyncio.run(download())

Asyncio Timeout: 
1.012 s

[gsakkis]

@falkoschindler you don't need asyncio.wait_for for this url, the httpx timeout (or read timeout) is sufficient. An example where it's not sufficient is the one in the original post, where the response is returned slowly with a short interval between the chunks.

[falkoschindler]

@gsakkis You're right, the other URL "http://httpbin.org/drip" is even better for testing the total timeout. But even with "https://httpbin.org/delay/3" the normal httpx timeout takes around 1.4s instead of 1.0s, while wait_for terminates after 1.0s.

[lovelydinosaur]

Yep. Using async primitives for timeouts is the sensible approach for the asyncio and trio cases.

I'd suggest that trio has the neater design here, because it gets that timeouts make sense as context blocks. The same design is also available for asyncio codebases via anyio.

Implementing total-request-timeouts would mostly be beneficial for the sync case, since there's no cancellation semantics available to us with threaded code.

Any guidance or suggestions how to go about it would be great.

I think the timeout would need to be implemented as two separate blocks...

• A timeout block across the time required to acquire a connection, send the request and receive the response.
• A remaining timeout applied to the response bytestream iterator.

---

Also useful reading: https://stackoverflow.com/questions/21965484/timeout-for-python-requests-get-entire-response

[rthalley]

Well said Tom! And yeah, it's indeed much easier with trio as you just scope your whole task under a cancelation timeout and don't worry about the details, and you don't have to have each individual thing the blocks figure out how much time it has left. It's the sync case for httpx that's the open ticket for me. The idea you propose would work for my purposes.

[cknv]

Python 3.11 comes with asyncio.timeout, which is favoured over asyncio.wait_for and seems very similar to the trio cancellation context block, while it is probably too new to be used in httpx/httpcore, it might already be of use to anyone writing applications using httpx that have encountered this problem.

I have used it to prevent servers doing a "reverse slowloris" (unintentionally I assume) in a webhook delivery system at work, where a few slow servers would respond slower than the timeout (300+ vs 5 seconds), and would eventually slow the system so much down it was unable to keep up with the stream of new events.