diff --git a/src/httpnenv.c b/src/httpnenv.c index 274db05..aaf560e 100644 --- a/src/httpnenv.c +++ b/src/httpnenv.c @@ -14,9 +14,17 @@ httpnenv(const UCHAR *name, const UCHAR *value) size_t total = offsetof(HTTPV, name) + namelen + 1 + vallen + 1; HTTPV *v; - if (namelen + vallen > 8192) return NULL; /* sanity limit */ + /* Both rejects return NULL, so set errno to tell the caller which one it + was: httpsenv() reports an oversized variable differently from a storage + shortage, and since libc370#82 the shortage is a real outcome rather + than an S878 (issue #162). */ + if (namelen + vallen > 8192) { /* sanity limit */ + errno = E2BIG; + return NULL; + } v = calloc(1, total); + if (!v) errno = ENOMEM; if (v) { strcpy(v->eye, HTTPV_EYE); diff --git a/src/httppc.c b/src/httppc.c index 5fb5157..d457214 100644 --- a/src/httppc.c +++ b/src/httppc.c @@ -104,7 +104,14 @@ httppc(HTTPC *httpc) char scriptfile[384]; snprintf(scriptfile, sizeof(scriptfile), "%s%s", httpd->docroot, path); - http_set_env(httpc, "SCRIPT_FILENAME", scriptfile); + /* every other http_set_env() caller checks; dispatching the + CGI without SCRIPT_FILENAME would run it against no file at + all, so fail the request instead of guessing (#162) */ + if (http_set_env(httpc, "SCRIPT_FILENAME", scriptfile)) { + http_resp_internal_error(httpc); + httpc->state = CSTATE_DONE; + goto check_done; + } } /* path needs to be processed by external program */ diff --git a/src/httpsenv.c b/src/httpsenv.c index 8336ee2..fa70a92 100644 --- a/src/httpsenv.c +++ b/src/httpsenv.c @@ -12,6 +12,19 @@ httpsenv(HTTPC *httpc, const UCHAR *name, const UCHAR *value) /* allocate a new environment variable */ v = http_new_env(name, value); if (!v) { + /* Say so. Before libc370#82 a storage shortage here abended S878 and + try(serve_client) logged HTTPD062E; now it is a quiet -1 that every + caller turns into CSTATE_RESET, i.e. a connection reset with no + response and no log line -- indistinguishable from a malformed + request (issue #162). This is the one choke point every env-var + caller passes through, and it still knows which variable failed, + which the reset sites no longer do. */ + if (errno == E2BIG) + wtof("HTTPD905E Environment variable %.32s too large " + "client(%08X)", name, httpc); + else + wtof("HTTPD904E No storage for environment variable %.32s " + "client(%08X)", name, httpc); rc = -1; goto quit; } @@ -26,6 +39,8 @@ httpsenv(HTTPC *httpc, const UCHAR *name, const UCHAR *value) the new HTTPV would be an unreachable orphan -- free it and report. */ if (array_add(&httpc->env, v)) { free(v); + wtof("HTTPD904E No storage for environment variable %.32s " + "client(%08X)", name, httpc); rc = -1; } diff --git a/src/httpshen.c b/src/httpshen.c index a9f9e25..7ea1b56 100644 --- a/src/httpshen.c +++ b/src/httpshen.c @@ -28,10 +28,11 @@ parse_cookies(HTTPC *httpc, const UCHAR *in) UCHAR *value; if (!buf) { - wtof("%s: out of memory", __func__); - return ENOMEM; + wtof("HTTPD904E No storage for environment variable HTTP_Cookie " + "client(%08X)", httpc); + return -1; /* -1, like every other setter in this path */ } - + for (name = buf; name && *name; ) { /* skip leading delimiters */ while (*name == ';' || *name == ' ') name++; @@ -49,10 +50,16 @@ parse_cookies(HTTPC *httpc, const UCHAR *in) else { value = ""; } - set_cookie(httpc, name, value); + /* stop at the first failure: the caller resets the connection + anyway, and carrying on would emit one HTTPD904E per remaining + cookie instead of one per request */ + if (set_cookie(httpc, name, value)) { + free(buf); + return -1; + } name = end; } - + free(buf); return 0; } diff --git a/test/tstnenv.c b/test/tstnenv.c index 5940c15..22ba2fb 100644 --- a/test/tstnenv.c +++ b/test/tstnenv.c @@ -52,5 +52,37 @@ int main(void) free(v); } + /* Both rejects return NULL, and httpsenv() tells them apart by errno to + decide whether to report a storage shortage (HTTPD904E) or an oversized + variable (HTTPD905E) -- issue #162. The sanity limit is the half that + needs no storage shortage to reach, so it is the half a test can pin. */ + { + UCHAR *big = malloc(9000); + + CHECK(big != NULL, "oversize probe buffer allocated"); + if (big) { + memset(big, 'X', 8999); + big[8999] = 0; + + errno = 0; + v = http_new_env((const UCHAR *)"BIG", big); + CHECK(v == NULL, "name+value over the sanity limit is rejected"); + CHECK(errno == E2BIG, "oversize reject reports E2BIG, not ENOMEM"); + if (v) free(v); + + /* just under the limit still succeeds -- guards the boundary + against the reject swallowing legitimate variables */ + big[8000] = 0; + v = http_new_env((const UCHAR *)"BIG", big); + CHECK(v != NULL, "name+value under the sanity limit is accepted"); + if (v) { + CHECK(v->value && strlen(v->value) == 8000, + "under-limit value stored at full length"); + free(v); + } + free(big); + } + } + return mbt_test_summary("TSTNENV"); }