Must really be safe_read(), not full_read()? (was: [PATCH] fix httpd lockup in cgi POSTs)

[Matthias Reichl]

On Mon, Feb 12, 2007 at 10:01:13PM +0100, Denis Vlasenko wrote:
> On Monday 12 February 2007 18:09, Matthias Reichl wrote:
> > if (firstline)
> >     count = full_read(inFd, rbuf, 4);
> >     /* read 4 bytes so we can check if the line begins with "HTTP" */
> 
> So far I am happy with "bbox httpd doesn't support insane cgis
> which split their HTTP response" way.

Would be OK for me, too. OTOH: it's a simple patch and it assures
that the current httpd could also deal with insane cgis.

> > Better yet, do a full line read for the first line or completely
> > switch to line buffered input, as you suggested.
> 
> Are you suggesting using stdio?
4> Can't do that, or POSTDATA will break again.
> 
> You basically need to _open-code_ buffering here. Than will work.

No, I was thinking about using a function like getLine() when
reading the first line from the cgi. But that's not needed ATM
and would only be useful for future versions of httpd if they'd like
to parse the full first line.

To clean up the patch a little bit more, I'd suggest something like
this:

static const char HTTP_200[] = "HTTP/1.0 200 OK\r\n\r\n";
/* httpd only checks for a response beginning with "HTTP" */
/* in the first line of cgi output */
#define HTTP_200_check_len 4
if (firstline)
	count = full_read(inFd, rbuf, HTTP_200_check_len);
} else {
	count = safe_read(inFd, rbuf, PIPESIZE);
}
...
	if (memcmp(rbuf, HTTP_200, HTTP_200_check_len) != 0) {
...
#undef HTTP_200_check_len

AFAICT this should work fine and it's clear for other developers.

The only thing that may go wrong is a cgi that outputs only
something like "hi\n" and then waits for POST data. Also insane,
and can only be handled with a getLine()-like approach.

so long,

Hias