Skip to content
This repository has been archived by the owner on Jun 30, 2021. It is now read-only.

Confusion with on_conn_error and on_error hooks #93

Open
damonearp opened this issue Apr 19, 2018 · 2 comments
Open

Confusion with on_conn_error and on_error hooks #93

damonearp opened this issue Apr 19, 2018 · 2 comments

Comments

@damonearp
Copy link

Details

evhtp_hook_on_conn_error is never used. A quick search of the repo only shows this hook being set here evhtp.c:4325
evhtp_hook_on_error isn't used if only set in struct evhtp_connection_s. It must be explicitly set on every request struct. evhtp.c:758

Partial Workaround

Use evhtp_hook_on_path to set it on the evhtp_request_t with evhtp_request_set_hook(). However on_error will only be called if the connection get's to this callback. For Example

perl -e '$|=1; print "GET /file00 HTTP/1.1\r\n"; while(<>){}' | nc localhost 8080
$ ./server 8080
error_cb

Steps or code to reproduce the problem.

  1. Set a short timeout, I used 1 second
  2. Set hooks on_conn_error and on_error
  3. connect to server with empty or partial request: ex nc localhost 8080
  4. Wait for timeout to close connection and you won't see logging from either error callbacks.

Example code (if applicable)

evhtp_res 
conn_error_cb(evhtp_connection_t *conn, evhtp_error_flags type, void *arg)
{
    printf("%s\n", __FUNCTION__);
    return EVHTP_RES_OK;
}

evhtp_res 
error_cb(evhtp_request_t *req, evhtp_error_flags type, void *arg)
{
    printf("%s\n", __FUNCTION__);
    return EVHTP_RES_OK;
}

evhtp_res 
pre_accept_cb(evhtp_connection_t *conn, void *arg)
{
    evhtp_connection_set_hook(conn, evhtp_hook_on_conn_error, &conn_error_cb, NULL);
    evhtp_connection_set_hook(conn, evhtp_hook_on_error, &error_cb, NULL);
    return EVHTP_RES_OK;
}

int
main(int argc, const char ** argv)
{
    struct timeval = { .tv_sec = 1, .tv_usec = 0 };
    ...
    evhtp_set_timeouts(htp, &to, &to);
    evhtp_set_pre_accept_callback(htp, &pre_accept_cb, NULL);
    ...
}

Version

1.2.16

@NathanFrench NathanFrench self-assigned this Apr 20, 2018
NathanFrench added a commit to NathanFrench/libevhtp that referenced this issue Jun 23, 2018
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.
@NathanFrench
Copy link
Collaborator

Please check out https://github.com/criticalstack/libevhtp/tree/issue93
And the documentation for the pull request here #98

@NathanFrench
Copy link
Collaborator

Bump... Any issues with this set of changes?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants