[hackers] [quark] Improve http_prepare_response()'s error semantics || Laslo Hunhold
commit c0909c70e4767fa47b44b0964cf03e7962e430c3
Author: Laslo Hunhold <dev_AT_frign.de>
AuthorDate: Fri Aug 28 22:48:32 2020 +0200
Commit: Laslo Hunhold <dev_AT_frign.de>
CommitDate: Fri Aug 28 22:52:04 2020 +0200
Improve http_prepare_response()'s error semantics
I don't like the juggling with status-values in serve. It makes
sense for http_recv_header() and http_parse_header(), because we
don't have a response-struct yet that we can "fill". We could pass
it to them, but that would make the usage a bit messy.
However, in http_prepare_response(), we are already entrusted with
a pointer to a response-struct, and just failing here (by returning
an error value) leaves the response-struct in an invalid state. Instead,
we make it a void function and reflect the status using the status field
in the passed response struct.
This way, there is no case where the response struct is in an
invalid state after calling a http_prepare_*()-method.
Signed-off-by: Laslo Hunhold <dev_AT_frign.de>
diff --git a/http.c b/http.c
index 5b0390d..633c2f9 100644
--- a/http.c
+++ b/http.c
_AT_@ -523,11 +523,11 @@ parse_range(const char *str, size_t size, size_t *lower, size_t *upper)
#undef RELPATH
#define RELPATH(x) ((!*(x) || !strcmp(x, "/")) ? "." : ((x) + 1))
-enum status
+void
http_prepare_response(const struct request *req, struct response *res,
const struct server *srv)
{
- enum status returnstatus;
+ enum status s;
struct in6_addr addr;
struct stat st;
struct tm tm = { 0 };
_AT_@ -544,7 +544,8 @@ http_prepare_response(const struct request *req, struct response *res,
/* make a working copy of the URI and normalize it */
memcpy(realuri, req->uri, sizeof(realuri));
if (normabspath(realuri)) {
- return S_BAD_REQUEST;
+ s = S_BAD_REQUEST;
+ goto err;
}
/* match vhost */
_AT_@ -559,13 +560,15 @@ http_prepare_response(const struct request *req, struct response *res,
}
}
if (i == srv->vhost_len) {
- return S_NOT_FOUND;
+ s = S_NOT_FOUND;
+ goto err;
}
/* if we have a vhost prefix, prepend it to the URI */
if (vhost->prefix &&
prepend(realuri, LEN(realuri), vhost->prefix)) {
- return S_REQUEST_TOO_LARGE;
+ s = S_REQUEST_TOO_LARGE;
+ goto err;
}
}
_AT_@ -583,7 +586,8 @@ http_prepare_response(const struct request *req, struct response *res,
/* swap out URI prefix */
memmove(realuri, realuri + len, strlen(realuri) + 1);
if (prepend(realuri, LEN(realuri), srv->map[i].to)) {
- return S_REQUEST_TOO_LARGE;
+ s = S_REQUEST_TOO_LARGE;
+ goto err;
}
break;
}
_AT_@ -591,19 +595,22 @@ http_prepare_response(const struct request *req, struct response *res,
/* normalize URI again, in case we introduced dirt */
if (normabspath(realuri)) {
- return S_BAD_REQUEST;
+ s = S_BAD_REQUEST;
+ goto err;
}
/* stat the relative path derived from the URI */
if (stat(RELPATH(realuri), &st) < 0) {
- return (errno == EACCES) ? S_FORBIDDEN : S_NOT_FOUND;
+ s = (errno == EACCES) ? S_FORBIDDEN : S_NOT_FOUND;
+ goto err;
}
if (S_ISDIR(st.st_mode)) {
/* append '/' to URI if not present */
len = strlen(realuri);
if (len + 1 + 1 > PATH_MAX) {
- return S_REQUEST_TOO_LARGE;
+ s = S_REQUEST_TOO_LARGE;
+ goto err;
}
if (len > 0 && realuri[len - 1] != '/') {
realuri[len] = '/';
_AT_@ -617,7 +624,8 @@ http_prepare_response(const struct request *req, struct response *res,
*/
if (strstr(realuri, "/.") && strncmp(realuri,
"/.well-known/", sizeof("/.well-known/") - 1)) {
- return S_FORBIDDEN;
+ s = S_FORBIDDEN;
+ goto err;
}
/*
_AT_@ -646,7 +654,8 @@ http_prepare_response(const struct request *req, struct response *res,
* honor that later when we fill the "Location"-field */
if ((ipv6host = inet_pton(AF_INET6, targethost,
&addr)) < 0) {
- return S_INTERNAL_SERVER_ERROR;
+ s = S_INTERNAL_SERVER_ERROR;
+ goto err;
}
/* write location to response struct */
_AT_@ -657,18 +666,20 @@ http_prepare_response(const struct request *req, struct response *res,
targethost,
ipv6host ? "]" : "", hasport ? ":" : "",
hasport ? srv->port : "", tmpuri)) {
- return S_REQUEST_TOO_LARGE;
+ s = S_REQUEST_TOO_LARGE;
+ goto err;
}
} else {
/* write relative redirection URI to response struct */
if (esnprintf(res->field[RES_LOCATION],
sizeof(res->field[RES_LOCATION]),
"%s", tmpuri)) {
- return S_REQUEST_TOO_LARGE;
+ s = S_REQUEST_TOO_LARGE;
+ goto err;
}
}
- return 0;
+ return;
} else {
/*
* the URI is well-formed, we can now write the URI into
_AT_@ -677,11 +688,13 @@ http_prepare_response(const struct request *req, struct response *res,
* into the actual response-path
*/
if (esnprintf(res->uri, sizeof(res->uri), "%s", req->uri)) {
- return S_REQUEST_TOO_LARGE;
+ s = S_REQUEST_TOO_LARGE;
+ goto err;
}
if (esnprintf(res->path, sizeof(res->path), "%s%s",
vhost ? vhost->dir : "", RELPATH(req->uri))) {
- return S_REQUEST_TOO_LARGE;
+ s = S_REQUEST_TOO_LARGE;
+ goto err;
}
}
_AT_@ -692,7 +705,8 @@ http_prepare_response(const struct request *req, struct response *res,
*/
if (esnprintf(tmpuri, sizeof(tmpuri), "%s%s",
req->uri, srv->docindex)) {
- return S_REQUEST_TOO_LARGE;
+ s = S_REQUEST_TOO_LARGE;
+ goto err;
}
/* stat the docindex, which must be a regular file */
_AT_@ -706,14 +720,16 @@ http_prepare_response(const struct request *req, struct response *res,
if (esnprintf(res->field[RES_CONTENT_TYPE],
sizeof(res->field[RES_CONTENT_TYPE]),
"%s", "text/html; charset=utf-8")) {
- return S_INTERNAL_SERVER_ERROR;
+ s = S_INTERNAL_SERVER_ERROR;
+ goto err;
}
- return 0;
+ return;
} else {
/* reject */
- return (!S_ISREG(st.st_mode) || errno == EACCES) ?
- S_FORBIDDEN : S_NOT_FOUND;
+ s = (!S_ISREG(st.st_mode) || errno == EACCES) ?
+ S_FORBIDDEN : S_NOT_FOUND;
+ goto err;
}
}
}
_AT_@ -723,32 +739,33 @@ http_prepare_response(const struct request *req, struct response *res,
/* parse field */
if (!strptime(req->field[REQ_IF_MODIFIED_SINCE],
"%a, %d %b %Y %T GMT", &tm)) {
- return S_BAD_REQUEST;
+ s = S_BAD_REQUEST;
+ goto err;
}
/* compare with last modification date of the file */
if (difftime(st.st_mtim.tv_sec, timegm(&tm)) <= 0) {
res->status = S_NOT_MODIFIED;
- return 0;
+ return;
}
}
/* range */
- if ((returnstatus = parse_range(req->field[REQ_RANGE], st.st_size,
- &(res->file.lower),
- &(res->file.upper)))) {
- if (returnstatus == S_RANGE_NOT_SATISFIABLE) {
+ if ((s = parse_range(req->field[REQ_RANGE], st.st_size,
+ &(res->file.lower), &(res->file.upper)))) {
+ if (s == S_RANGE_NOT_SATISFIABLE) {
res->status = S_RANGE_NOT_SATISFIABLE;
if (esnprintf(res->field[RES_CONTENT_RANGE],
sizeof(res->field[RES_CONTENT_RANGE]),
"bytes */%zu", st.st_size)) {
- return S_INTERNAL_SERVER_ERROR;
+ s = S_INTERNAL_SERVER_ERROR;
+ goto err;
}
- return 0;
+ return;
} else {
- return returnstatus;
+ goto err;
}
}
_AT_@ -774,34 +791,41 @@ http_prepare_response(const struct request *req, struct response *res,
if (esnprintf(res->field[RES_ACCEPT_RANGES],
sizeof(res->field[RES_ACCEPT_RANGES]),
"%s", "bytes")) {
- return S_INTERNAL_SERVER_ERROR;
+ s = S_INTERNAL_SERVER_ERROR;
+ goto err;
}
if (esnprintf(res->field[RES_CONTENT_LENGTH],
sizeof(res->field[RES_CONTENT_LENGTH]),
"%zu", res->file.upper - res->file.lower + 1)) {
- return S_INTERNAL_SERVER_ERROR;
+ s = S_INTERNAL_SERVER_ERROR;
+ goto err;
}
if (req->field[REQ_RANGE][0] != '\0') {
if (esnprintf(res->field[RES_CONTENT_RANGE],
sizeof(res->field[RES_CONTENT_RANGE]),
"bytes %zd-%zd/%zu", res->file.lower,
res->file.upper, st.st_size)) {
- return S_INTERNAL_SERVER_ERROR;
+ s = S_INTERNAL_SERVER_ERROR;
+ goto err;
}
}
if (esnprintf(res->field[RES_CONTENT_TYPE],
sizeof(res->field[RES_CONTENT_TYPE]),
"%s", mime)) {
- return S_INTERNAL_SERVER_ERROR;
+ s = S_INTERNAL_SERVER_ERROR;
+ goto err;
}
if (timestamp(res->field[RES_LAST_MODIFIED],
sizeof(res->field[RES_LAST_MODIFIED]),
st.st_mtim.tv_sec)) {
- return S_INTERNAL_SERVER_ERROR;
+ s = S_INTERNAL_SERVER_ERROR;
+ goto err;
}
- return 0;
+ return;
+err:
+ http_prepare_error_response(req, res, s);
}
void
diff --git a/http.h b/http.h
index caac765..d75d8d0 100644
--- a/http.h
+++ b/http.h
_AT_@ -103,8 +103,8 @@ enum status http_send_header(int, const struct response *);
enum status http_send_status(int, enum status);
enum status http_recv_header(int, char *, size_t, size_t *);
enum status http_parse_header(const char *, struct request *);
-enum status http_prepare_response(const struct request *, struct response *,
- const struct server *);
+void http_prepare_response(const struct request *, struct response *,
+ const struct server *);
void http_prepare_error_response(const struct request *,
struct response *, enum status);
diff --git a/main.c b/main.c
index fa0fa0f..590e558 100644
--- a/main.c
+++ b/main.c
_AT_@ -39,9 +39,10 @@ serve(int infd, const struct sockaddr_storage *in_sa, const struct server *srv)
/* handle request */
if ((status = http_recv_header(c.fd, c.header, LEN(c.header), &c.off)) ||
- (status = http_parse_header(c.header, &c.req)) ||
- (status = http_prepare_response(&c.req, &c.res, srv))) {
+ (status = http_parse_header(c.header, &c.req))) {
http_prepare_error_response(&c.req, &c.res, status);
+ } else {
+ http_prepare_response(&c.req, &c.res, srv);
}
status = http_send_header(c.fd, &c.res);
Received on Fri Aug 28 2020 - 22:56:33 CEST
This archive was generated by hypermail 2.3.0
: Fri Aug 28 2020 - 23:00:36 CEST