Solar Designer
unread,Mar 16, 2016, 9:27:37 PM3/16/16Sign in to reply to author
Sign in to forward
You do not have permission to delete messages in this group
Sign in to report message
Either email addresses are anonymous for this group or you need the view member email addresses permission to view the original message
to onion-dev
David,
When reviewing the poller code recently, I found it suspicious that you
don't poll for readiness to write (no EPOLLOUT, nor the handling this
would require).
This probably means that large or just unlucky responses may block the
thread or/and be truncated (or worse, have portions missing).
The sockets don't appear to be set to non-blocking, or are they? So
blocking on response write is possible, correct? If so, just a handful
of malicious or unlucky client connections (matching the number of
threads on the server) may result in a denial-of-service, even if the
number of fd's on the server is much larger. (In practice, this would
be tricky or impossible to do when responses are small. But some
servers may have URLs with sufficiently large responses.)
http.c: onion_http_write() simply uses the write(2) syscall, and returns
its return value. response.c sometimes uses ->write()'s return value,
and sometimes not. When it does not, a portion of the data may end up
being omitted or the response truncated. In particular, this is the
case when sending headers and short strings specific to chunked-encoding,
The code sending headers will require some effort to repair because
there are multiple internal APIs that don't pass a status back - e.g.,
even if write_header() is changed to have a non-void return type,
there's also onion_dict_preorder() to which write_header is passed, and
three other functions in the file call onion_response_write_headers()
ignoring its return value.
For a long-term fix, I think it'd be better for the poller (rewritten
from scratch) to manage both reads and writes.
Alexander