On 11/04/2019 06:12 PM, Yann Ylavic wrote:
> On Mon, Nov 4, 2019 at 4:16 PM Ruediger Pluem <rpl...@apache.org> wrote:
>>
>> On 11/03/2019 04:48 PM, yla...@apache.org wrote:
>>>
>>> + /* XXX: what's the point of "NONE"? We probably should _always_ check
>>> + * that the client wants an Upgrade..
>>> + */
>>
>> NONE was used to upgrade the client whether it requested it or no idea what
>> the purpose was.
>> Maybe Jean-Frederic who introduced it in r1792092 can explain.
>
> I asked Jean-Frédéric in another thread, let's wait.
> I think it was not the correct way to address the issue reported to
> JBoss, and wish we are not stuck with "NONE" compatibility (i.e. no
> user..).
>
>>
>>
>>> + upgrade_method = *worker->s->upgrade ? worker->s->upgrade :
>>> "WebSocket";
>>> if (ap_cstr_casecmp(upgrade_method, "NONE") != 0) {
>>> - const char *upgrade;
>>> upgrade = apr_table_get(r->headers_in, "Upgrade");
>>> if (!upgrade || (ap_cstr_casecmp(upgrade, upgrade_method) != 0 &&
>>> - ap_cstr_casecmp(upgrade_method, "ANY") !=0)) {
>>> - ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02900)
>>> - "declining URL %s (not %s, Upgrade: header is
>>> %s)",
>>> - url, upgrade_method, upgrade ? upgrade :
>>> "missing");
>>> - return DECLINED;
>>> + ap_cstr_casecmp(upgrade_method, "ANY") != 0)) {
>>> + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(02900)
>>> + "require upgrade for URL %s "
>>> + "(Upgrade header is %s, expecting %s)",
>>> + url, upgrade ? upgrade : "missing",
>>> upgrade_method);
>>> + apr_table_setn(r->err_headers_out, "Connection", "Upgrade");
>>> + apr_table_setn(r->err_headers_out, "Upgrade", upgrade_method);
>>> + return HTTP_UPGRADE_REQUIRED;
>>
>> Why don't we fallback any longer to normal HTTP handling if no upgrade
>> handler was sent?
>> This was itruduced by you in r1674632
>
> Actually we never fallback despite r1674632 or even later r1776290 by
> Eric, because previous proxy_trans hook already assigned "ws(s):"
> scheme to r->filename. So at this point HTTP_UPGRADE_REQUIRED is the
> best we can do IMHO.
>
> I'm about to commit something like the attached patch to make this
> work, but wouldn't want to handle "NONE" there nor in a future
> mod_proxy_http change to handle Upgrade..
Just that I get it correct:
This would require something like
ProxyPass / ws://hostname/
ProxyPass / http://hostname/
to work and would only work with ProxyPass, not with RewriteRules and [P] flag,
correct?
Regards
Rüdiger
