From 6ae2cff1e9d328b8b5d8bfda1fc6b7d29757818d Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Mon, 19 Aug 2024 19:45:30 +0100 Subject: [PATCH 1/3] =?UTF-8?q?Add=20API=20to=20determine=20the=20request?= =?UTF-8?q?=20target=20following=20RFC=209112=E1=BA=9E3.3.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * src/ne_request.c (ne_get_request_target, get_request_target_uri): New functions. * src/ne_request.h, src/neon.vers: Add API. * test/request.c (targets): New test case. --- src/ne_request.c | 38 ++++++++++++++++++++++++++++++++++++++ src/ne_request.h | 5 +++++ src/neon.vers | 1 + test/request.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 87 insertions(+) diff --git a/src/ne_request.c b/src/ne_request.c index 161b0e7b..bf6c986b 100644 --- a/src/ne_request.c +++ b/src/ne_request.c @@ -556,6 +556,44 @@ ne_request *ne_request_create(ne_session *sess, const char *method, return req; } +/* Reconstruct the request target URI following RFC 9112ẞ3.3. Returns + * zero on success or non-zero on error. */ +static int get_request_target_uri(ne_request *req, ne_uri *uri) +{ + if (strcmp(req->target, "*") == 0 + || strcmp(req->method, "CONNECT") == 0) { + /* asterisk-form or authority-form. Since neon only ever uses + * authority-form with a CONNECT to the origin server (which + * is the session host) there is no need to re-parse + * req->target to extract it. */ + ne_fill_server_uri(req->session, uri); + uri->path = ne_strdup(""); + return 0; + } + else if (req->target[0] == '/') { + /* origin-form. */ + ne_fill_server_uri(req->session, uri); + uri->path = ne_strdup(req->target); + return 0; + } + else { + /* absolute-form */ + return ne_uri_parse(req->target, uri); + } +} + +ne_uri *ne_get_request_target(ne_request *req) +{ + ne_uri *rv = ne_calloc(sizeof *rv); + + if (get_request_target_uri(req, rv)) { + ne_uri_free(rv); + return NULL; + } + + return rv; +} + /* Set the request body length to 'length' */ static void set_body_length(ne_request *req, ne_off_t length) { diff --git a/src/ne_request.h b/src/ne_request.h index 0398987d..f25a545e 100644 --- a/src/ne_request.h +++ b/src/ne_request.h @@ -166,6 +166,11 @@ void ne_print_request_header(ne_request *req, const char *name, const char *format, ...) ne_attribute((format(printf, 3, 4))); +/* Returns the request target URI as a malloc-allocated ne_uri object, + * or NULL on error if the request target cannot be determined. The + * session error string is not changed on error. */ +ne_uri *ne_get_request_target(ne_request *req); + /* If the response includes a Location header, this function parses * and resolves the URI-reference relative to the request target. If * a fragment ("#fragment") is used for the request target, it can be diff --git a/src/neon.vers b/src/neon.vers index 51bde7f4..945ed574 100644 --- a/src/neon.vers +++ b/src/neon.vers @@ -42,6 +42,7 @@ NEON_0_33 { NEON_0_34 { ne_get_response_location; + ne_get_request_target; ne_iaddr_set_scope; ne_iaddr_get_scope; }; diff --git a/test/request.c b/test/request.c index 7de0430d..292fc1a3 100644 --- a/test/request.c +++ b/test/request.c @@ -2435,6 +2435,48 @@ static int ipv6_literal(void) return destroy_and_wait(sess); } +static int targets(void) +{ + struct { + const char *scheme; + const char *host; + int port; + const char *method; + const char *target; + const char *expected; + } ts[] = { + { "http", "example.com", 80, "GET", "/fish", "http://example.com/fish" }, + { "http", "example.com", 8080, "GET", "/fish", "http://example.com:8080/fish" }, + { "https", "example.com", 443, "GET", "/", "https://example.com/" }, + { "http", "proxy.example.com", 80, "GET", "ftp://example.com/fishfood", "ftp://example.com/fishfood" }, + { "https", "example.com", 443, "OPTIONS", "*", "https://example.com" }, + { NULL } + }; + unsigned n; + + for (n = 0; ts[n].scheme != NULL; n++ ) { + ne_session *sess; + ne_request *req; + ne_uri *uri; + char *actual; + + sess = ne_session_create(ts[n].scheme, ts[n].host, ts[n].port); + req = ne_request_create(sess, ts[n].method, ts[n].target); + uri = ne_get_request_target(req); + actual = uri ? ne_uri_unparse(uri) : NULL; + + ONCMP(ts[n].expected, actual, "request target", "URI"); + + if (actual) ne_free(actual); + if (uri) ne_uri_free(uri); + + ne_request_destroy(req); + ne_session_destroy(sess); + } + + return OK; +} + /* TODO: test that ne_set_notifier(, NULL, NULL) DTRT too. */ ne_test tests[] = { @@ -2511,5 +2553,6 @@ ne_test tests[] = { T(dont_retry_408), T(ipv6_literal), T(redirect_error), + T(targets), T(NULL) }; From 0d302f8c363c9d17f5155d85ce2d02f39b0a009e Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Mon, 19 Aug 2024 19:54:42 +0100 Subject: [PATCH 2/3] Fix new Location: handling for non-origin-form request-target. * src/ne_request.c (ne_get_response_location): Rewrite to use target URI correctly determined from get_request_target_uri, fixing non-origin-form cases. * test/redirect.c (redirects): Test for non-origin-form targets. --- src/ne_request.c | 24 +++++++++++++++--------- test/redirect.c | 4 ++++ test/request.c | 5 ++++- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/ne_request.c b/src/ne_request.c index bf6c986b..781d77f0 100644 --- a/src/ne_request.c +++ b/src/ne_request.c @@ -762,24 +762,30 @@ static void free_response_headers(ne_request *req) ne_uri *ne_get_response_location(ne_request *req, const char *fragment) { const char *location; - ne_uri dest, base, *ret; + ne_uri dest, base, *ret = NULL; location = get_response_header_hv(req, HH_HV_LOCATION, "location"); if (location == NULL) return NULL; + memset(&base, 0, sizeof base); + memset(&dest, 0, sizeof dest); + + /* Location is a URI-reference (RFC9110ẞ10.2.2) relative to the + * request target URI; determine each of these then resolve. */ + /* Parse the Location header */ if (ne_uri_parse(location, &dest) || !dest.path) { - ne_set_error(req->session, _("Could not parse redirect " + ne_set_error(req->session, _("Could not parse redirect " "destination URL")); - return NULL; + goto fail; } - /* Location is a URI-reference (RFC9110ẞ10.2.2) relative to the - * request target URI; create that base URI: */ - memset(&base, 0, sizeof base); - ne_fill_server_uri(req->session, &base); - base.path = req->target; + if (get_request_target_uri(req, &base)) { + ne_set_error(req->session, _("Could not parse redirect " + "destination URL")); + goto fail; + } ret = ne_malloc(sizeof *ret); ne_uri_resolve(&base, &dest, ret); @@ -789,7 +795,7 @@ ne_uri *ne_get_response_location(ne_request *req, const char *fragment) ret->fragment = ne_strdup(fragment); } - base.path = NULL; /* owned by ne_request object, don't free */ +fail: ne_uri_free(&base); ne_uri_free(&dest); diff --git a/test/redirect.c b/test/redirect.c index 12ccae9d..0a6fa703 100644 --- a/test/redirect.c +++ b/test/redirect.c @@ -144,6 +144,10 @@ static int redirects(void) /* all 3xx should get NE_REDIRECT. */ {PATH, 399, DEST, DEST, NULL}, + /* Handling of various request-target cases. */ + {"*", 307, "/fish#food", "/fish#food", NULL}, + {"ftp://example.com/fish", 307, "/fish#food", "ftp://example.com/fish#food", NULL}, + /* More relative URIs: */ {"/blah", 307, "//example.com:8080/fish#food", "http://example.com:8080/fish#food", NULL}, {"/blah", 307, "#food", "/blah#food", NULL}, diff --git a/test/request.c b/test/request.c index 292fc1a3..8d42c033 100644 --- a/test/request.c +++ b/test/request.c @@ -2468,7 +2468,10 @@ static int targets(void) ONCMP(ts[n].expected, actual, "request target", "URI"); if (actual) ne_free(actual); - if (uri) ne_uri_free(uri); + if (uri) { + ne_uri_free(uri); + ne_free(uri); + } ne_request_destroy(req); ne_session_destroy(sess); From c7a26ba73764fa7cc745a8fe9b0e1490c04a4052 Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Mon, 19 Aug 2024 20:40:14 +0100 Subject: [PATCH 3/3] * src/ne_request.c (struct ne_request_s): Add target_uri field. (ne_get_response_location): Return const, cache the URI in req->target_uri. (ne_get_response_location): Update accordingly. * test/request.c (targets): Update accordingly. --- src/ne_request.c | 35 ++++++++++++++++++++++------------- src/ne_request.h | 2 +- test/request.c | 9 ++++----- 3 files changed, 27 insertions(+), 19 deletions(-) diff --git a/src/ne_request.c b/src/ne_request.c index 781d77f0..e99bd8e2 100644 --- a/src/ne_request.c +++ b/src/ne_request.c @@ -160,6 +160,7 @@ struct ne_request_s { struct interim_handler *interim_handler; /*** Miscellaneous ***/ + ne_uri *target_uri; unsigned int method_is_head; unsigned int can_persist; @@ -582,16 +583,21 @@ static int get_request_target_uri(ne_request *req, ne_uri *uri) } } -ne_uri *ne_get_request_target(ne_request *req) +const ne_uri *ne_get_request_target(ne_request *req) { - ne_uri *rv = ne_calloc(sizeof *rv); + if (req->target_uri == NULL) { + ne_uri *uri = ne_calloc(sizeof *uri); - if (get_request_target_uri(req, rv)) { - ne_uri_free(rv); - return NULL; + if (get_request_target_uri(req, uri) == 0) { + req->target_uri = uri; + } + else { + ne_uri_free(uri); + ne_free(uri); + } } - return rv; + return req->target_uri; } /* Set the request body length to 'length' */ @@ -762,13 +768,13 @@ static void free_response_headers(ne_request *req) ne_uri *ne_get_response_location(ne_request *req, const char *fragment) { const char *location; - ne_uri dest, base, *ret = NULL; + ne_uri dest, *ret = NULL; + const ne_uri *base; location = get_response_header_hv(req, HH_HV_LOCATION, "location"); if (location == NULL) return NULL; - memset(&base, 0, sizeof base); memset(&dest, 0, sizeof dest); /* Location is a URI-reference (RFC9110ẞ10.2.2) relative to the @@ -781,14 +787,14 @@ ne_uri *ne_get_response_location(ne_request *req, const char *fragment) goto fail; } - if (get_request_target_uri(req, &base)) { - ne_set_error(req->session, _("Could not parse redirect " - "destination URL")); + if ((base = ne_get_request_target(req)) == NULL) { + ne_set_error(req->session, _("Could not parse request " + "target URI")); goto fail; } ret = ne_malloc(sizeof *ret); - ne_uri_resolve(&base, &dest, ret); + ne_uri_resolve(base, &dest, ret); /* HTTP-specific fragment handling is a MUST in RFC9110ẞ10.2.2: */ if (fragment && !dest.fragment) { @@ -796,7 +802,6 @@ ne_uri *ne_get_response_location(ne_request *req, const char *fragment) } fail: - ne_uri_free(&base); ne_uri_free(&dest); return ret; @@ -832,6 +837,10 @@ void ne_request_destroy(ne_request *req) ne_free(req->target); ne_free(req->method); + if (req->target_uri) { + ne_uri_free(req->target_uri); + ne_free(req->target_uri); + } for (rdr = req->body_readers; rdr != NULL; rdr = next_rdr) { next_rdr = rdr->next; diff --git a/src/ne_request.h b/src/ne_request.h index f25a545e..f2a08c68 100644 --- a/src/ne_request.h +++ b/src/ne_request.h @@ -169,7 +169,7 @@ void ne_print_request_header(ne_request *req, const char *name, /* Returns the request target URI as a malloc-allocated ne_uri object, * or NULL on error if the request target cannot be determined. The * session error string is not changed on error. */ -ne_uri *ne_get_request_target(ne_request *req); +const ne_uri *ne_get_request_target(ne_request *req); /* If the response includes a Location header, this function parses * and resolves the URI-reference relative to the request target. If diff --git a/test/request.c b/test/request.c index 8d42c033..ad4ccbe7 100644 --- a/test/request.c +++ b/test/request.c @@ -2455,23 +2455,22 @@ static int targets(void) unsigned n; for (n = 0; ts[n].scheme != NULL; n++ ) { + const ne_uri *uri, *uri2; ne_session *sess; ne_request *req; - ne_uri *uri; char *actual; sess = ne_session_create(ts[n].scheme, ts[n].host, ts[n].port); req = ne_request_create(sess, ts[n].method, ts[n].target); uri = ne_get_request_target(req); + uri2 = ne_get_request_target(req); actual = uri ? ne_uri_unparse(uri) : NULL; ONCMP(ts[n].expected, actual, "request target", "URI"); + ONN("caching failed, different rv on second call", uri != uri2); + if (actual) ne_free(actual); - if (uri) { - ne_uri_free(uri); - ne_free(uri); - } ne_request_destroy(req); ne_session_destroy(sess);