On Thu, Jul 21, 2011 at 02:21:19PM +0100, Federico Schwindt wrote:
> On Thu, Jul 14, 2011 at 4:40 AM, David Gwynne <l...@animata.net> wrote:
> > in my environment i have nginx in front of apache to offload ssl
> > and to let me easily point different parts of the uri namespace at
> > all crazy backends we have. this works fine except if the apache
> > wants to canonicalise something on the "ssl" backends. because the
> > ssl is done in nginx, apache doesnt know that it should use https
> > as the scheme rather than just http and redirects the user to the
> > wrong port.
> >[..]
> > ok?
> 
> no as it is. please don't use atoi, use ap_strtol.

it seems a bit unfair of you to hold me to a higher standard than
the rest of the apache codebase, but ill live :)

> to check for `:' you can use strchr(). i believe you need an ap_pstrdup() for:
> 
> +               cmd->server->server_hostname = arg;

other stuff stores arg directly, so i assumed that was ok. again,
i'll live :)

> the strtonum() is wrong. it should be 65535 (the value is inclusive)

cool.

> but you could replace that bit just calling server_port().

but then id be calling atoi...

> you need to update the documentation as well.

true.

here's an update diff to the code:

Index: src/include/http_core.h
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/src/include/http_core.h,v
retrieving revision 1.12
diff -u -p -r1.12 http_core.h
--- src/include/http_core.h     24 Aug 2007 11:31:29 -0000      1.12
+++ src/include/http_core.h     25 Jul 2011 06:45:38 -0000
@@ -138,6 +138,8 @@ API_EXPORT(const char *) ap_get_remote_l
 API_EXPORT(char *) ap_construct_url(pool *p, const char *uri, request_rec *r);
 API_EXPORT(const char *) ap_get_server_name(request_rec *r);
 API_EXPORT(unsigned) ap_get_server_port(const request_rec *r);
+API_EXPORT(const char *) ap_get_server_method(const request_rec *r);
+API_EXPORT(unsigned) ap_get_default_port(const request_rec *r);
 API_EXPORT(unsigned long) ap_get_limit_req_body(const request_rec *r);
 API_EXPORT(void) ap_custom_response(request_rec *r, int status, char *string);
 API_EXPORT(int) ap_exists_config_define(char *name);
Index: src/include/httpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/src/include/httpd.h,v
retrieving revision 1.30
diff -u -p -r1.30 httpd.h
--- src/include/httpd.h 25 Feb 2010 07:49:53 -0000      1.30
+++ src/include/httpd.h 25 Jul 2011 06:45:38 -0000
@@ -141,12 +141,8 @@ extern "C" {
 #define DEFAULT_HTTP_PORT      80
 #define DEFAULT_HTTPS_PORT     443
 #define ap_is_default_port(port,r)     ((port) == ap_default_port(r))
-#define ap_http_method(r)   (((r)->ctx != NULL && ap_ctx_get((r)->ctx, \
-    "ap::http::method") != NULL) ? ((char *)ap_ctx_get((r)->ctx,       \
-    "ap::http::method")) : "http")
-#define ap_default_port(r)  (((r)->ctx != NULL && ap_ctx_get((r)->ctx, \
-    "ap::default::port") != NULL) ? atoi((char *)ap_ctx_get((r)->ctx,  \
-    "ap::default::port")) : DEFAULT_HTTP_PORT)
+#define ap_http_method(r)   ap_get_server_method(r)
+#define ap_default_port(r)  ap_get_default_port(r)
 
 /* --------- Default user name and group name running standalone ---------- */
 /* --- These may be specified as numbers by placing a # before a number --- */
Index: src/main/http_core.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/src/main/http_core.c,v
retrieving revision 1.27
diff -u -p -r1.27 http_core.c
--- src/main/http_core.c        10 May 2010 02:00:50 -0000      1.27
+++ src/main/http_core.c        25 Jul 2011 06:45:38 -0000
@@ -804,6 +804,42 @@ ap_get_server_port(const request_rec *r)
            : port;
 }
 
+API_EXPORT(const char *)
+ap_get_server_method(const request_rec *r)
+{
+       const char *method;
+
+       if (r->ctx != NULL) {
+               method = ap_ctx_get(r->ctx, "ap::http::method");
+               if (method != NULL)
+                       return (method);
+       }
+
+       if (r->server->ctx != NULL) {
+               method = ap_ctx_get(r->server->ctx, "ap::http::method");
+               if (method != NULL)
+                       return (method);
+       }
+
+       return ("http");
+}
+
+API_EXPORT(unsigned)
+ap_get_default_port(const request_rec *r)
+{
+       const char *v = NULL;
+
+       if (r->ctx != NULL)
+               v = ap_ctx_get(r->ctx, "ap::default::port");
+       if (v == NULL && r->server->ctx != NULL)
+               v = ap_ctx_get(r->server->ctx, "ap::default::port");
+
+       if (v == NULL)
+               return (DEFAULT_HTTP_PORT);
+
+       return ((unsigned)ap_strtol(v, NULL, 10));
+}
+
 API_EXPORT(char *)
 ap_construct_url(pool *p, const char *uri, request_rec *r)
 {
@@ -1751,6 +1787,43 @@ static const char *set_server_string_slo
     return NULL;
 }
 
+static const char *
+set_server_name(cmd_parms *cmd, void *dummy, char *arg)
+{
+       const char *err = ap_check_cmd_context(cmd,
+           NOT_IN_DIR_LOC_FILE|NOT_IN_LIMIT);
+       const char *part;
+       int port;
+
+       if (err != NULL)
+               return (err);
+
+       if (strncmp("https://";, arg, 8) == 0) {
+               ap_ctx_set(cmd->server->ctx, "ap::http::method", "https");
+               ap_ctx_set(cmd->server->ctx, "ap::default::port", "443");
+               arg += 8;
+       } else if (strncmp("http://";, arg, 7) == 0) {
+               /* defaults are fine */
+               arg += 7;
+       } else if (strstr(arg, "://") != NULL)
+               return ("unsupported scheme");
+
+       part = strstr(arg, ":");
+       if (part != NULL) {
+               port = (int)strtonum(part + 1, 1, 65535, &err);
+               if (err != NULL) {
+                       return ap_pstrcat(cmd->temp_pool,
+                           "The port number \"", part + 1, "\" is ", err, 
NULL);
+               }
+               cmd->server->port = port;
+               cmd->server->server_hostname = ap_pstrndup(cmd->pool, arg,
+                   part - arg);
+       } else 
+               cmd->server->server_hostname = ap_pstrdup(cmd->pool, arg);
+
+       return (NULL);
+}
+
 static const char *server_type(cmd_parms *cmd, void *dummy, char *arg)
 {
     const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
@@ -3152,8 +3225,7 @@ static const command_rec core_cmds[] = {
 { "ServerAdmin", set_server_string_slot,
   (void *)XtOffsetOf (server_rec, server_admin), RSRC_CONF, TAKE1,
   "The email address of the server administrator" },
-{ "ServerName", set_server_string_slot,
-  (void *)XtOffsetOf (server_rec, server_hostname), RSRC_CONF, TAKE1,
+{ "ServerName", set_server_name, NULL, RSRC_CONF, TAKE1,
   "The hostname of the server" },
 { "ServerSignature", set_signature_flag, NULL, OR_ALL, TAKE1,
   "En-/disable server signature (on|off|email)" },

Reply via email to