Re: Bug handling Upgrade: TLS/1.0 header in ssl_engine_io.c

[Joel J. Smith]

Joe Orton wrote:
On Tue, Mar 08, 2005 at 08:54:52PM -0700, Joel J Smith wrote:
Hi httpd folks,
It seems that Joe Orton introduced a bug while updating ssl_engine_io.c
between version 109499 and version 111159.  The same bug was introduced
into NetWare's mod_nw_ssl.c version 111327. (Please forgive me if that's
not the correct way to reference svn version numbers... I'm new to svn.)
The code in question is part of the TLS Upgrade feature described in
RFC 2817 and was originally written by Ryan Bloom and committed by
Bill Rowe if I'm not mistaken.

Ah, right, thanks.  For some reason I thought the ordering in the header
did matter but I can't see why now.  Can you send a patch to correct the
issue?
I figured that was probably the case.  Patch attached.  Would it be 
helpful for me to create a bug report with the details from my first 
email that you can reference in the commit, or would that be more of a 
nuisance?  Looking back, I probably should have gone through bugzilla to 
begin with instead of bothering the dev list.
Thanks again,
Joel
--- ssl_engine_io.c.orig        2005-03-10 09:38:12.000000000 -0700
+++ ssl_engine_io.c     2005-03-10 09:40:03.000000000 -0700
@@ -1183,6 +1183,9 @@
     apr_status_t rv;
     apr_bucket *b;
     SSL *ssl;
+    char *token_string;
+    char *token;
+    char *token_state;
 
     /* Just remove the filter, if it doesn't work the first time, it won't
      * work at all for this request.
@@ -1194,9 +1197,17 @@
      */
 
     upgrade = apr_table_get(r->headers_in, "Upgrade");
-    if (upgrade == NULL
-        || strcmp(ap_getword(r->pool, &upgrade, ','), "TLS/1.0")) {
-        /* "Upgrade: TLS/1.0, ..." header not found, don't do Upgrade */
+    if (upgrade == NULL) {
+        /* "Upgrade: " header not found, don't do Upgrade */
+        return ap_pass_brigade(f->next, bb);
+    }
+    token_string = apr_pstrdup(r->pool,upgrade);
+    token = apr_strtok(token_string,", ",&token_state);
+    while (token != NULL && strcasecmp(token,"TLS/1.0")) {
+        token = apr_strtok(NULL,", ",&token_state);
+    }
+    if (token == NULL) {
+        /* "TLS/1.0" token not in Upgrade header, * don't do Upgrade */
         return ap_pass_brigade(f->next, bb);
     }