mod_proxy_wstunnel ignores proxy_wstunnel_transfer errors

classic Classic list List threaded Threaded
16 messages Options
Reply | Threaded
Open this post in threaded view
|

mod_proxy_wstunnel ignores proxy_wstunnel_transfer errors

Jan Kaluža
Hi,

I have found out that when WSS is used and SSL handshake fails, httpd
closes client connection without any response to the client.

In the log, one can see following:

mod_proxy_wstunnel.c(131): (103)Software caused connection abort:
[client 127.0.0.1:49915] AH02442: error on sock - ap_get_brigade

Attached patch against 2.4.x fixes it. I'm not committing it, because
this problem has been introduced in r1493741 and seems like intentional
thing. This commit has been reverted in r1605946, so my theory is that
this particular part of mod_proxy_wstunnel has not been reverted
completely, but I want to be sure before I commit/propose.

Regards,
Jan Kaluza

httpd-2.4.x-wss-error.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: mod_proxy_wstunnel ignores proxy_wstunnel_transfer errors

Yann Ylavic
On Tue, Mar 17, 2015 at 12:38 PM, Jan Kaluža <[hidden email]> wrote:
> Hi,
>
> I have found out that when WSS is used and SSL handshake fails, httpd closes
> client connection without any response to the client.

If the SSL handshake fails, there is no SSL established connection
which we can send an HTTP response on.
We can only send an SSL alert in this case, and I think mod_ssl takes
care of this already (this occurs while reading the request header,
before mod_proxy_wstunnel IMHO).

>
> In the log, one can see following:
>
> mod_proxy_wstunnel.c(131): (103)Software caused connection abort: [client
> 127.0.0.1:49915] AH02442: error on sock - ap_get_brigade
>
> Attached patch against 2.4.x fixes it. I'm not committing it, because this
> problem has been introduced in r1493741 and seems like intentional thing.
> This commit has been reverted in r1605946, so my theory is that this
> particular part of mod_proxy_wstunnel has not been reverted completely, but
> I want to be sure before I commit/propose.

One the Upgrade is done, I don't think we can respond with 500 (in the
poll()ing phase, this is no more HTTP).
AFAICT r1605946 did nor revert r1493741, and I think this rather comes
for https://bz.apache.org/bugzilla/show_bug.cgi?id=56299#c7.

Regards,
Yann.
Reply | Threaded
Open this post in threaded view
|

Re: mod_proxy_wstunnel ignores proxy_wstunnel_transfer errors

Jan Kaluža
On 03/17/2015 01:23 PM, Yann Ylavic wrote:

> On Tue, Mar 17, 2015 at 12:38 PM, Jan Kaluža <[hidden email]> wrote:
>> Hi,
>>
>> I have found out that when WSS is used and SSL handshake fails, httpd closes
>> client connection without any response to the client.
>
> If the SSL handshake fails, there is no SSL established connection
> which we can send an HTTP response on.
> We can only send an SSL alert in this case, and I think mod_ssl takes
> care of this already (this occurs while reading the request header,
> before mod_proxy_wstunnel IMHO).

Hm, maybe I described it wrongly. What I see here is "Empty response
from server" when I do following:

1. Use this configuration:

ProxyTimeout 2
SSLProxyEngine on
<Location /test/>
     ProxyPass https://localhost:8080/
     ProxyPassReverse https://localhost:8080/
     ProxyPass wss://localhost:8080/
     ProxyPassReverse wss://localhost:8080/
</Location>


2. nc -l 8080 < /dev/null

