You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2009/01/31 10:53:55 UTC
Re: svn commit: r739382 - in /httpd/httpd/trunk: ./ include/ modules/filters/
modules/http/ modules/mappers/ modules/proxy/ server/
On 01/30/2009 08:12 PM, fielding@apache.org wrote:
> Author: fielding
> Date: Fri Jan 30 19:12:51 2009
> New Revision: 739382
>
> URL: http://svn.apache.org/viewvc?rev=739382&view=rev
> Log:
> Disabled DefaultType directive and removed ap_default_type()
> from core. We now exclude Content-Type from responses for which
> a media type has not been configured via mime.types, AddType,
> ForceType, or some other mechanism. MMN major bump to NZ time.
>
> PR: 13986
>
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/include/ap_mmn.h
> httpd/httpd/trunk/include/http_core.h
> httpd/httpd/trunk/include/http_protocol.h
> httpd/httpd/trunk/include/httpd.h
> httpd/httpd/trunk/modules/filters/mod_charset_lite.c
> httpd/httpd/trunk/modules/http/byterange_filter.c
> httpd/httpd/trunk/modules/http/http_filters.c
> httpd/httpd/trunk/modules/mappers/mod_actions.c
> httpd/httpd/trunk/modules/mappers/mod_negotiation.c
> httpd/httpd/trunk/modules/proxy/mod_proxy_ftp.c
> httpd/httpd/trunk/server/config.c
> httpd/httpd/trunk/server/core.c
> httpd/httpd/trunk/server/protocol.c
>
> Modified: httpd/httpd/trunk/server/config.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/config.c?rev=739382&r1=739381&r2=739382&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/config.c (original)
> +++ httpd/httpd/trunk/server/config.c Fri Jan 30 19:12:51 2009
> @@ -355,15 +355,15 @@
> return result;
> }
>
> - if (!r->handler) {
> - handler = r->content_type ? r->content_type : ap_default_type(r);
> + if (!r->handler && r->content_type) {
> + handler = r->content_type;
> if ((p=ap_strchr_c(handler, ';')) != NULL) {
> char *new_handler = (char *)apr_pmemdup(r->pool, handler,
> p - handler + 1);
> char *p2 = new_handler + (p - handler);
> handler = new_handler;
>
> - /* MIME type arguments */
> + /* exclude media type arguments */
> while (p2 > handler && p2[-1] == ' ')
> --p2; /* strip trailing spaces */
>
>
This causes the server to crash in case where no r->handler is set (e.g. in the case
of a non existing resource).
The following patch fixes this:
Index: server/config.c
===================================================================
--- server/config.c (Revision 739530)
+++ server/config.c (Arbeitskopie)
@@ -355,20 +355,25 @@
return result;
}
- if (!r->handler && r->content_type) {
- handler = r->content_type;
- if ((p=ap_strchr_c(handler, ';')) != NULL) {
- char *new_handler = (char *)apr_pmemdup(r->pool, handler,
- p - handler + 1);
- char *p2 = new_handler + (p - handler);
- handler = new_handler;
+ if (!r->handler) {
+ if (r->content_type) {
+ handler = r->content_type;
+ if ((p=ap_strchr_c(handler, ';')) != NULL) {
+ char *new_handler = (char *)apr_pmemdup(r->pool, handler,
+ p - handler + 1);
+ char *p2 = new_handler + (p - handler);
+ handler = new_handler;
- /* exclude media type arguments */
- while (p2 > handler && p2[-1] == ' ')
- --p2; /* strip trailing spaces */
+ /* exclude media type arguments */
+ while (p2 > handler && p2[-1] == ' ')
+ --p2; /* strip trailing spaces */
- *p2='\0';
+ *p2='\0';
+ }
}
+ else {
+ handler = "";
+ }
r->handler = handler;
}
Comments / thoughts / better ideas? Otherwise I would commit.
Regards
Rüdiger
Re: svn commit: r739382 - in /httpd/httpd/trunk: ./ include/ modules/filters/
modules/http/ modules/mappers/ modules/proxy/ server/
Posted by Ruediger Pluem <rp...@apache.org>.
On 01/31/2009 08:53 PM, Roy T. Fielding wrote:
> On Jan 31, 2009, at 1:53 AM, Ruediger Pluem wrote:
>> This causes the server to crash in case where no r->handler is set
>> (e.g. in the case
>> of a non existing resource).
>
> Bummer. I suppose it would be too difficult to fix the couple hundred
> places where strcmp is used on r->handler without checking for null.
Plus I bet there are a lot of third party modules whose handlers make
the assumption that r->handler != NULL. I am not sure if this is a promised
API guarantee, but I guess many rely on it.
> No objection to applying the patch, but mod_include uses
Done in r739598. I guess we need to find a more suitable place for
the DEFAULT_HANDLER_NAME define (and possibly prefix it with AP_).
Regards
Rüdiger
Re: svn commit: r739382 - in /httpd/httpd/trunk: ./ include/ modules/filters/ modules/http/ modules/mappers/ modules/proxy/ server/
Posted by "Roy T. Fielding" <fi...@gbiv.com>.
On Jan 31, 2009, at 1:53 AM, Ruediger Pluem wrote:
> This causes the server to crash in case where no r->handler is set
> (e.g. in the case
> of a non existing resource).
Bummer. I suppose it would be too difficult to fix the couple hundred
places where strcmp is used on r->handler without checking for null.
No objection to applying the patch, but mod_include uses
r->handler = "default-handler"
and others use symbols like
r->handler = DAV_HANDLER_NAME;
perhaps we should use
#define DEFAULT_HANDLER_NAME ""
r->handler = DEFAULT_HANDLER_NAME;
and then fix the other places that are inconsistent.
....Roy