httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From r..@covalent.net
Subject Re: [PATCH] filtering and canned error responses
Date Fri, 01 Sep 2000 19:51:18 GMT

I disagree with this.  Sorry for not speaking up before.  I have been very
busy, and I have had many other things on my mind.  All text needs to be
passed through all filters.  Think about the canned errors generated by
mistakes in SSIs.  Those are part of the document, and should be processed
by the rest of the filters in the chain.  None of our canned errors should
cause any filter to modify data, because they don't have any tags in them,
but that doesn't mean that somebody who write mod_mumblefoo, won't want
their filter to put the current date in the error using SSI tags.

Ryan

On 1 Sep 2000, Jeff Trawick wrote:

> Jeff Trawick <trawickj@bellsouth.net> writes:
> 
> > For the purposes of this discussion, "canned error responses" are
> > those strings generated by ap_send_error_response().
> > 
> > Currently, there is non-sensical filtering done on this text.  Normal
> > request processing triggered modules to set up filtering appropriate
> > to the original request.  But the original request failed for some
> > reason and now we're creating an error message.  Should content
> > filters see this message?
> > 
> > I think that these messages should be passed to the connection
> > filters, skipping the content filters.
> > 
> > An example problem: 
> > 
> > Tell Apache that pages in root/1047 are in character set IBM-1047.
> > Ask for non-existent file root/1047/index.html.  mod_charset_lite sets
> > up a translation filter.  The only content is the "404 Not Found"
> > document, which is in iso-8859-1, but mod_charset_lite translates it
> > as if it were IBM-1047, resulting in garbage.
> > 
> > mod_charset_lite might be able to perform extra checks to detect this
> > situation, but I'm thinking that in general all content filters could
> > run into non-sensical (or at least unnecessary) processing.
> > 
> > Comments?
> > 
> > -- 
> > Jeff Trawick | trawick@ibm.net | PGP public key at web site:
> >      http://www.geocities.com/SiliconValley/Park/9289/
> >           Born in Roswell... married an alien...
> > 
> 
> This patch implements the suggestion described above...  It is
> intended to work for the messages hard-coded in apache as well as
> ErrorDocument literal strings (e.g., `ErrorDocument 404 "You're such a
> loser"').
> 
> Any concerns to discuss?
> 
> It seems likely that rvputs_canned_msg() will eventually indicate in
> the buckets it creates that the strings are in the implementation
> character set.  Such a flag could be used to trigger translation of
> these error strings as well as chunk headers/trailers and headers.
> Unclear... 
> 
> Index: main/http_protocol.c
> ===================================================================
> RCS file: /home/cvspublic/apache-2.0/src/main/http_protocol.c,v
> retrieving revision 1.118
> diff -u -r1.118 http_protocol.c
> --- main/http_protocol.c	2000/08/30 01:09:03	1.118
> +++ main/http_protocol.c	2000/09/01 17:38:26
> @@ -2624,6 +2624,37 @@
>      return written;
>  }
>  
> +static int rvputs_canned_msg(request_rec *r, ...)
> +{
> +    ap_bucket_brigade *bb = NULL;
> +    apr_ssize_t written;
> +    va_list va;
> +
> +    if (r->connection->aborted)
> +        return EOF;
> +    bb = ap_brigade_create(r->pool);
> +    va_start(va, r);
> +    written = ap_brigade_vputstrs(bb, va);
> +    va_end(va);
> +    if (written != 0) {
> +        /* This is canned text which should not be processed by content
> +         * filters, so find the first connection filter and hand it
> +         * the brigade.
> +         */
> +        ap_filter_t *f = r->filters;
> +
> +        /* XXX: check should be "ftype != AP_FTYPE_CONNECTION", but we're 
> +         * playing tricks currently with the types of chunk_filter and 
> +         * core_filter
> +         */
> +        while (f->ftype == AP_FTYPE_CONTENT) {
> +            f = f->next;
> +        }
> +        ap_pass_brigade(f, bb);
> +    }
> +    return written;
> +}
> +
>  API_EXPORT(int) ap_rflush(request_rec *r)
>  {
>      /* ### this is probably incorrect, but we have no mechanism for telling
> @@ -2762,7 +2793,7 @@
>           * the original error and output that as well.
>           */
>          if (custom_response[0] == '\"') {
> -            ap_rputs(custom_response + 1, r);
> +            rvputs_canned_msg(r, custom_response + 1, NULL);
>              ap_finalize_request_protocol(r);
>              ap_rflush(r);
>              return;
> @@ -2794,187 +2825,187 @@
>          /* folks decided they didn't want the error code in the H1 text */
>          h1 = &title[4];
>  
> -        ap_rvputs(r,
> -                  DOCTYPE_HTML_2_0
> -                  "<HTML><HEAD>\n<TITLE>", title,
> -                  "</TITLE>\n</HEAD><BODY>\n<H1>", h1, "</H1>\n",
> -                  NULL);
> +        rvputs_canned_msg(r,
> +                          DOCTYPE_HTML_2_0
> +                          "<HTML><HEAD>\n<TITLE>", title,
> +                          "</TITLE>\n</HEAD><BODY>\n<H1>", h1,
"</H1>\n",
> +                          NULL);
>  
>  	switch (status) {
>  	case HTTP_MOVED_PERMANENTLY:
>  	case HTTP_MOVED_TEMPORARILY:
>  	case HTTP_TEMPORARY_REDIRECT:
> -	    ap_rvputs(r, "The document has moved <A HREF=\"",
> -		      ap_escape_html(r->pool, location), "\">here</A>.<P>\n",
> -		      NULL);
> +	    rvputs_canned_msg(r, "The document has moved <A HREF=\"",
> +                              ap_escape_html(r->pool, location), "\">here</A>.<P>\n",
> +                              NULL);
>  	    break;
>  	case HTTP_SEE_OTHER:
> -	    ap_rvputs(r, "The answer to your request is located <A HREF=\"",
> -		      ap_escape_html(r->pool, location), "\">here</A>.<P>\n",
> -		      NULL);
> +	    rvputs_canned_msg(r, "The answer to your request is located <A HREF=\"",
> +                              ap_escape_html(r->pool, location), "\">here</A>.<P>\n",
> +                              NULL);
>  	    break;
>  	case HTTP_USE_PROXY:
> -	    ap_rvputs(r, "This resource is only accessible "
> -		      "through the proxy\n",
> -		      ap_escape_html(r->pool, location),
> -		      "<BR>\nYou will need to ",
> -		      "configure your client to use that proxy.<P>\n", NULL);
> +	    rvputs_canned_msg(r, "This resource is only accessible "
> +                              "through the proxy\n",
> +                              ap_escape_html(r->pool, location),
> +                              "<BR>\nYou will need to ",
> +                              "configure your client to use that proxy.<P>\n",
NULL);
>  	    break;
>  	case HTTP_PROXY_AUTHENTICATION_REQUIRED:
>  	case HTTP_UNAUTHORIZED:
> -	    ap_rputs("This server could not verify that you\n"
> -	             "are authorized to access the document\n"
> -	             "requested.  Either you supplied the wrong\n"
> -	             "credentials (e.g., bad password), or your\n"
> -	             "browser doesn't understand how to supply\n"
> -	             "the credentials required.<P>\n", r);
> +	    rvputs_canned_msg(r, "This server could not verify that you\n"
> +                              "are authorized to access the document\n"
> +                              "requested.  Either you supplied the wrong\n"
> +                              "credentials (e.g., bad password), or your\n"
> +                              "browser doesn't understand how to supply\n"
> +                              "the credentials required.<P>\n", NULL);
>  	    break;
>  	case HTTP_BAD_REQUEST:
> -	    ap_rputs("Your browser sent a request that "
> -	             "this server could not understand.<P>\n", r);
> +	    rvputs_canned_msg(r, "Your browser sent a request that "
> +                              "this server could not understand.<P>\n", NULL);
>  	    if ((error_notes = apr_table_get(r->notes, "error-notes")) != NULL) {
> -		ap_rvputs(r, error_notes, "<P>\n", NULL);
> +		rvputs_canned_msg(r, error_notes, "<P>\n", NULL);
>  	    }
>  	    break;
>  	case HTTP_FORBIDDEN:
> -	    ap_rvputs(r, "You don't have permission to access ",
> -		      ap_escape_html(r->pool, r->uri),
> -		      "\non this server.<P>\n", NULL);
> +	    rvputs_canned_msg(r, "You don't have permission to access ",
> +                              ap_escape_html(r->pool, r->uri),
> +                              "\non this server.<P>\n", NULL);
>  	    break;
>  	case HTTP_NOT_FOUND:
> -	    ap_rvputs(r, "The requested URL ",
> -		      ap_escape_html(r->pool, r->uri),
> -		      " was not found on this server.<P>\n", NULL);
> +	    rvputs_canned_msg(r, "The requested URL ",
> +                              ap_escape_html(r->pool, r->uri),
> +                              " was not found on this server.<P>\n", NULL);
>  	    break;
>  	case HTTP_METHOD_NOT_ALLOWED:
> -	    ap_rvputs(r, "The requested method ", r->method,
> -		      " is not allowed "
> -		      "for the URL ", ap_escape_html(r->pool, r->uri),
> -		      ".<P>\n", NULL);
> +	    rvputs_canned_msg(r, "The requested method ", r->method,
> +                              " is not allowed "
> +                              "for the URL ", ap_escape_html(r->pool, r->uri),
> +                              ".<P>\n", NULL);
>  	    break;
>  	case HTTP_NOT_ACCEPTABLE:
> -	    ap_rvputs(r,
> -		      "An appropriate representation of the "
> -		      "requested resource ",
> -		      ap_escape_html(r->pool, r->uri),
> -		      " could not be found on this server.<P>\n", NULL);
> +	    rvputs_canned_msg(r,
> +                              "An appropriate representation of the "
> +                              "requested resource ",
> +                              ap_escape_html(r->pool, r->uri),
> +                              " could not be found on this server.<P>\n", NULL);
>  	    /* fall through */
>  	case HTTP_MULTIPLE_CHOICES:
>  	    {
>  		const char *list;
>  		if ((list = apr_table_get(r->notes, "variant-list")))
> -		    ap_rputs(list, r);
> +		    rvputs_canned_msg(r, list, NULL);
>  	    }
>  	    break;
>  	case HTTP_LENGTH_REQUIRED:
> -	    ap_rvputs(r, "A request of the requested method ", r->method,
> -		      " requires a valid Content-length.<P>\n", NULL);
> +	    rvputs_canned_msg(r, "A request of the requested method ", r->method,
> +                              " requires a valid Content-length.<P>\n", NULL);
>  	    if ((error_notes = apr_table_get(r->notes, "error-notes")) != NULL) {
> -		ap_rvputs(r, error_notes, "<P>\n", NULL);
> +		rvputs_canned_msg(r, error_notes, "<P>\n", NULL);
>  	    }
>  	    break;
>  	case HTTP_PRECONDITION_FAILED:
> -	    ap_rvputs(r, "The precondition on the request for the URL ",
> -		      ap_escape_html(r->pool, r->uri),
> -		      " evaluated to false.<P>\n", NULL);
> +	    rvputs_canned_msg(r, "The precondition on the request for the URL ",
> +                              ap_escape_html(r->pool, r->uri),
> +                              " evaluated to false.<P>\n", NULL);
>  	    break;
>  	case HTTP_NOT_IMPLEMENTED:
> -	    ap_rvputs(r, ap_escape_html(r->pool, r->method), " to ",
> -		      ap_escape_html(r->pool, r->uri),
> -		      " not supported.<P>\n", NULL);
> +	    rvputs_canned_msg(r, ap_escape_html(r->pool, r->method), " to ",
> +                              ap_escape_html(r->pool, r->uri),
> +                              " not supported.<P>\n", NULL);
>  	    if ((error_notes = apr_table_get(r->notes, "error-notes")) != NULL) {
> -		ap_rvputs(r, error_notes, "<P>\n", NULL);
> +		rvputs_canned_msg(r, error_notes, "<P>\n", NULL);
>  	    }
>  	    break;
>  	case HTTP_BAD_GATEWAY:
> -	    ap_rputs("The proxy server received an invalid" CRLF
> -	             "response from an upstream server.<P>" CRLF, r);
> +	    rvputs_canned_msg(r, "The proxy server received an invalid" CRLF
> +                              "response from an upstream server.<P>" CRLF, NULL);
>  	    if ((error_notes = apr_table_get(r->notes, "error-notes")) != NULL) {
> -		ap_rvputs(r, error_notes, "<P>\n", NULL);
> +		rvputs_canned_msg(r, error_notes, "<P>\n", NULL);
>  	    }
>  	    break;
>  	case HTTP_VARIANT_ALSO_VARIES:
> -	    ap_rvputs(r, "A variant for the requested resource\n<PRE>\n",
> -		      ap_escape_html(r->pool, r->uri),
> -		      "\n</PRE>\nis itself a negotiable resource. "
> -		      "This indicates a configuration error.<P>\n", NULL);
> +	    rvputs_canned_msg(r, "A variant for the requested resource\n<PRE>\n",
> +                              ap_escape_html(r->pool, r->uri),
> +                              "\n</PRE>\nis itself a negotiable resource. "
> +                              "This indicates a configuration error.<P>\n", NULL);
>  	    break;
>  	case HTTP_REQUEST_TIME_OUT:
> -	    ap_rputs("I'm tired of waiting for your request.\n", r);
> +	    rvputs_canned_msg(r, "I'm tired of waiting for your request.\n", NULL);
>  	    break;
>  	case HTTP_GONE:
> -	    ap_rvputs(r, "The requested resource<BR>",
> -		      ap_escape_html(r->pool, r->uri),
> -		      "<BR>\nis no longer available on this server ",
> -		      "and there is no forwarding address.\n",
> -		      "Please remove all references to this resource.\n",
> -		      NULL);
> +	    rvputs_canned_msg(r, "The requested resource<BR>",
> +                              ap_escape_html(r->pool, r->uri),
> +                              "<BR>\nis no longer available on this server ",
> +                              "and there is no forwarding address.\n",
> +                              "Please remove all references to this resource.\n",
> +                              NULL);
>  	    break;
>  	case HTTP_REQUEST_ENTITY_TOO_LARGE:
> -	    ap_rvputs(r, "The requested resource<BR>",
> -		      ap_escape_html(r->pool, r->uri), "<BR>\n",
> -		      "does not allow request data with ", r->method,
> -		      " requests, or the amount of data provided in\n",
> -		      "the request exceeds the capacity limit.\n", NULL);
> +	    rvputs_canned_msg(r, "The requested resource<BR>",
> +                              ap_escape_html(r->pool, r->uri), "<BR>\n",
> +                              "does not allow request data with ", r->method,
> +                              " requests, or the amount of data provided in\n",
> +                              "the request exceeds the capacity limit.\n", NULL);
>  	    break;
>  	case HTTP_REQUEST_URI_TOO_LARGE:
> -	    ap_rputs("The requested URL's length exceeds the capacity\n"
> -	             "limit for this server.<P>\n", r);
> +	    rvputs_canned_msg(r, "The requested URL's length exceeds the capacity\n"
> +                              "limit for this server.<P>\n", NULL);
>  	    if ((error_notes = apr_table_get(r->notes, "error-notes")) != NULL) {
> -		ap_rvputs(r, error_notes, "<P>\n", NULL);
> +		rvputs_canned_msg(r, error_notes, "<P>\n", NULL);
>  	    }
>  	    break;
>  	case HTTP_UNSUPPORTED_MEDIA_TYPE:
> -	    ap_rputs("The supplied request data is not in a format\n"
> -	             "acceptable for processing by this resource.\n", r);
> +	    rvputs_canned_msg(r, "The supplied request data is not in a format\n"
> +                              "acceptable for processing by this resource.\n", NULL);
>  	    break;
>  	case HTTP_RANGE_NOT_SATISFIABLE:
> -	    ap_rputs("None of the range-specifier values in the Range\n"
> -	             "request-header field overlap the current extent\n"
> -	             "of the selected resource.\n", r);
> +	    rvputs_canned_msg(r, "None of the range-specifier values in the Range\n"
> +                              "request-header field overlap the current extent\n"
> +                              "of the selected resource.\n", NULL);
>  	    break;
>  	case HTTP_EXPECTATION_FAILED:
> -	    ap_rvputs(r, "The expectation given in the Expect request-header"
> -	              "\nfield could not be met by this server.<P>\n"
> -	              "The client sent<PRE>\n    Expect: ",
> -	              apr_table_get(r->headers_in, "Expect"), "\n</PRE>\n"
> -	              "but we only allow the 100-continue expectation.\n",
> -	              NULL);
> +	    rvputs_canned_msg(r, "The expectation given in the Expect request-header"
> +                              "\nfield could not be met by this server.<P>\n"
> +                              "The client sent<PRE>\n    Expect: ",
> +                              apr_table_get(r->headers_in, "Expect"), "\n</PRE>\n"
> +                              "but we only allow the 100-continue expectation.\n",
> +                              NULL);
>  	    break;
>  	case HTTP_UNPROCESSABLE_ENTITY:
> -	    ap_rputs("The server understands the media type of the\n"
> -	             "request entity, but was unable to process the\n"
> -	             "contained instructions.\n", r);
> +	    rvputs_canned_msg(r, "The server understands the media type of the\n"
> +                              "request entity, but was unable to process the\n"
> +                              "contained instructions.\n", NULL);
>  	    break;
>  	case HTTP_LOCKED:
> -	    ap_rputs("The requested resource is currently locked.\n"
> -	             "The lock must be released or proper identification\n"
> -	             "given before the method can be applied.\n", r);
> +	    rvputs_canned_msg(r, "The requested resource is currently locked.\n"
> +                              "The lock must be released or proper identification\n"
> +                              "given before the method can be applied.\n", NULL);
>  	    break;
>  	case HTTP_FAILED_DEPENDENCY:
> -	    ap_rputs("The method could not be performed on the resource\n"
> -	             "because the requested action depended on another\n"
> -	             "action and that other action failed.\n", r);
> +	    rvputs_canned_msg(r, "The method could not be performed on the resource\n"
> +                              "because the requested action depended on another\n"
> +                              "action and that other action failed.\n", NULL);
>  	    break;
>  	case HTTP_INSUFFICIENT_STORAGE:
> -	    ap_rputs("The method could not be performed on the resource\n"
> -	             "because the server is unable to store the\n"
> -	             "representation needed to successfully complete the\n"
> -	             "request.  There is insufficient free space left in\n"
> -	             "your storage allocation.\n", r);
> +	    rvputs_canned_msg(r, "The method could not be performed on the resource\n"
> +                              "because the server is unable to store the\n"
> +                              "representation needed to successfully complete the\n"
> +                              "request.  There is insufficient free space left in\n"
> +                              "your storage allocation.\n", NULL);
>  	    break;
>  	case HTTP_SERVICE_UNAVAILABLE:
> -	    ap_rputs("The server is temporarily unable to service your\n"
> -	             "request due to maintenance downtime or capacity\n"
> -	             "problems. Please try again later.\n", r);
> +	    rvputs_canned_msg(r, "The server is temporarily unable to service your\n"
> +                              "request due to maintenance downtime or capacity\n"
> +                              "problems. Please try again later.\n", NULL);
>  	    break;
>  	case HTTP_GATEWAY_TIME_OUT:
> -	    ap_rputs("The proxy server did not receive a timely response\n"
> -	             "from the upstream server.\n", r);
> +	    rvputs_canned_msg(r, "The proxy server did not receive a timely response\n"
> +                              "from the upstream server.\n", NULL);
>  	    break;
>  	case HTTP_NOT_EXTENDED:
> -	    ap_rputs("A mandatory extension policy in the request is not\n"
> -	             "accepted by the server for this resource.\n", r);
> +	    rvputs_canned_msg(r, "A mandatory extension policy in the request is not\n"
> +                              "accepted by the server for this resource.\n", NULL);
>  	    break;
>  	default:            /* HTTP_INTERNAL_SERVER_ERROR */
>  	    /*
> @@ -2987,19 +3018,19 @@
>  	    if (((error_notes = apr_table_get(r->notes, "error-notes")) != NULL)
>  		&& (h1 = apr_table_get(r->notes, "verbose-error-to")) != NULL
>  		&& (strcmp(h1, "*") == 0)) {
> -	        ap_rvputs(r, error_notes, "<P>\n", NULL);
> +	        rvputs_canned_msg(r, error_notes, "<P>\n", NULL);
>  	    }
>  	    else {
> -	        ap_rvputs(r, "The server encountered an internal error or\n"
> -	             "misconfiguration and was unable to complete\n"
> -	             "your request.<P>\n"
> -	             "Please contact the server administrator,\n ",
> -	             ap_escape_html(r->pool, r->server->server_admin),
> -	             " and inform them of the time the error occurred,\n"
> -	             "and anything you might have done that may have\n"
> -	             "caused the error.<P>\n"
> -		     "More information about this error may be available\n"
> -		     "in the server error log.<P>\n", NULL);
> +	        rvputs_canned_msg(r, "The server encountered an internal error or\n"
> +                                  "misconfiguration and was unable to complete\n"
> +                                  "your request.<P>\n"
> +                                  "Please contact the server administrator,\n ",
> +                                  ap_escape_html(r->pool, r->server->server_admin),
> +                                  " and inform them of the time the error occurred,\n"
> +                                  "and anything you might have done that may have\n"
> +                                  "caused the error.<P>\n"
> +                                  "More information about this error may be available\n"
> +                                  "in the server error log.<P>\n", NULL);
>  	    }
>  	 /*
>  	  * It would be nice to give the user the information they need to
> @@ -3011,20 +3042,20 @@
>  	  * can figure out a way to remove the pathname, leave this commented.
>  	  *
>  	  * if ((error_notes = apr_table_get(r->notes, "error-notes")) != NULL) {
> -	  *     ap_rvputs(r, error_notes, "<P>\n", NULL);
> +	  *     rvputs_canned_msg(r, error_notes, "<P>\n", NULL);
>  	  * }
>  	  */
>  	    break;
>  	}
>  
>          if (recursive_error) {
> -            ap_rvputs(r, "<P>Additionally, a ",
> -                      status_lines[ap_index_of_response(recursive_error)],
> -                      "\nerror was encountered while trying to use an "
> -                      "ErrorDocument to handle the request.\n", NULL);
> +            rvputs_canned_msg(r, "<P>Additionally, a ",
> +                              status_lines[ap_index_of_response(recursive_error)],
> +                              "\nerror was encountered while trying to use an "
> +                              "ErrorDocument to handle the request.\n", NULL);
>          }
> -        ap_rputs(ap_psignature("<HR>\n", r), r);
> -        ap_rputs("</BODY></HTML>\n", r);
> +        rvputs_canned_msg(r, ap_psignature("<HR>\n", r), NULL);
> +        rvputs_canned_msg(r, "</BODY></HTML>\n", NULL);
>      }
>      ap_finalize_request_protocol(r);
>      ap_rflush(r);
> 
> -- 
> Jeff Trawick | trawick@ibm.net | PGP public key at web site:
>      http://www.geocities.com/SiliconValley/Park/9289/
>           Born in Roswell... married an alien...
> 


_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


Mime
View raw message