Re: httpd: don't send error body with HEAD method
Bertrand Janin wrote :
Philip Guenther wrote :
On Mon, Nov 24, 2014 at 11:24 AM, Florian Obser flor...@openbsd.org wrote:
...
Since we are probably not supposed to send a Content-Type header I
think it makes sense to duplicate the httpmsg generating code in this
case;
If a GET of that resource would have a Content-Type, then the HEAD of
it should have one. Per RFC 2616, HEAD and GET should return the same
header fields and only differ by HEAD leaving out the body.
Here is an updated patch which fix the leak identified by Florian, ensures the
headers are identical with HEAD and non-HEAD methods and adds Content-Length.
Replaced strlen() with the output from asprintf(), any comments?
Index: usr.sbin/httpd/server_http.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
retrieving revision 1.54
diff -u -p -r1.54 server_http.c
--- usr.sbin/httpd/server_http.c25 Oct 2014 03:23:49 - 1.54
+++ usr.sbin/httpd/server_http.c26 Nov 2014 03:05:48 -
@@ -665,10 +665,11 @@ server_abort_http(struct client *clt, u_
struct server *srv = clt-clt_srv;
struct server_config*srv_conf = srv-srv_conf;
struct bufferevent *bev = clt-clt_bev;
- const char *httperr = NULL, *text = ;
- char*httpmsg, *extraheader = NULL;
+ struct http_descriptor *desc = clt-clt_descreq;
+ const char *httperr = NULL, *style;
+ char*httpmsg, *body = NULL, *extraheader = NULL;
char tmbuf[32], hbuf[128];
- const char *style;
+ int bodylen;
if ((httperr = server_httperror_byid(code)) == NULL)
httperr = Unknown Error;
@@ -710,15 +711,9 @@ server_abort_http(struct client *clt, u_
style = body { background-color: white; color: black; font-family:
'Comic Sans MS', 'Chalkboard SE', 'Comic Neue', sans-serif; }\n
hr { border: 0; border-bottom: 1px dashed; }\n;
- /* Generate simple HTTP+HTML error document */
- if (asprintf(httpmsg,
- HTTP/1.0 %03d %s\r\n
- Date: %s\r\n
- Server: %s\r\n
- Connection: close\r\n
- Content-Type: text/html\r\n
- %s
- \r\n
+
+ /* Generate simple HTML error document */
+ if ((bodylen = asprintf(body,
!DOCTYPE HTML PUBLIC
\-//W3C//DTD HTML 4.01 Transitional//EN\\n
html\n
@@ -728,14 +723,26 @@ server_abort_http(struct client *clt, u_
/head\n
body\n
h1%03d %s/h1\n
- div id='m'%s/div\n
hr\naddress%s/address\n
/body\n
/html\n,
- code, httperr, tmbuf, HTTPD_SERVERNAME,
+ code, httperr, style, code, httperr, HTTPD_SERVERNAME)) == -1)
+ goto done;
+
+ /* Add basic HTTP headers */
+ if (asprintf(httpmsg,
+ HTTP/1.0 %03d %s\r\n
+ Date: %s\r\n
+ Server: %s\r\n
+ Connection: close\r\n
+ Content-Type: text/html\r\n
+ Content-Length: %d\r\n
+ %s
+ \r\n
+ %s,
+ code, httperr, tmbuf, HTTPD_SERVERNAME, bodylen,
extraheader == NULL ? : extraheader,
- code, httperr, style, code, httperr, text,
- HTTPD_SERVERNAME) == -1)
+ desc-http_method == HTTP_METHOD_HEAD ? : body) == -1)
goto done;
/* Dump the message without checking for success */
@@ -743,6 +750,7 @@ server_abort_http(struct client *clt, u_
free(httpmsg);
done:
+ free(body);
free(extraheader);
if (asprintf(httpmsg, %s (%03d %s), msg, code, httperr) == -1) {
server_close(clt, msg);
Re: httpd: don't send error body with HEAD method
Florian Obser wrote : On Sun, Nov 23, 2014 at 08:15:47PM -0500, Bertrand Janin wrote: Hi, This patch updates server_abort_http() to only send the body of default http error if the method is not HEAD. I first noticed that with curl -v -I which complains about the excess data: * Excess found in a non pipelined read: excess = 397 url = /asd (zero-length body) Nice catch. However, your diff will leak body if the second asprintf(3) fails. Since we are probably not supposed to send a Content-Type header I think it makes sense to duplicate the httpmsg generating code in this case; it feels easier. While in there nuke unneeded text variable and shuffle variable declarations around. Indeed, that's much better. -b
Re: httpd: don't send error body with HEAD method
On Mon, Nov 24, 2014 at 11:24 AM, Florian Obser flor...@openbsd.org wrote: ... Since we are probably not supposed to send a Content-Type header I think it makes sense to duplicate the httpmsg generating code in this case; If a GET of that resource would have a Content-Type, then the HEAD of it should have one. Per RFC 2616, HEAD and GET should return the same header fields and only differ by HEAD leaving out the body. Philip
httpd: don't send error body with HEAD method
Hi,
This patch updates server_abort_http() to only send the body of default http
error if the method is not HEAD. I first noticed that with curl -v -I which
complains about the excess data:
* Excess found in a non pipelined read: excess = 397 url = /asd
(zero-length body)
Index: usr.sbin/httpd/server_http.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
retrieving revision 1.54
diff -u -p -r1.54 server_http.c
--- usr.sbin/httpd/server_http.c25 Oct 2014 03:23:49 - 1.54
+++ usr.sbin/httpd/server_http.c23 Nov 2014 23:49:44 -
@@ -665,8 +665,9 @@ server_abort_http(struct client *clt, u_
struct server *srv = clt-clt_srv;
struct server_config*srv_conf = srv-srv_conf;
struct bufferevent *bev = clt-clt_bev;
+ struct http_descriptor *desc = clt-clt_descreq;
const char *httperr = NULL, *text = ;
- char*httpmsg, *extraheader = NULL;
+ char*httpmsg, *body = NULL, *extraheader = NULL;
char tmbuf[32], hbuf[128];
const char *style;
@@ -706,10 +707,33 @@ server_abort_http(struct client *clt, u_
break;
}
- /* A CSS stylesheet allows minimal customization by the user */
- style = body { background-color: white; color: black; font-family:
- 'Comic Sans MS', 'Chalkboard SE', 'Comic Neue', sans-serif; }\n
- hr { border: 0; border-bottom: 1px dashed; }\n;
+ /* Generate the body (not needed for HEAD requests) */
+ if (desc-http_method != HTTP_METHOD_HEAD) {
+ /* A CSS stylesheet allows minimal customization by the user */
+ style = body { background-color: white; color: black;
+ font-family: 'Comic Sans MS', 'Chalkboard SE',
+ 'Comic Neue', sans-serif; }\n
+ hr { border: 0; border-bottom: 1px dashed; }\n;
+
+ if (asprintf(body,
+ !DOCTYPE HTML PUBLIC
+ \-//W3C//DTD HTML 4.01 Transitional//EN\\n
+ html\n
+ head\n
+ title%03d %s/title\n
+ style type=\text/css\!--\n%s\n--/style\n
+ /head\n
+ body\n
+ h1%03d %s/h1\n
+ div id='m'%s/div\n
+ hr\naddress%s/address\n
+ /body\n
+ /html\n,
+ code, httperr, style, code, httperr, text,
+ HTTPD_SERVERNAME) == -1)
+ goto done;
+ }
+
/* Generate simple HTTP+HTML error document */
if (asprintf(httpmsg,
HTTP/1.0 %03d %s\r\n
@@ -719,28 +743,16 @@ server_abort_http(struct client *clt, u_
Content-Type: text/html\r\n
%s
\r\n
- !DOCTYPE HTML PUBLIC
- \-//W3C//DTD HTML 4.01 Transitional//EN\\n
- html\n
- head\n
- title%03d %s/title\n
- style type=\text/css\!--\n%s\n--/style\n
- /head\n
- body\n
- h1%03d %s/h1\n
- div id='m'%s/div\n
- hr\naddress%s/address\n
- /body\n
- /html\n,
+ %s,
code, httperr, tmbuf, HTTPD_SERVERNAME,
extraheader == NULL ? : extraheader,
- code, httperr, style, code, httperr, text,
- HTTPD_SERVERNAME) == -1)
+ body == NULL ? : body) == -1)
goto done;
/* Dump the message without checking for success */
server_dump(clt, httpmsg, strlen(httpmsg));
free(httpmsg);
+ free(body);
done:
free(extraheader);
