Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix active req handling #244

Merged
merged 5 commits into from
Oct 8, 2024
Merged

Fix active req handling #244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
26e3167
fix own cert chain refcounting(openssl)
ekoby Oct 8, 2024
3fb4a77
fix http client close during req callbacks
ekoby Oct 8, 2024
f7b06ac
check for websocket processing
ekoby Oct 8, 2024
cb710ae
fix tests (sanitizer errors)
ekoby Oct 8, 2024
8710d90
cannot free req during parser callback
ekoby Oct 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/tlsuv/http.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ struct tlsuv_http_s {
um_header_list headers;

int connected;
bool keepalive;
tlsuv_src_t *src;
bool own_src;

Expand Down
97 changes: 42 additions & 55 deletions src/http.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,6 @@ extern tls_context *get_default_tls(void);

static void http_read_cb(uv_link_t *link, ssize_t nread, const uv_buf_t *buf);

static int http_status_cb(llhttp_t *parser, const char *status, size_t len);

static int http_message_cb(llhttp_t *parser);

static int http_body_cb(llhttp_t *parser, const char *body, size_t len);

static int http_header_field_cb(llhttp_t *parser, const char *f, size_t len);

static int http_header_value_cb(llhttp_t *parser, const char *v, size_t len);

static int http_headers_complete_cb(llhttp_t *p);

static void fail_active_request(tlsuv_http_t *c, int code, const char *msg);

static void close_connection(tlsuv_http_t *c);
Expand Down Expand Up @@ -81,51 +69,35 @@ static void http_read_cb(uv_link_t *link, ssize_t nread, const uv_buf_t *buf) {

close_connection(c);
uv_async_send(&c->proc);
if (buf && buf->base) {
tlsuv__free(buf->base);
}
return;
}

if (c->active != NULL) {

if (nread > 0) {
if (http_req_process(c->active, buf->base, nread) < 0) {
} else if (nread > 0) {
if (c->active != NULL) {
tlsuv_http_req_t *ar = c->active;
if (http_req_process(ar, buf->base, nread) < 0) {
UM_LOG(WARN, "failed to parse HTTP response");
fail_active_request(c, UV_EINVAL, "failed to parse HTTP response");
close_connection(c);
tlsuv__free(buf->base);
return;
}
}

if (c->active->state == completed) {
tlsuv_http_req_t *hr = c->active;
c->active = NULL;

bool keep_alive = true;
const char *keep_alive_hdr = tlsuv_http_resp_header(&hr->resp, "Connection");
if (strcmp(hr->resp.http_version, "1.1") == 0) {
if (keep_alive_hdr && strcasecmp("close", keep_alive_hdr) == 0)
keep_alive = false;
} else if (strcmp(hr->resp.http_version, "1.0") == 0) {
keep_alive = keep_alive_hdr && strcasecmp("keep-alive", keep_alive_hdr) == 0;
} else {
UM_LOG(WARN, "unexpected HTTP version(%s)", hr->resp.http_version);
keep_alive = false;
}
if (ar->state == completed) {
bool keepalive = c->keepalive;
const char *keep_alive_hdr = tlsuv_http_resp_header(&ar->resp, "Connection");
if (keep_alive_hdr) {
keepalive = strcasecmp(keep_alive_hdr, "close") != 0;
}

http_req_free(hr);
tlsuv__free(hr);
c->active = NULL;
http_req_free(ar);
tlsuv__free(ar);

if (!keep_alive) {
close_connection(c);
} else {
uv_async_send(&c->proc);
if (keepalive) {
uv_async_send(&c->proc);
} else {
close_connection(c);
}
}
} else {
UM_LOG(ERR, "received %zd bytes without active request", nread);
}
} else if (nread > 0) {
UM_LOG(ERR, "received %zd bytes without active request", nread);
}

if (buf && buf->base) {
Expand All @@ -148,15 +120,24 @@ static void clear_req_body(tlsuv_http_req_t *req, int code) {
}

static void fail_active_request(tlsuv_http_t *c, int code, const char *msg) {
if (c->active != NULL && c->active->resp_cb != NULL) {
if (c->active == NULL) return;

// this is called from active request callback
if (c->active->state == completed) return;

if (c->active->resp_cb != NULL) {
c->active->resp.code = code;
c->active->resp.status = tlsuv__strdup(msg);
c->active->resp_cb(&c->active->resp, c->active->data);
clear_req_body(c->active, code);
http_req_free(c->active);
tlsuv__free(c->active);
c->active = NULL;
c->active->resp_cb = NULL;
} else if (c->active->resp.body_cb != NULL) {
c->active->resp.body_cb(c->active, NULL, code);
}

clear_req_body(c->active, code);
http_req_free(c->active);
tlsuv__free(c->active);
c->active = NULL;
}

static void fail_all_requests(tlsuv_http_t *c, int code, const char *msg) {
Expand Down Expand Up @@ -395,6 +376,11 @@ static void process_requests(uv_async_t *ar) {
if (c->active == NULL && !STAILQ_EMPTY(&c->requests)) {
c->active = STAILQ_FIRST(&c->requests);
STAILQ_REMOVE_HEAD(&c->requests, _next);

// if not keepalive close connection before next request
if (!c->keepalive) {
close_connection(c);
}
}

if (c->active == NULL) {
Expand Down Expand Up @@ -656,8 +642,9 @@ int http_req_cancel_err(tlsuv_http_t *clt, tlsuv_http_req_t *req, int error, con
req->resp.status = tlsuv__strdup(msg ? msg : uv_strerror(error));
clear_req_body(req, req->resp.code);

if (req->state < headers_received) { // resp_cb has not been called yet
req->resp_cb(&r->resp, r->data);
if (req->state < headers_received && req->resp_cb) { // resp_cb has not been called yet
req->resp_cb(&req->resp, req->data);
req->resp_cb = NULL;
} else if (req->resp.body_cb) {
req->resp.body_cb(req, NULL, req->resp.code);
}
Expand Down
6 changes: 6 additions & 0 deletions src/http_req.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,15 +361,21 @@ static int http_status_cb(llhttp_t *parser, const char *status, size_t len) {
tlsuv_http_req_t *r = parser->data;
r->resp.code = (int) parser->status_code;
snprintf(r->resp.http_version, sizeof(r->resp.http_version), "%1d.%1d", parser->http_major, parser->http_minor);
if (r->client) {
r->client->keepalive = !(parser->http_major == 1 && parser->http_minor == 0);
}
r->resp.status = tlsuv__calloc(1, len+1);
strncpy(r->resp.status, status, len);
return 0;
}



static int http_message_cb(llhttp_t *parser) {
UM_LOG(VERB, "message complete");
tlsuv_http_req_t *r = parser->data;
r->state = completed;

if (r->resp.body_cb) {
if (r->inflater == NULL || um_inflate_state(r->inflater) == 1) {
r->resp.body_cb(r, NULL, UV_EOF);
Expand Down
12 changes: 5 additions & 7 deletions src/openssl/engine.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -719,16 +719,14 @@ if ((op) != 1) { \
static X509* tls_set_cert_internal (SSL_CTX* ssl, X509_STORE *store) {
STACK_OF(X509_OBJECT) *certs = X509_STORE_get0_objects(store);
X509 *crt = X509_OBJECT_get0_X509(sk_X509_OBJECT_value(certs, 0));
X509_up_ref(crt);
SSL_CTX_use_certificate(ssl, crt);

// rest of certs go to chain
if (sk_X509_OBJECT_num(certs) > 1) {
X509_STORE *chain = X509_STORE_new();
// skip first one
for (int i = 1; i < sk_X509_OBJECT_num(certs); i++) {
X509_STORE_add_cert(chain, X509_OBJECT_get0_X509(sk_X509_OBJECT_value(certs, i)));
}
SSL_CTX_set0_chain_cert_store(ssl, chain);
for (int i = 1; i < sk_X509_OBJECT_num(certs); i++) {
X509 *x509 = X509_OBJECT_get0_X509(sk_X509_OBJECT_value(certs, i));
X509_up_ref(x509);
SSL_CTX_add_extra_chain_cert(ssl, x509);
}
return crt;
}
Expand Down
25 changes: 25 additions & 0 deletions tests/http_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ TEST_CASE("conn failures", "[http]") {
REQUIRE(resp.code == UV_ECONNREFUSED);

tlsuv_http_close(&clt, nullptr);
test.run();
}


Expand Down Expand Up @@ -282,6 +283,7 @@ TEST_CASE("http_tests", "[http]") {
}
tlsuv_http_close(&clt, nullptr);
tlsuv_set_global_connector(nullptr);
test.run();
}

#if defined(HSM_LIB)
Expand Down Expand Up @@ -316,6 +318,7 @@ TEST_CASE("pkcs11_client_cert_test","[http]") {
tlsuv_http_close(&clt, nullptr);
if (tls)
tls->free_ctx(tls);
test.run();
}
#endif

Expand Down Expand Up @@ -423,11 +426,13 @@ TEST_CASE("client_cert_test","[http]") {
CHECK(resp.resp_body_end_called);
CHECK(resp.body == "you are 'CN=BadSSL Client Certificate,O=BadSSL,L=San Francisco,ST=California,C=US' by CN=BadSSL Client Root Certificate Authority,O=BadSSL,L=San Francisco,ST=California,C=US");
c->free(c);
test.run();
}

tlsuv_http_close(&clt, nullptr);
if (tls)
tls->free_ctx(tls);
test.run();
}

const int ONE_SECOND = 1000000;
Expand Down Expand Up @@ -470,6 +475,7 @@ TEST_CASE("client_idle_test","[http]") {

tlsuv_http_close(&clt, nullptr);
}
test.run();
}

// hidden test
Expand Down Expand Up @@ -511,6 +517,7 @@ TEST_CASE("server_idle_close","[.]") {

tlsuv_http_close(&clt, nullptr);
}
test.run();
}

TEST_CASE("basic_test", "[http]") {
Expand Down Expand Up @@ -538,6 +545,7 @@ TEST_CASE("basic_test", "[http]") {
tlsuv_http_close(&clt, nullptr);

tlsuv_set_global_connector(nullptr);
test.run();
}

TEST_CASE("invalid CA", "[http]") {
Expand All @@ -555,6 +563,7 @@ TEST_CASE("invalid CA", "[http]") {
}

tlsuv_http_close(&clt, nullptr);
test.run();
}


Expand All @@ -573,6 +582,7 @@ TEST_CASE("http_prefix", "[http]") {
CHECK_THAT(resp.headers["Content-Length"], Equals("256"));

tlsuv_http_close(&clt, nullptr);
test.run();
}

TEST_CASE("http_prefix_after", "[http]") {
Expand All @@ -592,6 +602,7 @@ TEST_CASE("http_prefix_after", "[http]") {
CHECK_THAT(resp.headers["Content-Length"], Equals("256"));

tlsuv_http_close(&clt, nullptr);
test.run();
}

TEST_CASE("content_length_test", "[http]") {
Expand Down Expand Up @@ -630,6 +641,7 @@ TEST_CASE("content_length_test", "[http]") {
}

tlsuv_http_close(&clt, nullptr);
test.run();
}

TEST_CASE("multiple requests", "[http]") {
Expand Down Expand Up @@ -685,6 +697,7 @@ TEST_CASE("multiple requests", "[http]") {
}

tlsuv_http_close(&clt, nullptr);
test.run();
}

// test proper client->engine cleanup between requests
Expand All @@ -708,6 +721,7 @@ TEST_CASE("TLS reconnect", "[http]") {
CHECK(resp2.code == 200);

tlsuv_http_close(&clt, nullptr);
test.run();
}

typedef struct verify_ctx_s {
Expand Down Expand Up @@ -751,6 +765,7 @@ TEST_CASE("large POST(GH-87)", "[http][gh-87]") {

tlsuv_http_close(&clt, nullptr);
free(buf);
test.run();
}

TEST_CASE("TLS verify with JWT", "[http]") {
Expand Down Expand Up @@ -802,6 +817,7 @@ TEST_CASE("TLS verify with JWT", "[http]") {

free(vtx.sig);
tls->free_ctx(tls);
test.run();
}

TEST_CASE("TLS to IP address", "[http]") {
Expand All @@ -822,6 +838,7 @@ TEST_CASE("TLS to IP address", "[http]") {
CHECK(resp.headers["Content-Type"] == "application/dns-json");
CHECK_THAT(resp.body, Catch::Matchers::ContainsSubstring("\"Answer\":[{\"name\":\"google.com\""));
tlsuv_http_close(&clt, nullptr);
test.run();
}

TEST_CASE("connect timeout", "[http]") {
Expand All @@ -848,6 +865,7 @@ TEST_CASE("connect timeout", "[http]") {
CHECK(resp.code == UV_ETIMEDOUT);
CHECK(resp2.code == UV_ETIMEDOUT);
tlsuv_http_close(&clt, nullptr);
test.run();
}


Expand Down Expand Up @@ -876,6 +894,7 @@ TEST_CASE("HTTP gzip", "[http]") {
tlsuv_http_close(clt, [](tlsuv_http_t *c) {
delete c;
});
test.run();
}

TEST_CASE("HTTP deflate", "[http]") {
Expand All @@ -902,6 +921,7 @@ TEST_CASE("HTTP deflate", "[http]") {
std::cout << resp.req_body << std::endl;

tlsuv_http_close(&clt, nullptr);
test.run();
}

TEST_CASE("URL encode", "[http]") {
Expand Down Expand Up @@ -939,6 +959,7 @@ TEST_CASE("URL encode", "[http]") {

tlsuv_http_close(&clt, nullptr);
json_value_free(j);
test.run();
}

class testdata {
Expand Down Expand Up @@ -989,6 +1010,7 @@ TEST_CASE("form test", "[http]") {
tlsuv_http_close(&clt, nullptr);

json_value_free(jval);
test.run();
}

TEST_CASE("form test too big", "[http]") {
Expand Down Expand Up @@ -1021,6 +1043,7 @@ TEST_CASE("form test too big", "[http]") {
REQUIRE(resp.status == "form data too big");

tlsuv_http_close(&clt, nullptr);
test.run();
}

TEST_CASE("test req header too big", "[http]") {
Expand Down Expand Up @@ -1051,6 +1074,7 @@ TEST_CASE("test req header too big", "[http]") {
REQUIRE(resp.status == "request header too big");

tlsuv_http_close(&clt, nullptr);
test.run();
}
}

Expand Down Expand Up @@ -1083,6 +1107,7 @@ TEST_CASE("test request cancel", "[http]") {
CHECK(td.resp2.code == UV_ECANCELED);

tlsuv_http_close(&td.clt, nullptr);
test.run();
}

#define TEST_FIELD(f, VAL) \
Expand Down
Loading