[#93] make on_conn_error and on_error hooks unique #98
Conversation
The functions htp_hook_error_() is only called for request-ish errors
while htp_hook_connection_error_() is only called for connection-ish
errors.
This mainly applies to the following functions:
`htp__connection_readcb_` :
if `EVHTP_RES_DATA_TOO_LONG` is the connection request return value,
both `hook_error_()` and `hook_connection_error()` are called.
if there is a parser error, `hook_connection_error_()` is called
with the error type of `EVHTP_RES_PARSER_ERROR`
`evbuffer_drain` is now called before the connection request status
switch/case statement, and `EVHTP_RES_PAUSE` is now a case (moved)
`htp__connection_eventcb_`
!!!!! PLEASE MAKE NOTE OF THIS CHANGE !!!!!
this function now calls `htp__hook_connection_error_()` but no
longer calls `htp__hook_request_error_()` to match the naming
scheme.
prior to this, both request_error and connection_error was called
within this function.
|
Just did a test of the connection timeouts and found a segfault. set error handlers like so: static void connection_error_callback(evhtp_connection_t *conn, evhtp_error_flags type, void *arg)
{
log("unhandled connection error of type: 0x%02x", type);
}
static void request_error_callback(evhtp_request_t *req, evhtp_error_flags type, void *arg)
{
log("unhandled request error of type: 0x%02x", type);
}
evhtp_res pre_accept_callback(evhtp_connection_t *conn, void *arg)
{
evhtp_connection_set_hook(conn, evhtp_hook_on_conn_error, (void*) &connection_error_callback, NULL);
evhtp_connection_set_hook(conn, evhtp_hook_on_error, (void*) &request_error_callback, NULL);
return EVHTP_RES_OK;
}
int server_init()
{
struct timeval connection_timeout;
connection_timeout.tv_sec = 1;
connection_timeout.tv_usec = 0;
evhtp_set_timeouts(server->evhtp, &connection_timeout, &connection_timeout);
...
}I then connected to the server with nc and no http request: $ nc localhost 8080Server Output$ ./evhtp_to
connection_error_callback:159 unhandled connection error of type: 0x41
Segmentation fault: 11Debugging with |
|
I'm thinking about removing the request error callback, as it was not being used before, and it's even confusing me, as request can free connection, and vice versa. (That's what is happening here, connection_free frees request, and sets the pointer to NULL So I think it would be prudent to have an event callback, and a generic error callback. You know, like it was before, but without having the option for a request error callback. |
|
Basically it would be nice to be able to differentiate between timeouts, parsing errors, client disconnects, etc so they can be logged. If that's achievable with one error callback go for it. |
I'm on it. Thanks for the feedback! |
|
|
The functions htp_hook_error_() is only called for request-ish errors
while htp_hook_connection_error_() is only called for connection-ish
errors.
This mainly applies to the following functions:
htp__connection_readcb_:if
EVHTP_RES_DATA_TOO_LONGis the connection request return value,both
hook_error_()andhook_connection_error()are called.if there is a parser error,
hook_connection_error_()is calledwith the error type of
EVHTP_RES_PARSER_ERRORevbuffer_drainis now called before the connection request statusswitch/case statement, and
EVHTP_RES_PAUSEis now a case (moved)htp__connection_eventcb_!!!!! PLEASE MAKE NOTE OF THIS CHANGE !!!!!
this function now calls
htp__hook_connection_error_()but nolonger calls
htp__hook_request_error_()to match the namingscheme.
prior to this, both request_error and connection_error was called
within this function.