ComputerSurge/bind9
3
0
Fork 0
Code Pull Requests 1 Activity

Merge branch '4126-statschannel-connection-close-keepalive' into 'main'

Browse Source 
Improve statschannel HTTP Connection: header logic

See merge request isc-projects/bind9!8014
This commit is contained in:
Tony Finch
2023-06-15 16:03:51 +00:00
parent a0efcf0e58 e18ca83a3b
commit 5f9790feca
3 changed files with 99 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
CHANGES
View File

@@ -1,3 +1,6 @@
6199. [bug] Improve HTTP Connection: header protocol conformance
in the statistics channel. [GL #4126]
6198. [func] Remove the holes in the isc_result_t enum to compact
the isc_result tables. [GL #4149]

61
bin/tests/system/statschannel/tests.sh
View File

@@ -392,6 +392,11 @@ Connection: close
EOF
lines=$(grep -c "^<statistics version" nc.out$n)
test "$lines" = 2 || ret=1
# keep-alive not needed in HTTP/1.1, second response has close
lines=$(grep -c "^Connection: Keep-Alive" nc.out$n)
test "$lines" = 0 || ret=1
lines=$(grep -c "^Connection: close" nc.out$n)
test "$lines" = 1 || ret=1
else
echo_i "skipping test as nc not found"
fi
@@ -419,6 +424,62 @@ Connection: close
EOF
lines=$(grep -c "^<statistics version" nc.out$n)
test "$lines" = 2 || ret=1
# keep-alive not needed in HTTP/1.1, second response has close
lines=$(grep -c "^Connection: Keep-Alive" nc.out$n)
test "$lines" = 0 || ret=1
lines=$(grep -c "^Connection: close" nc.out$n)
test "$lines" = 1 || ret=1
else
echo_i "skipping test as nc not found"
fi
if [ $ret != 0 ]; then echo_i "failed"; fi
status=$((status + ret))
n=$((n + 1))
echo_i "Check HTTP/1.0 keep-alive ($n)"
ret=0
if [ -x "${NC}" ]; then
"${NC}" 10.53.0.3 "${EXTRAPORT1}" << EOF > nc.out$n || ret=1
GET /xml/v3/status HTTP/1.0
Connection: keep-alive
GET /xml/v3/status HTTP/1.0
EOF
# should be two responses
lines=$(grep -c "^<statistics version" nc.out$n)
test "$lines" = 2 || ret=1
# first response has keep-alive, second has close
lines=$(grep -c "^Connection: Keep-Alive" nc.out$n)
test "$lines" = 1 || ret=1
lines=$(grep -c "^Connection: close" nc.out$n)
test "$lines" = 1 || ret=1
else
echo_i "skipping test as nc not found"
fi
if [ $ret != 0 ]; then echo_i "failed"; fi
status=$((status + ret))
n=$((n + 1))
echo_i "Check inconsistent Connection: headers ($n)"
ret=0
if [ -x "${NC}" ]; then
"${NC}" 10.53.0.3 "${EXTRAPORT1}" << EOF > nc.out$n || ret=1
GET /xml/v3/status HTTP/1.0
Connection: keep-alive
Connection: close
GET /xml/v3/status HTTP/1.0
EOF
# should be one response (second is ignored)
lines=$(grep -c "^<statistics version" nc.out$n)
test "$lines" = 1 || ret=1
# no keep-alive, one close
lines=$(grep -c "^Connection: Keep-Alive" nc.out$n)
test "$lines" = 0 || ret=1
lines=$(grep -c "^Connection: close" nc.out$n)
test "$lines" = 1 || ret=1
else
echo_i "skipping test as nc not found"
fi

54
lib/isc/httpd.c
View File

@@ -54,10 +54,11 @@
#define HTTP_HEADERS_NUM 100
#define HTTP_MAX_REQUEST_LEN 4096
#define HTTPD_CLOSE 0x0001 /* Got a Connection: close header */
#define HTTPD_FOUNDHOST 0x0002 /* Got a Host: header */
#define HTTPD_KEEPALIVE 0x0004 /* Got a Connection: Keep-Alive */
#define HTTPD_ACCEPT_DEFLATE 0x0008
typedef enum httpd_flags {
CONNECTION_CLOSE = 1 << 0, /* connection must close */
CONNECTION_KEEP_ALIVE = 1 << 1, /* response needs a keep-alive header */
ACCEPT_DEFLATE = 1 << 2, /* response can be compressed */
} httpd_flags_t;
#define HTTPD_MAGIC ISC_MAGIC('H', 't', 'p', 'd')
#define VALID_HTTPD(m) ISC_MAGIC_VALID(m, HTTPD_MAGIC)
@@ -95,8 +96,6 @@ struct isc_httpd {
isc_nmhandle_t *handle; /* Permanent pointer to handle */
int flags;
/*%
* Received data state.
*/
@@ -106,6 +105,7 @@ struct isc_httpd {
method_t method;
int minor_version;
httpd_flags_t flags;
const char *path;
isc_url_parser_t up;
isc_time_t if_modified_since;
@@ -415,8 +415,14 @@ process_request(isc_httpd_t *httpd, size_t last_len) {
}
httpd->path = path;
/*
* Examine headers that can affect this request's response
*/
httpd->flags = 0;
ssize_t content_len = 0;
bool keep_alive = false;
bool host_header = false;
isc_time_set(&httpd->if_modified_since, 0, 0);
@@ -432,16 +438,18 @@ process_request(isc_httpd_t *httpd, size_t last_len) {
return (ISC_R_RANGE);
}
} else if (name_match(header, "Connection")) {
/* multiple fields in a connection header are allowed */
if (value_match(header, "close")) {
httpd->flags |= HTTPD_CLOSE;
} else if (value_match(header, "keep-alive")) {
httpd->flags |= CONNECTION_CLOSE;
}
if (value_match(header, "keep-alive")) {
keep_alive = true;
}
} else if (name_match(header, "Host")) {
httpd->flags |= HTTPD_FOUNDHOST;
host_header = true;
} else if (name_match(header, "Accept-Encoding")) {
if (value_match(header, "deflate")) {
httpd->flags |= HTTPD_ACCEPT_DEFLATE;
httpd->flags |= ACCEPT_DEFLATE;
}
} else if (name_match(header, "If-Modified-Since")) {
char timestamp[ISC_FORMATHTTPTIMESTAMP_SIZE + 1];
@@ -476,14 +484,18 @@ process_request(isc_httpd_t *httpd, size_t last_len) {
switch (httpd->minor_version) {
case 0:
if (keep_alive == true) {
httpd->flags |= HTTPD_KEEPALIVE;
/*
* RFC 9112 section 9.3 says close takes priority if
* keep-alive is also present
*/
if ((httpd->flags & CONNECTION_CLOSE) == 0 && keep_alive) {
httpd->flags |= CONNECTION_KEEP_ALIVE;
} else {
httpd->flags |= HTTPD_CLOSE;
httpd->flags |= CONNECTION_CLOSE;
}
break;
case 1:
if ((httpd->flags & HTTPD_FOUNDHOST) == 0) {
if (!host_header) {
return (ISC_R_RANGE);
}
break;
@@ -773,7 +785,7 @@ prepare_response(isc_httpdmgr_t *mgr, isc_httpd_t *httpd,
}
#ifdef HAVE_ZLIB
if ((httpd->flags & HTTPD_ACCEPT_DEFLATE) != 0) {
if ((httpd->flags & ACCEPT_DEFLATE) != 0) {
result = httpd_compress(req);
if (result == ISC_R_SUCCESS) {
is_compressed = true;
@@ -782,7 +794,10 @@ prepare_response(isc_httpdmgr_t *mgr, isc_httpd_t *httpd,
#endif /* ifdef HAVE_ZLIB */
httpd_response(httpd, req);
if ((httpd->flags & HTTPD_KEEPALIVE) != 0) {
/* RFC 9112 § 9.6: SHOULD send Connection: close in last response */
if ((httpd->flags & CONNECTION_CLOSE) != 0) {
httpd_addheader(req, "Connection", "close");
} else if ((httpd->flags & CONNECTION_KEEP_ALIVE) != 0) {
httpd_addheader(req, "Connection", "Keep-Alive");
}
httpd_addheader(req, "Content-Type", req->mimetype);
@@ -905,6 +920,7 @@ httpd_request(isc_nmhandle_t *handle, isc_result_t eresult,
return;
}
/* XXXFANF it would be more polite to reply 400 bad request */
if (result != ISC_R_SUCCESS) {
goto close_readhandle;
}
@@ -1012,13 +1028,13 @@ httpd_senddone(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) {
goto detach;
}
if (eresult == ISC_R_SUCCESS && (httpd->flags & HTTPD_CLOSE) != 0) {
if (eresult == ISC_R_SUCCESS && (httpd->flags & CONNECTION_CLOSE) != 0)
{
eresult = ISC_R_EOF;
}
/*
* Calling httpd_request() with region NULL restarts
* reading.
* Calling httpd_request() with region NULL restarts reading.
*/
httpd_request(handle, eresult, NULL, httpd);