3. curl -v --insecure https://127.0.0.1/test/
(...)
 > GET /test/ HTTP/1.1
 > User-Agent: curl/7.29.0
 > Host: 127.0.0.1
 > Accept: */*
 >
* Empty reply from server
* Connection #0 to host 127.0.0.1 left intact
curl: (52) Empty reply from server

With httpd-2.4.6 I see an error response in this case and I think it
really should do return something.

Regards,
Jan Kaluza

>>
>> In the log, one can see following:
>>
>> mod_proxy_wstunnel.c(131): (103)Software caused connection abort: [client
>> 127.0.0.1:49915] AH02442: error on sock - ap_get_brigade
>>
>> Attached patch against 2.4.x fixes it. I'm not committing it, because this
>> problem has been introduced in r1493741 and seems like intentional thing.
>> This commit has been reverted in r1605946, so my theory is that this
>> particular part of mod_proxy_wstunnel has not been reverted completely, but
>> I want to be sure before I commit/propose.
>
> One the Upgrade is done, I don't think we can respond with 500 (in the
> poll()ing phase, this is no more HTTP).
> AFAICT r1605946 did nor revert r1493741, and I think this rather comes
> for https://bz.apache.org/bugzilla/show_bug.cgi?id=56299#c7.
>
> Regards,
> Yann.
>

Reply | Threaded
Open this post in threaded view
|

Re: mod_proxy_wstunnel ignores proxy_wstunnel_transfer errors

Yann Ylavic
On Tue, Mar 17, 2015 at 1:47 PM, Jan Kaluža <[hidden email]> wrote:

> On 03/17/2015 01:23 PM, Yann Ylavic wrote:
>>
>> On Tue, Mar 17, 2015 at 12:38 PM, Jan Kaluža <[hidden email]> wrote:
>>>
>>> Hi,
>>>
>>> I have found out that when WSS is used and SSL handshake fails, httpd
>>> closes
>>> client connection without any response to the client.
>>
>>
>> If the SSL handshake fails, there is no SSL established connection
>> which we can send an HTTP response on.
>> We can only send an SSL alert in this case, and I think mod_ssl takes
>> care of this already (this occurs while reading the request header,
>> before mod_proxy_wstunnel IMHO).
>
>
> Hm, maybe I described it wrongly. What I see here is "Empty response from
> server"

Sorry, you were obviously talking about SSL handshake with the backend...

> when I do following:
>
> 1. Use this configuration:
>
> ProxyTimeout 2
> SSLProxyEngine on
> <Location /test/>
>     ProxyPass https://localhost:8080/
>     ProxyPassReverse https://localhost:8080/
>     ProxyPass wss://localhost:8080/
>     ProxyPassReverse wss://localhost:8080/
> </Location>
>
>
> 2. nc -l 8080 < /dev/null
>
> 3. curl -v --insecure https://127.0.0.1/test/
> (...)
>> GET /test/ HTTP/1.1
>> User-Agent: curl/7.29.0
>> Host: 127.0.0.1
>> Accept: */*
>>
> * Empty reply from server
> * Connection #0 to host 127.0.0.1 left intact
> curl: (52) Empty reply from server
>
> With httpd-2.4.6 I see an error response in this case and I think it really
> should do return something.

I see now, the handshake failure indeed occurs in the poll()ing loop
when the first packets are read/send from/to the backend.
But still once the connection is Upgrade-d, it is quite application
specific whether or not an HTTP response should be sent to the client,
and when (only if nothing has been sent already, anytime?). IOW, what
would the backend do if it fails after the Upgrade has been
negociated?

Regards,
Yann.
Reply | Threaded
Open this post in threaded view
|

Re: mod_proxy_wstunnel ignores proxy_wstunnel_transfer errors

Eric Covener
On Tue, Mar 17, 2015 at 9:06 AM, Yann Ylavic <[hidden email]> wrote:
>>> GET /test/ HTTP/1.1
>>> User-Agent: curl/7.29.0
>>> Host: 127.0.0.1
>>> Accept: */*
>>>

No Upgrade header in this test?
Reply | Threaded
Open this post in threaded view
|

Re: mod_proxy_wstunnel ignores proxy_wstunnel_transfer errors

Jan Kaluža
On 03/17/2015 02:10 PM, Eric Covener wrote:
> On Tue, Mar 17, 2015 at 9:06 AM, Yann Ylavic <[hidden email]> wrote:
>>>> GET /test/ HTTP/1.1
>>>> User-Agent: curl/7.29.0
>>>> Host: 127.0.0.1
>>>> Accept: */*
>>>>
>
> No Upgrade header in this test?

Right, no Upgrade header. That's the particular situation where one of
our httpd tests fail. I've already tried even with Upgrade header and it
returns empty response in this case too.


Reply | Threaded
Open this post in threaded view
|

Re: mod_proxy_wstunnel ignores proxy_wstunnel_transfer errors

Jan Kaluža
In reply to this post by Yann Ylavic
On 03/17/2015 02:06 PM, Yann Ylavic wrote:

