karlseguin / http.zig

An HTTP/1.1 server for zig
MIT License
454 stars 31 forks source link

allow calls to response.writer to re-use the same writer result #46

Closed zigster64 closed 2 months ago

zigster64 commented 2 months ago

There is an interesting thing happening with calls to response.writer() - each call to this will return a different Writer instance that uses the same response buffers.

This may be intentional - or it may be a bug ?

As a result, if you do things like this :

in your dispatcher you call action(G, req, res) ... and in your handlers, call res.writer() to get direct access to the writer.

then after action() returns, call some other things that take res as a param, and use that to get a res.writer() to output more data

Inside the new writer, the length of the new buffer will be set to the minimum (say 4096) .. but the data that tracks the writer thinks that the buffer is much larger. Pretty quickly, it will segfault as it tries to access a slice that it thinks is large, but is now only 4kb

Solution here is to make res.writer() a singleton - once its called, subsequent calls get the same writer object instead of a new one that has different buffer accounting

Bit hard to explain beyond that, but have a look and you will see what I mean. Its quite subtle :)

I might try adding a test case to this PR (need to sleep first) I think calling res.writer(), writing a lot of data to it so that it has to call grow() ... and then calling res.writer() again, and trying to write should invoke the segfault.

Anyway - Im running my app with this patch, and pushing some very large responses through now, and no probs

karlseguin commented 2 months ago

Your solution would have worked, but I think a better solution is to make Writer.init reference the correct (active) body. Currently, every call to res.writer(), which calls Writer.init(), forces static_body_body into body_buffer, even though there might already be an existing body_buffer.

A quick solution is to check if body_buffer != null. But I decided to make body_buff non-nullable. It'll always point to static_body_buffer or a pooled/dynamic buffer , and body_len > 0 can be used to determine if there's a body.

https://github.com/karlseguin/http.zig/commit/ee92e0a6feca8b831fea9657dcf69a79af35a07c