On 2022-03-01 10:22 -08, j...@bitminer.ca wrote:
> Looking at the gz option, I noticed some kv structs allocated on
> stack but not fully initialized.
Nice catch.
>
> This patches initializes the kv struct to avoid randomly getting
> KV_GLAG_GLOBBING in kv_find depending on stack contents, whenever
> a kv struct is allocated.
>
> Only kv structs seem to be affected.
The diff is correct, but a bit inconsistent. in server_fcgi.c you set
kv_flags every time kv_key is set, but you could just initialize it at
the beginning of the function. In server_http.c server_log_http() you
depend on kv_flags being initialized once.
TBH, I'd like my bikeshed green, how about we remove kv_flags, it's
unused and alreayd diverged from relayd(8).
OK?
If people want to keep the functionality, OK florian for John's diff.
diff --git httpd.c httpd.c
index 99687a18939..86d5ea9f96f 100644
--- httpd.c
+++ httpd.c
@@ -1063,22 +1063,7 @@ kv_free(struct kv *kv)
struct kv *
kv_find(struct kvtree *keys, struct kv *kv)
{
- struct kv *match;
- const char *key;
-
- if (kv->kv_flags & KV_FLAG_GLOBBING) {
- /* Test header key using shell globbing rules */
- key = kv->kv_key == NULL ? "" : kv->kv_key;
- RB_FOREACH(match, kvtree, keys) {
- if (fnmatch(key, match->kv_key, FNM_CASEFOLD) == 0)
- break;
- }
- } else {
- /* Fast tree-based lookup only works without globbing */
- match = RB_FIND(kvtree, keys, kv);
- }
-
- return (match);
+ return (RB_FIND(kvtree, keys, kv));
}
int
diff --git httpd.h httpd.h
index 692c5611bb5..1b37d87c6e6 100644
--- httpd.h
+++ httpd.h
@@ -131,10 +131,6 @@ struct kv {
char *kv_key;
char *kv_value;
-#define KV_FLAG_INVALID 0x01
-#define KV_FLAG_GLOBBING 0x02
- uint8_t kv_flags;
-
struct kvlist kv_children;
struct kv *kv_parent;
TAILQ_ENTRY(kv) kv_entry;
diff --git server_fcgi.c server_fcgi.c
index 6542b1f1739..810b217ebe0 100644
--- server_fcgi.c
+++ server_fcgi.c
@@ -702,9 +702,6 @@ server_fcgi_writeheader(struct client *clt, struct kv *hdr,
void *arg)
const char *key;
int ret;
- if (hdr->kv_flags & KV_FLAG_INVALID)
- return (0);
-
/* The key might have been updated in the parent */
if (hdr->kv_parent != NULL && hdr->kv_parent->kv_key != NULL)
key = hdr->kv_parent->kv_key;
diff --git server_http.c server_http.c
index d5d31fa03ef..40e202665b5 100644
--- server_http.c
+++ server_http.c
@@ -1647,9 +1647,6 @@ server_writeheader_http(struct client *clt, struct kv
*hdr, void *arg)
char *ptr;
const char *key;
- if (hdr->kv_flags & KV_FLAG_INVALID)
- return (0);
-
/* The key might have been updated in the parent */
if (hdr->kv_parent != NULL && hdr->kv_parent->kv_key != NULL)
key = hdr->kv_parent->kv_key;
>
>
>
> John
>
> Index: server_fcgi.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
> retrieving revision 1.89
> diff -u -p -u -p -r1.89 server_fcgi.c
> --- server_fcgi.c 23 Oct 2021 15:52:44 -0000 1.89
> +++ server_fcgi.c 1 Mar 2022 15:35:41 -0000
> @@ -629,6 +629,7 @@ server_fcgi_header(struct client *clt, u
>
> /* But then we need a Content-Length unless method is HEAD... */
> if (desc->http_method != HTTP_METHOD_HEAD) {
> + key.kv_flags = 0;
> key.kv_key = "Content-Length";
> if ((kv = kv_find(&resp->http_headers, &key)) == NULL) {
> if (kv_add(&resp->http_headers,
> @@ -641,6 +642,7 @@ server_fcgi_header(struct client *clt, u
> /* Send chunked encoding header */
> if (clt->clt_fcgi.chunked) {
> /* but only if no Content-Length header is supplied */
> + key.kv_flags = 0;
> key.kv_key = "Content-Length";
> if ((kv = kv_find(&resp->http_headers, &key)) != NULL) {
> clt->clt_fcgi.chunked = 0;
> @@ -679,6 +681,7 @@ server_fcgi_header(struct client *clt, u
> }
>
> /* Date header is mandatory and should be added as late as possible */
> + key.kv_flags = 0;
> key.kv_key = "Date";
> if (kv_find(&resp->http_headers, &key) == NULL &&
> (server_http_time(time(NULL), tmbuf, sizeof(tmbuf)) <= 0 ||
> Index: server_file.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
> retrieving revision 1.71
> diff -u -p -u -p -r1.71 server_file.c
> --- server_file.c 27 Feb 2022 20:30:30 -0000 1.71
> +++ server_file.c 1 Mar 2022 15:35:42 -0000
> @@ -136,6 +136,7 @@ server_file_access(struct httpd *env, st
> goto fail;
> }
>
> + key.kv_flags = 0;
> key.kv_key = "Range";
> r = kv_find(&desc->http_headers, &key);
> if (r != NULL)
> @@ -248,6 +249,7 @@ server_file_request(struct httpd *env, s
> struct kv *r, key;
>
> /* check Accept-Encoding header */
> + key.kv_flags = 0;
> key.kv_key = "Accept-Encoding";
> r = kv_find(&req->http_headers, &key);
>
> @@ -691,6 +693,7 @@ server_file_modified_since(struct http_d
> struct kv key, *since;
> struct tm tm;
>
> + key.kv_flags = 0;
> key.kv_key = "If-Modified-Since";
> if ((since = kv_find(&desc->http_headers, &key)) != NULL &&
> since->kv_value != NULL) {
> Index: server_http.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
> retrieving revision 1.149
> diff -u -p -u -p -r1.149 server_http.c
> --- server_http.c 11 Nov 2021 15:52:33 -0000 1.149
> +++ server_http.c 1 Mar 2022 15:35:42 -0000
> @@ -140,6 +140,7 @@ server_http_authenticate(struct server_c
> char *clt_user = NULL, *clt_pass = NULL;
>
> memset(decoded, 0, sizeof(decoded));
> + key.kv_flags = 0;
> key.kv_key = "Authorization";
>
> if ((ba = kv_find(&desc->http_headers, &key)) == NULL ||
> @@ -1292,6 +1293,7 @@ server_response(struct httpd *httpd, str
> if ((desc->http_path = strdup(path)) == NULL)
> goto fail;
>
> + key.kv_flags = 0;
> key.kv_key = "Host";
> if ((host = kv_find(&desc->http_headers, &key)) != NULL &&
> host->kv_value == NULL)
> @@ -1891,6 +1893,7 @@ server_log_http(struct client *clt, unsi
>
> case LOG_FORMAT_COMBINED:
> case LOG_FORMAT_FORWARDED:
> + key.kv_flags = 0;
> key.kv_key = "Referer"; /* sic */
> if ((referrer = kv_find(&desc->http_headers, &key)) != NULL &&
> referrer->kv_value == NULL)
>
--
I'm not entirely sure you are real.