> On Tue, Mar 17, 2015 at 1:47 PM, Jan Kaluža <[hidden email]> wrote:
>> On 03/17/2015 01:23 PM, Yann Ylavic wrote:
>>>
>>> On Tue, Mar 17, 2015 at 12:38 PM, Jan Kaluža <[hidden email]> wrote:
>>>>
>>>> Hi,
>>>>
>>>> I have found out that when WSS is used and SSL handshake fails, httpd
>>>> closes
>>>> client connection without any response to the client.
>>>
>>>
>>> If the SSL handshake fails, there is no SSL established connection
>>> which we can send an HTTP response on.
>>> We can only send an SSL alert in this case, and I think mod_ssl takes
>>> care of this already (this occurs while reading the request header,
>>> before mod_proxy_wstunnel IMHO).
>>
>>
>> Hm, maybe I described it wrongly. What I see here is "Empty response from
>> server"
>
> Sorry, you were obviously talking about SSL handshake with the backend...
>
>> when I do following:
>>
>> 1. Use this configuration:
>>
>> ProxyTimeout 2
>> SSLProxyEngine on
>> <Location /test/>
>>      ProxyPass https://localhost:8080/
>>      ProxyPassReverse https://localhost:8080/
>>      ProxyPass wss://localhost:8080/
>>      ProxyPassReverse wss://localhost:8080/
>> </Location>
>>
>>
>> 2. nc -l 8080 < /dev/null
>>
>> 3. curl -v --insecure https://127.0.0.1/test/
>> (...)
>>> GET /test/ HTTP/1.1
>>> User-Agent: curl/7.29.0
>>> Host: 127.0.0.1
>>> Accept: */*
>>>
>> * Empty reply from server
>> * Connection #0 to host 127.0.0.1 left intact
>> curl: (52) Empty reply from server
>>
>> With httpd-2.4.6 I see an error response in this case and I think it really
>> should do return something.
>
> I see now, the handshake failure indeed occurs in the poll()ing loop
> when the first packets are read/send from/to the backend.
> But still once the connection is Upgrade-d, it is quite application
> specific whether or not an HTTP response should be sent to the client,
> and when (only if nothing has been sent already, anytime?). IOW, what
> would the backend do if it fails after the Upgrade has been
> negociated?

I have no big knowledge of WebSockets, but it should be possible to
detect Switching Protocol header and return HTTP error if some error
happens before we switch to WebSocket.

Would this be acceptable, or you think this empty reply is not worth
fixing this way?

> Regards,
> Yann.
>

Regards,
Jan Kaluza

Reply | Threaded
Open this post in threaded view
|

Re: mod_proxy_wstunnel ignores proxy_wstunnel_transfer errors

Yann Ylavic
On Wed, Mar 18, 2015 at 8:07 AM, Jan Kaluža <[hidden email]> wrote:
>
> I have no big knowledge of WebSockets, but it should be possible to detect
> Switching Protocol header and return HTTP error if some error happens before
> we switch to WebSocket.
>
> Would this be acceptable, or you think this empty reply is not worth fixing
> this way?

The problem is that at this point (poll() loop), "101 Switching
Protocol" is already sent to the backend, and hence the
ap_proxy_pass_brigade() doing so in proxy_wstunnel_request() should
already have failed with the correct status code returned to the
handler.

Why isn't that happening?

>
>> Regards,
>> Yann.
>>
Reply | Threaded
Open this post in threaded view
|

Re: mod_proxy_wstunnel ignores proxy_wstunnel_transfer errors

Jan Kaluža
On 03/18/2015 09:23 AM, Yann Ylavic wrote:

> On Wed, Mar 18, 2015 at 8:07 AM, Jan Kaluža <[hidden email]> wrote:
>>
>> I have no big knowledge of WebSockets, but it should be possible to detect
>> Switching Protocol header and return HTTP error if some error happens before
>> we switch to WebSocket.
>>
>> Would this be acceptable, or you think this empty reply is not worth fixing
>> this way?
>
> The problem is that at this point (poll() loop), "101 Switching
> Protocol" is already sent to the backend, and hence the
> ap_proxy_pass_brigade() doing so in proxy_wstunnel_request() should
> already have failed with the correct status code returned to the
> handler.

Hm, I think I'm little bit confused how it's supposed to work. I thought
mod_proxy_wstunnel appends Upgrade and Connection header to the
request's headers, sends it to the backend and then the backend responds
with "101 Switching Protocols".

But when reading the code now, it would mean that "101 Switch Protocols"
is forwarded to the client, which, I think, should not happen.

Well, I will test it more with some real websocket server as a backend
to fully understand how it works.

> Why isn't that happening?
>
>>
>>> Regards,
>>> Yann.
>>>

Regards,
Jan Kaluza

Reply | Threaded
Open this post in threaded view
|

Re: mod_proxy_wstunnel ignores proxy_wstunnel_transfer errors

Yann Ylavic
On Wed, Mar 18, 2015 at 9:48 AM, Jan Kaluža <[hidden email]> wrote:

> On 03/18/2015 09:23 AM, Yann Ylavic wrote:
>>
>> On Wed, Mar 18, 2015 at 8:07 AM, Jan Kaluža <[hidden email]> wrote:
>>>
>>>
>>> I have no big knowledge of WebSockets, but it should be possible to
>>> detect
>>> Switching Protocol header and return HTTP error if some error happens
>>> before
>>> we switch to WebSocket.
>>>
>>> Would this be acceptable, or you think this empty reply is not worth
>>> fixing
>>> this way?
>>
>>
>> The problem is that at this point (poll() loop), "101 Switching
>> Protocol" is already sent to the backend, and hence the
>> ap_proxy_pass_brigade() doing so in proxy_wstunnel_request() should
>> already have failed with the correct status code returned to the
>> handler.
>
>
> Hm, I think I'm little bit confused how it's supposed to work. I thought
> mod_proxy_wstunnel appends Upgrade and Connection header to the request's
> headers, sends it to the backend and then the backend responds with "101
> Switching Protocols".

Yes, this is what's supposed to be done, with all but the backend's
response happening before the poll() loop.
That's why I wonder why, before that, there is no failure when the
request (with appended Upgrade and Connection headers) is sent to the
backend.

>
> But when reading the code now, it would mean that "101 Switch Protocols" is
> forwarded to the client, which, I think, should not happen.

This is not happening, ap_proxy_pass_brigade() forwards to the backend.
Reply | Threaded
Open this post in threaded view
|

Re: mod_proxy_wstunnel ignores proxy_wstunnel_transfer errors

Jan Kaluža
On 03/18/2015 10:01 AM, Yann Ylavic wrote:

> On Wed, Mar 18, 2015 at 9:48 AM, Jan Kaluža <[hidden email]> wrote:
>> On 03/18/2015 09:23 AM, Yann Ylavic wrote:
>>>
>>> On Wed, Mar 18, 2015 at 8:07 AM, Jan Kaluža <[hidden email]> wrote:
>>>>
>>>>
>>>> I have no big knowledge of WebSockets, but it should be possible to
>>>> detect
>>>> Switching Protocol header and return HTTP error if some error happens
>>>> before
>>>> we switch to WebSocket.
>>>>
>>>> Would this be acceptable, or you think this empty reply is not worth
>>>> fixing
>>>> this way?
>>>
>>>
>>> The problem is that at this point (poll() loop), "101 Switching
>>> Protocol" is already sent to the backend, and hence the
>>> ap_proxy_pass_brigade() doing so in proxy_wstunnel_request() should
>>> already have failed with the correct status code returned to the
>>> handler.
>>
>>
>> Hm, I think I'm little bit confused how it's supposed to work. I thought
>> mod_proxy_wstunnel appends Upgrade and Connection header to the request's
>> headers, sends it to the backend and then the backend responds with "101
>> Switching Protocols".
>
> Yes, this is what's supposed to be done, with all but the backend's
> response happening before the poll() loop.
> That's why I wonder why, before that, there is no failure when the
> request (with appended Upgrade and Connection headers) is sent to the
> backend.

What's going on here is that backend does not reply anything.
proxy_wstunnel_transfer returns APR_EOF without receiving "101 Switching
Protocols" from the backend and without passing anything to the client.
We could check if APR_EOF happens before "101 Switching Protocols" and
send HTTP error to client, right?

>>
>> But when reading the code now, it would mean that "101 Switch Protocols" is
>> forwarded to the client, which, I think, should not happen.
>
> This is not happening, ap_proxy_pass_brigade() forwards to the backend.
>

Reply | Threaded
Open this post in threaded view
|

Re: mod_proxy_wstunnel ignores proxy_wstunnel_transfer errors

Yann Ylavic
On Wed, Mar 18, 2015 at 10:30 AM, Jan Kaluža <[hidden email]> wrote:
>
> What's going on here is that backend does not reply anything.
> proxy_wstunnel_transfer returns APR_EOF without receiving "101 Switching
> Protocols" from the backend and without passing anything to the client.

Hm, so ap_proxy_pass_brigade() succeeded, strange...

> We
> could check if APR_EOF happens before "101 Switching Protocols" and send
> HTTP error to client, right?

BTW, this is still possible that the Upgrade succeeds (I did not
expect it in your case because of the very beginning SSL handshake,
though), but if reading the first bytes of the response fails later,
we won't send anything to the client, which I agree is bad.

So a patch is probably needed to handle this case, but I think it
would have to detect whether or not some bytes have already been sent
to the client, we don't want to send multiple responses headers (the
one from the backend, and our if we return an error from the handler
after that).

So maybe something like this?

Index: modules/proxy/mod_proxy_wstunnel.c
===================================================================
--- modules/proxy/mod_proxy_wstunnel.c    (revision 1665828)
+++ modules/proxy/mod_proxy_wstunnel.c    (working copy)
@@ -91,7 +91,8 @@ static int proxy_wstunnel_canon(request_rec *r, ch


 static int proxy_wstunnel_transfer(request_rec *r, conn_rec *c_i,
conn_rec *c_o,
-                                     apr_bucket_brigade *bb, char *name)
+                                     apr_bucket_brigade *bb, char *name,
+                                     int *sent)
 {
     int rv;
 #ifdef DEBUGGING
@@ -125,6 +126,9 @@ static int proxy_wstunnel_transfer(request_rec *r,
                               "error on %s - ap_pass_brigade",
                               name);
             }
+            if (sent) {
+                *sent = 1;
+            }
         } else if (!APR_STATUS_IS_EAGAIN(rv) && !APR_STATUS_IS_EOF(rv)) {
             ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv, r, APLOGNO(02442)
                           "error on %s - ap_get_brigade",
@@ -167,6 +171,7 @@ static int proxy_wstunnel_request(apr_pool_t *p, r
     char *old_te_val = NULL;
     apr_bucket_brigade *bb = apr_brigade_create(p, c->bucket_alloc);
     apr_socket_t *client_socket = ap_get_conn_socket(c);
+    int request_sent = 0, response_sent = 0;

     header_brigade = apr_brigade_create(p, backconn->bucket_alloc);

@@ -244,7 +249,8 @@ static int proxy_wstunnel_request(apr_pool_t *p, r
                 if (pollevent & (APR_POLLIN | APR_POLLHUP)) {
                     ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, r,
APLOGNO(02446)
                                   "sock was readable");
-                    rv = proxy_wstunnel_transfer(r, backconn, c, bb, "sock");
+                    rv = proxy_wstunnel_transfer(r, backconn, c, bb, "sock",
+                                                 &response_sent);
                 }
                 else if (pollevent & APR_POLLERR) {
                     rv = APR_EPIPE;
@@ -263,7 +269,8 @@ static int proxy_wstunnel_request(apr_pool_t *p, r
                 if (pollevent & (APR_POLLIN | APR_POLLHUP)) {
                     ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, r,
APLOGNO(02448)
                                   "client was readable");
-                    rv = proxy_wstunnel_transfer(r, c, backconn, bb, "client");
+                    rv = proxy_wstunnel_transfer(r, c, backconn, bb, "client",
+                                                 &request_sent);
                 }
                 else if (pollevent & APR_POLLERR) {
                     rv = APR_EPIPE;
@@ -292,7 +299,15 @@ static int proxy_wstunnel_request(apr_pool_t *p, r
     ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, r,
                   "finished with poll() - cleaning up");

-    return OK;
+    if (!response_sent) {
+        return HTTP_BAD_GATEWAY;
+    }
+    else if (!request_sent) {
+        return HTTP_BAD_REQUEST;
+    }
+    else {
+        return OK;
+    }
 }

 /*
--

Also, maybe mod_proxy_wstunnel should DECLINE[D] the request if it is
not an "Upgrade: WebSocket" one?

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);

--

Regards,
Yann.
Reply | Threaded
Open this post in threaded view
|

Re: mod_proxy_wstunnel ignores proxy_wstunnel_transfer errors

Yann Ylavic
On Wed, Mar 18, 2015 at 10:44 AM, Yann Ylavic <[hidden email]> wrote:
>
[]
> Index: modules/proxy/mod_proxy_wstunnel.c
> ===================================================================
> --- modules/proxy/mod_proxy_wstunnel.c    (revision 1665828)
> +++ modules/proxy/mod_proxy_wstunnel.c    (working copy)
[]

> @@ -292,7 +299,15 @@ static int proxy_wstunnel_request(apr_pool_t *p, r
>      ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, r,
>                    "finished with poll() - cleaning up");
>
> -    return OK;
> +    if (!response_sent) {
> +        return HTTP_BAD_GATEWAY;
> +    }
> +    else if (!request_sent) {
> +        return HTTP_BAD_REQUEST;

This case is probably not a good idea, so we'd better not handle
request_sent at all (only response_sent), and use NULL instead for
proxy_wstunnel_transfer().

> +    }
> +    else {
> +        return OK;
> +    }
>  }
>
>  /*
> --
Reply | Threaded
Open this post in threaded view
|

Re: mod_proxy_wstunnel ignores proxy_wstunnel_transfer errors

Yann Ylavic
Corresponding patch attached...

On Wed, Mar 18, 2015 at 10:57 AM, Yann Ylavic <[hidden email]> wrote:

> On Wed, Mar 18, 2015 at 10:44 AM, Yann Ylavic <[hidden email]> wrote:
>>
> []
>> Index: modules/proxy/mod_proxy_wstunnel.c
>> ===================================================================
>> --- modules/proxy/mod_proxy_wstunnel.c    (revision 1665828)
>> +++ modules/proxy/mod_proxy_wstunnel.c    (working copy)
> []
>> @@ -292,7 +299,15 @@ static int proxy_wstunnel_request(apr_pool_t *p, r
>>      ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, r,
>>                    "finished with poll() - cleaning up");
>>
>> -    return OK;
>> +    if (!response_sent) {
>> +        return HTTP_BAD_GATEWAY;
>> +    }
>> +    else if (!request_sent) {
>> +        return HTTP_BAD_REQUEST;
>
> This case is probably not a good idea, so we'd better not handle
> request_sent at all (only response_sent), and use NULL instead for
> proxy_wstunnel_transfer().
>
>> +    }
>> +    else {
>> +        return OK;
>> +    }
>>  }
>>
>>  /*
>> --

httpd-2.4.x-mod_proxy_wstunnel-response_sent.patch (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: mod_proxy_wstunnel ignores proxy_wstunnel_transfer errors

Jan Kaluža
On 03/18/2015 11:07 AM, Yann Ylavic wrote:

> Corresponding patch attached...
>
> On Wed, Mar 18, 2015 at 10:57 AM, Yann Ylavic <[hidden email]> wrote:
>> On Wed, Mar 18, 2015 at 10:44 AM, Yann Ylavic <[hidden email]> wrote:
>>>
>> []
>>> Index: modules/proxy/mod_proxy_wstunnel.c
>>> ===================================================================
>>> --- modules/proxy/mod_proxy_wstunnel.c    (revision 1665828)
>>> +++ modules/proxy/mod_proxy_wstunnel.c    (working copy)
>> []
>>> @@ -292,7 +299,15 @@ static int proxy_wstunnel_request(apr_pool_t *p, r
>>>       ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, r,
>>>                     "finished with poll() - cleaning up");
>>>
>>> -    return OK;
>>> +    if (!response_sent) {
>>> +        return HTTP_BAD_GATEWAY;
>>> +    }
>>> +    else if (!request_sent) {
>>> +        return HTTP_BAD_REQUEST;
>>
>> This case is probably not a good idea, so we'd better not handle
>> request_sent at all (only response_sent), and use NULL instead for
>> proxy_wstunnel_transfer().

+1 for the last patch. It fixes the bug I see.

Jan Kaluza

>>> +    }
>>> +    else {
>>> +        return OK;
>>> +    }
>>>   }
>>>
>>>   /*
>>> --

Reply | Threaded
Open this post in threaded view
|

Re: mod_proxy_wstunnel ignores proxy_wstunnel_transfer errors

Yann Ylavic
On Wed, Mar 18, 2015 at 12:10 PM, Jan Kaluža <[hidden email]> wrote:
>
> +1 for the last patch. It fixes the bug I see.

Thanks, committed in r1669299.

Regards,
Yann.