On Wed, Mar 18, 2015 at 10:30 AM, Jan Kaluža <[email protected]> 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.
