Yes, that's better/more efficient (rfc 6455 mandates "Connection:
upgrade" though).
Thanks, commited in r1674632.

On Sun, Apr 19, 2015 at 1:17 AM, Eric Covener <cove...@gmail.com> wrote:
> +1, but why not skip right to the Upgrade: websocket check?
>
> On Fri, Apr 17, 2015 at 4:26 PM, Yann Ylavic <ylavic....@gmail.com> wrote:
>> Follow up to [1] based on users@ experience...
>>
>> On Fri, Apr 17, 2015 at 3:38 PM, Marc Hörsken <i...@marc-hoersken.de> wrote:
>>>
>>> I just figured out the configuration issue causing my problem.
>>>
>>> Original configuration mod_proxy_wstunnel with SwampDragon:
>>>
>>> ProxyPass /data/ ws://127.0.0.1:9001/data/
>>> ProxyPassReverse /data/ ws://127.0.0.1:9001/data/
>>> ProxyPass /settings.js http://127.0.0.1:9001/settings.js
>>> ProxyPassReverse /settings.js http://127.0.0.1:9001/settings.js
>>> ProxyPreserveHost On
>>> ProxyRequests Off
>>> ProxyVia On
>>>
>>> The following configuration for mod_proxy_wstunnel with SwampDragon works 
>>> fine:
>>>
>>> ProxyPassMatch /data/(\d+)/(\w+)/websocket 
>>> ws://127.0.0.1:9001/data/$1/$2/websocket
>>> ProxyPass /data/info http://127.0.0.1:9001/data/info
>>> ProxyPassReverse /data/info http://127.0.0.1:9001/data/info
>>> ProxyPass /settings.js http://127.0.0.1:9001/settings.js
>>> ProxyPassReverse /settings.js http://127.0.0.1:9001/settings.js
>>> ProxyPreserveHost On
>>> ProxyRequests Off
>>> ProxyVia On
>>>
>>> The issue was caused by the fact that /data/info is requested before a 
>>> WebSocket-upgrade
>>> request to /data/(\d+)/(\w+)/websocket is performed. Because /data/info was 
>>> redirected to
>>> the WebSocket-server using the same rule as /data/(\d+)/(\w+)/websocket 
>>> before,
>>> mod_proxy_wstunnel continued to also forward all HTTP-traffic to the 
>>> WebSocket-server.
>>>
>>
>> As already proposed in [1], shouldn't mod_proxy_wstunnel should
>> DECLINE[D] the request if it is not an "Upgrade: WebSocket" one?
>>
>> This could possibly simplify the configuration above to:
>>   ProxyPass / ws://127.0.0.1:9001/
>>   ProxyPass / http://127.0.0.1:9001/
>>
>> The patch would be something like:
>>
>> Index: modules/proxy/mod_proxy_wstunnel.c
>> ===================================================================
>> --- modules/proxy/mod_proxy_wstunnel.c    (revision 1665828)
>> +++ modules/proxy/mod_proxy_wstunnel.c    (working copy)
>> @@ -305,7 +320,7 @@ static int proxy_wstunnel_handler(request_rec *r,
>>      int status;
>>      char server_portstr[32];
>>      proxy_conn_rec *backend = NULL;
>> -    char *scheme;
>> +    const char *scheme, *upgrade;
>>      int retry;
>>      conn_rec *c = r->connection;
>>      apr_pool_t *p = r->pool;
>> @@ -324,6 +339,25 @@ static int proxy_wstunnel_handler(request_rec *r,
>>          return DECLINED;
>>      }
>>
>> +    upgrade = apr_table_get(r->headers_in, "Connection");
>> +    if (upgrade) {
>> +        if (strcasecmp(upgrade, "Upgrade") != 0) {
>> +            upgrade = NULL;
>> +        }
>> +        else {
>> +            upgrade = apr_table_get(r->headers_in, "Upgrade");
>> +            if (upgrade && strcasecmp(upgrade, "WebSocket") != 0) {
>> +                upgrade = NULL;
>> +            }
>> +        }
>> +    }
>> +    if (!upgrade) {
>> +        ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
>> +                      APLOGNO() "%s: declining URL %s (not WebSocket)",
>> +                      scheme, url);
>> +        return DECLINED;
>> +    }
>> +
>>      uri = apr_palloc(p, sizeof(*uri));
>>      ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02451)
>> "serving URL %s", url);
>>
>> --
>>
>> [1] 
>> https://mail-archives.apache.org/mod_mbox/httpd-dev/201503.mbox/%3CCAKQ1sVPq-9cqS4zKvfwuC5Hi1mXZ=pkfuxrqsv2lexsnrzd...@mail.gmail.com%3E
>
>
>
> --
> Eric Covener
> cove...@gmail.com

Reply via email to