You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Tom Tromey <tr...@creche.cygnus.com> on 1996/09/25 23:46:46 UTC

CGI-related core

I'm still using Apache 1.1.1.

This CGI script causes Apache to dump core:

================================================================
#! /bin/sh

# CGI script that returns status 204 and no data.

echo "Status: 204 No Content"

exit 0
================================================================


The problem occurs in the send_error_response function in
http_protocol.c.  The problem is that the function index_of_response
can return -1, but the function response_code_string isn't prepared to
handle that case.

I fixed this by having send_error_response check for the -1 return,
and switch to a "500" error code in this case.  I don't know if this
is the best solution or not.  However, it did eliminate the core dump.

Patch appended.

I've noticed there are a number of defined status codes missing from
the tables in http_protocol.c.  From the HTTP/1.0 spec: 201, 202, 204,
301, 503.  Is there any reason why?  It makes sense for a CGI script
to return some of these codes, and this is something I'd like to check
in my test suite.  If there is no reason, I'll happily generate a
patch.

Last, I've noticed a minor inconsistency in send_error_response.  Most
of the function uses the "status" variable.  But the switch statement
towards the end switches on r->status.  Is there a reason for this
discrepancy (and if so, could someone please add a comment explaining
the reason?).  My patch assumes that this is just an oversight, and
changes r->status to just status.

All the above problems seem to exist in the current snapshot.  I
haven't tried it, though.

Tom
-- 
tromey@cygnus.com                 Member, League for Programming Freedom

Index: http_protocol.c
===================================================================
RCS file: /rel/cvsfiles/devo/apache/src/http_protocol.c,v
retrieving revision 1.4
diff -c -5 -r1.4 http_protocol.c
*** http_protocol.c	1996/09/23 17:20:21	1.4
--- http_protocol.c	1996/09/25 21:40:13
***************
*** 847,856 ****
--- 847,862 ----
      char *custom_response;
      int status = r->status;
      int idx = index_of_response (status);
      char *location = table_get (r->headers_out, "Location");
  
