httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Marion & Christophe JAILLET <christophe.jail...@wanadoo.fr>
Subject Re: svn commit: r1715294 - /httpd/httpd/trunk/server/core.c
Date Fri, 20 Nov 2015 17:58:10 GMT
That was I first thought too.

When I looked at r1715255, I noticed:
          const char *conn = apr_table_get(r->headers_in, "Connection");
          if (ap_find_token(r->pool, conn, "upgrade")) {
and looked for inconsistencies.

When digging deeper, I found that "Connection Upgrade" was used in most 
cases in httpd. That's why I reverted.

The only places where I found the lowercase version are:
   core.c:5366:               if (ap_find_token(r->pool, conn, "upgrade")) {
   ssl_engine_kernel.c:1344:  apr_table_mergen(r->headers_out, 
"Connection", "upgrade");
   http2:                     in different places

So, for consistency, it's maybe these places that should be updated???


In any case, I don't think that it would avoid some case folding.
For "Connection upgrade", the only places I've found that process it is 
the one given above (core.c:5366)
In this case, taking advantage of a lower case version would require to 
tweak ap_find_token.
This would, IMHO, add complexity to the code and would be worse for the 
common case (i.e. if what we have does not match what we test, we would 
test twice)


If having upgrade in lower case makes it way, other places should also 
be looked at:
     Upgrade: WebSocket    vs    websocket

CJ


Le 20/11/2015 00:24, William A Rowe Jr a écrit :
> It wouldn't hurt to change this though, the tokens are generally 
> represented in lowercase, and this could avoid case folding I suppose.
>
> How often do we see value tokens as upper case from httpd? Let's be 
> consistent although it isn't strictly required.
>
>
>
> On Thu, Nov 19, 2015 at 3:54 PM, <jailletc36@apache.org 
> <mailto:jailletc36@apache.org>> wrote:
>
>     Author: jailletc36
>     Date: Thu Nov 19 21:54:48 2015
>     New Revision: 1715294
>
>     URL: http://svn.apache.org/viewvc?rev=1715294&view=rev
>     Log:
>     Revert r1715289 (Connection header field should use "upgrade"
>     instead of "Upgrade")
>
>     This is case-insensitive, so no need for such a change.
>
>     Modified:
>         httpd/httpd/trunk/server/core.c
>
>     Modified: httpd/httpd/trunk/server/core.c
>     URL:
>     http://svn.apache.org/viewvc/httpd/httpd/trunk/server/core.c?rev=1715294&r1=1715293&r2=1715294&view=diff
>     ==============================================================================
>     --- httpd/httpd/trunk/server/core.c (original)
>     +++ httpd/httpd/trunk/server/core.c Thu Nov 19 21:54:48 2015
>     @@ -5379,7 +5379,7 @@ static int core_upgrade_handler(request_
>                          /* Let the client know what we are upgrading
>     to. */
>                          apr_table_clear(r->headers_out);
>                          apr_table_setn(r->headers_out, "Upgrade",
>     protocol);
>     -                    apr_table_setn(r->headers_out, "Connection",
>     "upgrade");
>     +                    apr_table_setn(r->headers_out, "Connection",
>     "Upgrade");
>
>                          r->status = HTTP_SWITCHING_PROTOCOLS;
>                          r->status_line = ap_get_status_line(r->status);
>     @@ -5404,7 +5404,7 @@ static int core_upgrade_handler(request_
>              if (upgrades && upgrades->nelts > 0) {
>                  char *protocols = apr_array_pstrcat(r->pool,
>     upgrades, ',');
>                  apr_table_setn(r->headers_out, "Upgrade", protocols);
>     -            apr_table_setn(r->headers_out, "Connection", "upgrade");
>     +            apr_table_setn(r->headers_out, "Connection", "Upgrade");
>              }
>          }
>
>
>
>


Mime
View raw message