clj-commons / clj-commons/aleph

Add support for aggregating all inbound data before calling server Ring handler

Open
#692 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Clojure
Stars
2.6k
Forks
242
Avg merge
4d 5h
Merged PRs (30d)
1

Description

## About

The official Ring spec states that the body of a request should be an InputStream, and Aleph complies with this (when not in raw mode).

However, InputStreams aren't always desirable, because they're a blocking interface.

If no data is available (and you haven't reached EOS), any read calls will block. Block on enough of these in your Ring handlers, and you can run out of threads/resources. This limitation propagates to many common transformations of your InputStream, including `slurp` and `bs/to-string`.

One way around this is to let Aleph/Netty accumulate all the body ByteBufs before calling the user handler with the Ring request map. Netty won't consume extra threads for this, though of course, memory will be consumed.

Aggregation currently happens automatically if you set the `max-request-body-size` option, but that's an implementation detail. This issue aims to be more explicit.

## Proposal

Add an option like `aggregate-data?`, `wait-for-body-to-finish?`, or whatever, which adds aggregation. For HTTP1, it would be the HttpObjectAggregator. For HTTP2, it'll use the custom aggregator used for the body size limit.

While we shouldn't always force a _size_ limit, we must still apply _some_ limits, or a trivial DoS attack will be to send an infinite stream to a server, and force it to consume all memory.

We can either apply a size limit, a time limit, or both. If the aggregation option is set, so must one of the limit options. We can
stick with `max-request-body-size`, but we should probably add a new one for a time limit. Maybe `max-request-time` or `max-execution-time`. We already have a bunch of timeouts on the client side, like `request-timeout` and `read-timeout`, maybe we should call it `read-timeout` for consistency.

Contributor guide

Open the contributing guide

Research direction

Start by tracing the existing max-request-body-size handling for HTTP/1 and HTTP/2, then compare it with the Ring handler invocation. Define the aggregation option and required size or time limit, and consider the issue's proposed HttpObjectAggregator and custom HTTP/2 aggregator; done means bounded request bodies are fully aggregated before the handler runs.

Written by the indexing model from the issue text.

Assessment

Tech stack
clojure
Domain
api, backend
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.