+     /* If status code not found, use code 500.  */
+     if (idx == -1) {
+         status = SERVER_ERROR;
+         idx = index_of_response (SERVER_ERROR);
+     }
+ 
      if (!r->assbackwards) {
  	int i;
  	table *err_hdrs_arr = r->err_headers_out;
  	table_entry *err_hdrs = (table_entry *)err_hdrs_arr->elts;
    
***************
*** 889,899 ****
  	BUFF *fd = c->client;
  	
          bvputs(fd,"<HEAD><TITLE>", title, "</TITLE></HEAD>\n<BODY><H1>", title,
  	       "</H1>\n", NULL);
  	
!         switch (r->status) {
  	case REDIRECT:
  	    bvputs(fd, "The document has moved <A HREF=\"",
  		    escape_html(r->pool, location), "\">here</A>.<P>\n", NULL);
  	    break;
  	case AUTH_REQUIRED:
--- 895,905 ----
  	BUFF *fd = c->client;
  	
          bvputs(fd,"<HEAD><TITLE>", title, "</TITLE></HEAD>\n<BODY><H1>", title,
  	       "</H1>\n", NULL);
  	
!         switch (status) {
  	case REDIRECT:
  	    bvputs(fd, "The document has moved <A HREF=\"",
  		    escape_html(r->pool, location), "\">here</A>.<P>\n", NULL);
  	    break;
  	case AUTH_REQUIRED:

Re: CGI-related core

Posted by Tom Tromey <tr...@creche.cygnus.com>.
>> Last, I've noticed a minor inconsistency in send_error_response.
>> Most of the function uses the "status" variable.  But the switch
>> statement towards the end switches on r->status.  Is there a reason
>> for this discrepancy (and if so, could someone please add a comment
>> explaining the reason?).  My patch assumes that this is just an
>> oversight, and changes r->status to just status.

Brian> Yeah, looks like an oversight to me too - since the local
Brian> variable "status" doesn't get assigned anywhere except at the
Brian> top to r->status (though your patch now erases that
Brian> distinction, thus the resolution on the issue needed), that
Brian> seems right to me.

Brian> Bigger question, though - should your catch for the missing
Brian> error code change r-> status?

I'm afraid I don't know the answer to this one.  I don't know what is
done with r->status after send_error_response returns.

Tom
-- 
tromey@cygnus.com                 Member, League for Programming Freedom

Re: CGI-related core

Posted by Tom Tromey <tr...@creche.cygnus.com>.
A while back I wrote:

>> I've noticed there are a number of defined status codes missing
>> from the tables in http_protocol.c.  From the HTTP/1.0 spec: 201,
>> 202, 204, 301, 503.  Is there any reason why?  It makes sense for a
>> CGI script to return some of these codes, and this is something I'd
>> like to check in my test suite.  If there is no reason, I'll
>> happily generate a patch.

Appended is a patch to add all the missing codes from the HTTP/1.1
spec.  This patch is against the cvs repository.

A couple notes:

* I made the status_lines array static
* I didn't understand why response_titles and status_lines were
  separate.  I unified them.
* I left in the "506 Variant Also Varies" response.  Where is this
  response defined?  I didn't see it in HTTP/1.1 draft 07.  Is this an
  Apache extension?
* I left RESPONSE_CODE_LIST defined.  But it seems to me that
  index_of_response should just search status_lines for this info.
  That way all this information would be encoded in only one place.
  If this is an ok idea, I'll make this change.

BTW does anybody have a feel for how widely HTTP/1.1 will be received?
Eg is netscape going to use it?  Or is it going to be another HTML
3.0?

Tom
-- 
tromey@cygnus.com                 Member, League for Programming Freedom

Index: httpd.h
===================================================================
RCS file: /export/home/cvs/apache/src/httpd.h,v
retrieving revision 1.51
diff -c -5 -r1.51 httpd.h
*** httpd.h	1996/09/30 05:56:26	1.51
--- httpd.h	1996/09/30 20:44:52
***************
*** 270,280 ****
  #define SERVER_ERROR 500
  #define NOT_IMPLEMENTED 501
  #define BAD_GATEWAY 502
  #define HTTP_SERVICE_UNAVAILABLE 503
  #define VARIANT_ALSO_VARIES 506
! #define RESPONSE_CODES 18
  
  #define METHODS 8
  #define M_GET 0
  #define M_PUT 1
  #define M_POST 2
--- 270,280 ----
  #define SERVER_ERROR 500
  #define NOT_IMPLEMENTED 501
  #define BAD_GATEWAY 502
  #define HTTP_SERVICE_UNAVAILABLE 503
  #define VARIANT_ALSO_VARIES 506
! #define RESPONSE_CODES 38
  
  #define METHODS 8
  #define M_GET 0
  #define M_PUT 1
  #define M_POST 2
Index: http_protocol.c
===================================================================
RCS file: /export/home/cvs/apache/src/http_protocol.c,v
retrieving revision 1.51
diff -c -5 -r1.51 http_protocol.c
*** http_protocol.c	1996/09/30 05:56:25	1.51
--- http_protocol.c	1996/09/30 20:45:19
***************
*** 789,849 ****
      *pw = t;
  
      return OK;
  }
  
! #define RESPONSE_CODE_LIST " 200 206 300 301 302 304 400 401 403 404 405 406 411 412 500 503 501 502 506"
  
  /* New Apache routine to map error responses into array indicies 
   *  e.g.  400 -> 0,  500 -> 1,  502 -> 2 ...                     
   * the indicies have no significance
   */
  
! char *status_lines[] = {
     "200 OK",
     "206 Partial Content",
     "300 Multiple Choices",
     "301 Moved Permanently",
     "302 Found",
     "304 Not Modified",
     "400 Bad Request",
     "401 Unauthorized",
     "403 Forbidden",
     "404 Not found",
     "405 Method Not Allowed",
     "406 Not Acceptable",
     "411 Length Required",
     "412 Precondition Failed",
     "500 Server error",
-    "503 Out of resources",
     "501 Not Implemented",
     "502 Bad Gateway",
     "506 Variant Also Varies"
  }; 
  
- char *response_titles[] = {
-    "200 OK",			/* Never actually sent, barring die(200,...) */
-    "206 Partial Content",	/* Never sent as an error (we hope) */
-    "Multiple Choices",		/* 300 Multiple Choices */
-    "Document moved",		/* 301 Redirect */
-    "Document moved",		/* 302 Redirect */
-    "304 Not Modified",		/* Never sent... 304 MUST be header only */
-    "Bad Request",
-    "Authorization Required",
-    "Forbidden",
-    "File Not found",
-    "Method Not Allowed",
-    "Not Acceptable",
-    "Length Required",
-    "Precondition Failed",
-    "Server Error",
-    "Out of resources",
-    "Method not implemented",
-    "Bad Gateway",
-    "Variant Also Varies"
- };
- 
  int index_of_response(int err_no) {
     char *cptr, err_string[10];
     static char *response_codes = RESPONSE_CODE_LIST;
     int index_number;
     
--- 789,846 ----
      *pw = t;
  
      return OK;
  }
  
! #define RESPONSE_CODE_LIST " 100 101 200 201 202 203 204 205 206 300 301 302 303 304 305 400 401 402 403 404 405 406 407 408 409 410 411 412 413 414 415 500 501 502 503 504 505 506"
  
  /* New Apache routine to map error responses into array indicies 
   *  e.g.  400 -> 0,  500 -> 1,  502 -> 2 ...                     
   * the indicies have no significance
   */
  
! static char *status_lines[] = {
!    "100 Continue",
!    "101 Switching Protocols",
     "200 OK",
+    "201 Created",
+    "202 Accepted",
+    "203 Non-Authoritative Information",
+    "204 No Content",
+    "205 Reset Content",
     "206 Partial Content",
     "300 Multiple Choices",
     "301 Moved Permanently",
     "302 Found",
+    "303 See Other",
     "304 Not Modified",
+    "305 Use Proxy",
     "400 Bad Request",
     "401 Unauthorized",
+    "402 Payment Required",
     "403 Forbidden",
     "404 Not found",
     "405 Method Not Allowed",
     "406 Not Acceptable",
+    "407 Proxy Authentication Required",
+    "408 Request Time-out",
+    "409 Conflict",
+    "410 Gone",
     "411 Length Required",
     "412 Precondition Failed",
+    "413 Request Entity Too Large",
+    "414 Request-URI Too Large",
+    "415 Unsupported Media Type",
     "500 Server error",
     "501 Not Implemented",
     "502 Bad Gateway",
+    "503 Out of resources",
+    "504 Gateway Time-out",
+    "505 HTTP Version not supported",
     "506 Variant Also Varies"
  }; 
  
  int index_of_response(int err_no) {
     char *cptr, err_string[10];
     static char *response_codes = RESPONSE_CODE_LIST;
     int index_number;
     
***************
*** 1325,1335 ****
  	 */
  	while (r->prev && r->prev->status != 200)
            r = r->prev;
      }
      {
! 	char *title = response_titles[idx];
  	BUFF *fd = c->client;
  	
          bvputs(fd,"<HEAD><TITLE>", title, "</TITLE></HEAD>\n<BODY><H1>", title,
  	       "</H1>\n", NULL);
  	
--- 1322,1333 ----
  	 */
  	while (r->prev && r->prev->status != 200)
            r = r->prev;
      }
      {
!         /* Magic number "4" is length of a response code and a space.  */
! 	char *title = status_lines[idx] + 4;
  	BUFF *fd = c->client;
  	
          bvputs(fd,"<HEAD><TITLE>", title, "</TITLE></HEAD>\n<BODY><H1>", title,
  	       "</H1>\n", NULL);
  	

Re: CGI-related core

Posted by Brian Behlendorf <br...@organic.com>.
On 25 Sep 1996, Tom Tromey wrote:
> The problem occurs in the send_error_response function in
> http_protocol.c.  The problem is that the function index_of_response
> can return -1, but the function response_code_string isn't prepared to
> handle that case.
> 
> I fixed this by having send_error_response check for the -1 return,
> and switch to a "500" error code in this case.  I don't know if this
> is the best solution or not.  However, it did eliminate the core dump.

The patch made sense to me, no vetos in 3 days, it's now committed.  

> I've noticed there are a number of defined status codes missing from
> the tables in http_protocol.c.  From the HTTP/1.0 spec: 201, 202, 204,
> 301, 503.  Is there any reason why?  It makes sense for a CGI script
> to return some of these codes, and this is something I'd like to check
> in my test suite.  If there is no reason, I'll happily generate a
> patch.

No contest here.

> Last, I've noticed a minor inconsistency in send_error_response.  Most
> of the function uses the "status" variable.  But the switch statement
> towards the end switches on r->status.  Is there a reason for this
> discrepancy (and if so, could someone please add a comment explaining
> the reason?).  My patch assumes that this is just an oversight, and
> changes r->status to just status.

Yeah, looks like an oversight to me too - since the local variable "status"
doesn't get assigned anywhere except at the top to r->status (though your patch
now erases that distinction, thus the resolution on the issue needed), that
seems right to me.

Bigger question, though - should your catch for the missing error code change
r->status?

	Brian

--=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=--
brian@organic.com  www.apache.org  hyperreal.com  http://www.organic.com/JOBS