Re: [Libevent-users] http server and infinite streams

[Nick Mathewson <nickm@xxxxxxxxxxxxx>]

On Sat, Apr 30, 2011 at 9:55 PM, Cliff Frey <cliff@xxxxxxxxxx> wrote:
> I added tests, handled the infinite-receive case as well as the
> infinite-send case, and slightly renamed a few things.
>
> https://github.com/clifffrey/Libevent/tree/http-transfer-throttling
>
> I believe that the changes there are enough to prevent out-of-memory
> conditions when sending or receiving huge HTTP requests.

Great; I will try to review and either merge or comment some time this
week.  Please bug me if I don't!  (My apologies if I'm slow; things
are busy in my day job and family life this week.  Good, but still
busy!)

> Part of me wonders if this is the wrong way to go with the fixes, and
> if it would somehow be better/natural to make the HTTP layer act a
> little bit more like a filter, and expose a struct bufferevent
> interface to the incoming/outgoing data stream.  However, that seems
> like it would end up being a bigger rewrite, and this is a concrete
> fix to a specific problem, so this change still feels worthwhile to
> me.

Yeah; I'm no big fan of the current interface, and I would certainly
like to expose a better one.  Exposing the bufferevent substrate would
get us a lot.  The tricky parts would be actually designing an
improved interface and making sure that we can build it without
breaking code that relies on the old one.  If somebody wants to sketch
out what they think a better API should look like, that would be
grand.

Either way though, it's a good idea to separate out bugfixes from
rewrites; for stability, I'm trying hard to keep 2.0 bugfix-only and
to make sure all the new features and major refactorings go into 2.1
or later.

>>> Also, a completely different bug:  If you want to support potentially
>>> infinite POST streams from clients (imagine that you wanted to
>>> implement word-count as an http server, where they POST a document,
>>> and you return the word count) then clients can run your server out of
>>> memory by sending one very large chunk.  I think that the
>>> evhttp_request_set_chunked_cb callback should be called on every read,
>>> not just when a complete HTTP chunk has been read.  I have made a
>>> patch that does this, but I worry that maybe some user out there
>>> depends on the only-read-complete-http-chunk behavior.
>>
>> Perhaps a flag of some kind could be set so that anybody depending on
>> the old behavior can get it?
>
> I did not add this flag because I really feel like it would be a weird
> flag to have, and do not imagine any code actually depended on the old
> behavior.  If someone has a real-world example that this would break,
> I will add the flag.

I am really hesitant to merge stuff like that during a stable series.
While conformant applications *shouldn't* are how HTTP objects are
chunked, I can totally imagine somebody writing an application that
assumed a given chunking, and getting surprised when the behavior
changed.

> Let me know if any more test cases are necessary, or if you want me to
> make any other changes.

Will do, and thanks for the patches!  Again, please bug me if I
haven't reviewed them by week's end.

yrs,
-- 
Nick
***********************************************************************
To unsubscribe, send an e-mail to majordomo@xxxxxxxxxxxxx with
unsubscribe libevent-users    in the body.