httpd(8): read all headers from fcgi server

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

httpd(8): read all headers from fcgi server

Florian Obser-2
As has been reported on multiple occasions, httpd(8) assumes that the
complete http answer header is in the first fastcgi stdout record:
* https://marc.info/?l=openbsd-tech&m=144571751203238&w=2
* https://github.com/reyk/httpd/issues/63
(I think there were more)

To reproduce:
$ doas cp /bin/{ksh,sleep} /var/www/bin
cat <<END > /var/www/cgi-bin/slow-header.cgi
#!/bin/ksh
print 'Status: 200 OK'
sleep 1
print 'Content-Type: text/plain'
print ''
print 'bad'
END

and enable httpd(8) and slowcgi(8).

A browser will show:

------------------------------------------------------------------------
Content-Type: text/plain

bad
------------------------------------------------------------------------

because there is an extra empty line terminating the header:
------------------------------------------------------------------------
HTTP/1.0 200 OK
Connection: close
Date: Mon, 29 Aug 2016 12:30:44 GMT
Server: OpenBSD httpd

Content-Type: text/plain

bad
------------------------------------------------------------------------

This fixes that by only starting the header output once the whole
header has been read.

OK?

diff --git httpd.h httpd.h
index 93b0d34..97813d2 100644
--- httpd.h
+++ httpd.h
@@ -314,6 +314,8 @@ struct client {
  int clt_fcgi_type;
  int clt_fcgi_chunked;
  int clt_fcgi_end;
+ int clt_fcgi_status;
+ int clt_fcgi_headersdone;
  char *clt_remote_user;
  struct evbuffer *clt_srvevb;
 
diff --git server_fcgi.c server_fcgi.c
index 21ebeed..f831665 100644
--- server_fcgi.c
+++ server_fcgi.c
@@ -143,6 +143,8 @@ server_fcgi(struct httpd *env, struct client *clt)
  memset(hbuf, 0, sizeof(hbuf));
  clt->clt_fcgi_state = FCGI_READ_HEADER;
  clt->clt_fcgi_toread = sizeof(struct fcgi_record_header);
+ clt->clt_fcgi_status = 200;
+ clt->clt_fcgi_headersdone = 0;
 
  if (clt->clt_srvevb != NULL)
  evbuffer_free(clt->clt_srvevb);
@@ -549,13 +551,20 @@ server_fcgi_read(struct bufferevent *bev, void *arg)
  }
  break;
  case FCGI_STDOUT:
- if (++clt->clt_chunk == 1) {
- if (server_fcgi_header(clt,
-    server_fcgi_getheaders(clt))
-    == -1) {
- server_abort_http(clt, 500,
-    "malformed fcgi headers");
- return;
+ ++clt->clt_chunk;
+ if (!clt->clt_fcgi_headersdone) {
+ clt->clt_fcgi_headersdone =
+    server_fcgi_getheaders(clt);
+ if (clt->clt_fcgi_headersdone) {
+ if (server_fcgi_header(clt,
+    clt->clt_fcgi_status)
+    == -1) {
+ server_abort_http(clt,
+    500,
+    "malformed fcgi "
+    "headers");
+ return;
+ }
  }
  if (!EVBUFFER_LENGTH(clt->clt_srvevb))
  break;
@@ -751,7 +760,7 @@ server_fcgi_getheaders(struct client *clt)
 {
  struct http_descriptor *resp = clt->clt_descresp;
  struct evbuffer *evb = clt->clt_srvevb;
- int code = 200;
+ int code;
  char *line, *key, *value;
  const char *errstr;
 
@@ -775,11 +784,12 @@ server_fcgi_getheaders(struct client *clt)
  if (errstr != NULL || server_httperror_byid(
     code) == NULL)
  code = 200;
+ clt->clt_fcgi_status = code;
  } else {
  (void)kv_add(&resp->http_headers, key, value);
  }
  free(line);
  }
 
- return (code);
+ return (line != NULL && *line == '\0');
 }

--
I'm not entirely sure you are real.