possible httpd applet bug?

Paul Albrecht albrecht at rdi1.com
Wed Jan 3 14:45:32 UTC 2007


On Wed, 2007-01-03 at 04:30 +0100, Denis Vlasenko wrote: 
> On Tuesday 02 January 2007 16:15, Paul Albrecht wrote:
> > 
> > I have a question about the way the busybox httpd applet reads cgi
> > program output: Why is safe_read used rather than full_read?
> > 
> > The problem with safe_read is you're not guaranteed the entire first
> > line of cgi output on the first read so the applet doesn't handle the
> > status line or content-type header correctly.
> > 
> > Seems like if you don't want to read all the cgi output at once, then
> > you should buffer the first line, but I don't think it makes much of a
> > difference for busybox httpd.
> 
> This code?
> 
>                 if (FD_ISSET(inFd, &readSet)) {
>                         int s = config->accepted_socket;
>                         char *rbuf = config->buf;
> 
> #ifndef PIPE_BUF
> # define PIPESIZE 4096          /* amount of buffering in a pipe */
> #else
> # define PIPESIZE PIPE_BUF
> #endif
> #if PIPESIZE >= MAX_MEMORY_BUFF
> # error "PIPESIZE >= MAX_MEMORY_BUFF"
> #endif
> 
>                         /* There is something to read */
>                         count = safe_read(inFd, rbuf, PIPESIZE);
> 

Yes, this is a problem because safe_read is unbuffered so you can't
assume you have the first line of output after the initial read, but
here's what the server does next:

if (count == 0)
break;  /* closed */ 
if (count > 0) {
	if (firstLine) {
		rbuf[count] = 0;
		/* check to see if the user script added headers */
		if (strncmp(rbuf, "HTTP/1.0 200 OK\r\n", 4) != 0) {
				full_write(s, "HTTP/1.0 200 OK\r\n", 17);
		}
		/* Sometimes CGI is writing to pipe in small chunks
		 * and we don't see Content-type (because the read
		 * is too short) and we emit bogus "text/plain"!
		 * Is it a bug or CGI *has to* write it in one piece? */
		if (strstr(rbuf, "ontent-") == 0) {
			full_write(s, "Content-type: text/plain\r\n\r\n", 28);
		}
		firstLine = 0;
	}
	if (full_write(s, rbuf, count) != count)
		break;

	if (DEBUG)
		fprintf(stderr, "cgi read %d bytes: '%.*s'\n", count, count, rbuf);
}

The problem with the preceding code is it assumes the initial safe_read has read at
least four bytes and that's not guaranteed because safe_read is unbuffered.

I have worked around the problem by changing the safe_read to full_read so I get all
the cgi output on the first read.

If you change the safe_read to a full_read then I don't think you need to write the 
content-type header, but I haven't tested that yet.

> What will happen if there is less than PIPESIZE bytes? Hmm...
> if child has died, we will get short read anyway;
> if it has paused, we will block on this read... is it bad? why?
> 
> (btw, I read around that place. What kind of English is this
> variable name - post_readed_xxx? I think it's vodz contribution...)
> 
> Need to think about it a bit more.
> 
> Suggestions in the form of patches are always welcome. ;)
> --
> 
> vda



More information about the busybox mailing list