Re: relayd websocket
ok benno@ Reyk Floeter(r...@openbsd.org) on 2019.05.08 20:35:46 +0200: > On Wed, May 08, 2019 at 07:07:43PM +0200, Reyk Floeter wrote: > > On Wed, May 08, 2019 at 06:26:45PM +0200, Reyk Floeter wrote: > > > On Wed, Mar 06, 2019 at 05:36:32PM +0100, Sebastian Benoit wrote: > > > > Rivo Nurges(rivo.nur...@smit.ee) on 2019.03.05 22:42:13 +: > > > > > Hi! > > > > > > > > > > On 3/5/19 10:36 PM, Claudio Jeker wrote: > > > > > > I guess that this would need strcasestr() instead of strcasecmp(), > > > > > > since you > > > > > > are looking for the substring "Upgrade" in value. Maybe more is > > > > > > needed if > > > > > > we want to be sure that 'Connection: Upgrade-maybe' does not match. > > > > > > > > > > You are correct about strcasestr. "Connection: Upgrade-maybe" would > > > > > need > > > > > to have correct "Upgrade: websocket". Anyway, lets be strict. > > > > > > > > > > Does something like this make sense? > > > > > > > > i think the seperator list needs to include '\t' > > > > because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB. > > > > > > > > And i dont think you can mix "," with " \t" seperators, > > > > because otherwise "Foo Upgrade, Bar" will match. > > > > > > > > Something more is needed to parse elements of a header. > > > > > > > > > > I would reshuffle the websocket handling a bit as I don't think that > > > we need the http priv struct. We can lookup the parsed headers later. > > > > > > The attached diff moves it all into one places and introduces a new > > > function kv_find_value() that is able to to match headers with > > > multiple values (I think we might need it elsewhere. And even if not, > > > I would prefer to have this in a function intead of stuffing it into > > > relay_read_http). > > > > > > A minor question is if the lookup should be done before or after > > > applying the filters (relay_test - my diff does it afterwards, the > > > current code does it before). > > > > > > Tests? Comments? Ok? > > > > > > > I keep on replying to my own diffs... the updated one below uses > > strcasecmp instead of strcasestr in kv_find_value(). There is no need > > for substring- or fn- matching in this function. > > > > Next one... > > benno@ pointed out that the RFC allows whitespace before the comma. > We also don't have to strip \r\n as it is done by relayd's kv parser. > > OK? > > Reyk > > Index: usr.sbin/relayd/http.h > === > RCS file: /cvs/src/usr.sbin/relayd/http.h,v > retrieving revision 1.10 > diff -u -p -u -p -r1.10 http.h > --- usr.sbin/relayd/http.h4 Mar 2019 21:25:03 - 1.10 > +++ usr.sbin/relayd/http.h8 May 2019 18:34:09 - > @@ -251,10 +251,4 @@ struct http_descriptor { > struct kvtreehttp_headers; > }; > > -struct relay_http_priv { > -#define HTTP_CONNECTION_UPGRADE 0x01 > -#define HTTP_UPGRADE_WEBSOCKET 0x02 > - int http_upgrade_req; > -}; > - > #endif /* HTTP_H */ > Index: usr.sbin/relayd/relay.c > === > RCS file: /cvs/src/usr.sbin/relayd/relay.c,v > retrieving revision 1.242 > diff -u -p -u -p -r1.242 relay.c > --- usr.sbin/relayd/relay.c 4 Mar 2019 21:25:03 - 1.242 > +++ usr.sbin/relayd/relay.c 8 May 2019 18:34:09 - > @@ -1410,13 +1410,7 @@ relay_session(struct rsession *con) > return; > } > > - if (rlay->rl_proto->type == RELAY_PROTO_HTTP) { > - if (relay_http_priv_init(con) == -1) { > - relay_close(con, > - "failed to allocate http session data", 1); > - return; > - } > - } else { > + if (rlay->rl_proto->type != RELAY_PROTO_HTTP) { > if (rlay->rl_conf.fwdmode == FWD_TRANS) > relay_bindanyreq(con, 0, IPPROTO_TCP); > else if (relay_connect(con) == -1) { > Index: usr.sbin/relayd/relay_http.c > === > RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v > retrieving revision 1.72 > diff -u -p -u -p -r1.72 relay_http.c > --- usr.sbin/relayd/relay_http.c 4 Mar 2019 21:25:03 - 1.72 > +++ usr.sbin/relayd/relay_http.c 8 May 2019 18:34:09 - > @@ -110,17 +110,6 @@ relay_http_init(struct relay *rlay) > } > > int > -relay_http_priv_init(struct rsession *con) > -{ > - struct relay_http_priv *p; > - if ((p = calloc(1, sizeof(struct relay_http_priv))) == NULL) > - return (-1); > - > - con->se_priv = p; > - return (0); > -} > - > -int > relay_httpdesc_init(struct ctl_relay_event *cre) > { > struct http_descriptor *desc; > @@ -163,13 +152,13 @@ relay_read_http(struct bufferevent *bev, > struct relay*rlay = con->se_relay; > struct protocol *proto = rlay->rl_proto; > struct ev
Re: relayd websocket
Hi! Seems to work fine. Rivo On 2019-05-08 21:37, Reyk Floeter wrote: > On Wed, May 08, 2019 at 07:07:43PM +0200, Reyk Floeter wrote: >> On Wed, May 08, 2019 at 06:26:45PM +0200, Reyk Floeter wrote: >>> On Wed, Mar 06, 2019 at 05:36:32PM +0100, Sebastian Benoit wrote: Rivo Nurges(rivo.nur...@smit.ee) on 2019.03.05 22:42:13 +: > Hi! > > On 3/5/19 10:36 PM, Claudio Jeker wrote: >> I guess that this would need strcasestr() instead of strcasecmp(), since >> you >> are looking for the substring "Upgrade" in value. Maybe more is needed if >> we want to be sure that 'Connection: Upgrade-maybe' does not match. > > You are correct about strcasestr. "Connection: Upgrade-maybe" would need > to have correct "Upgrade: websocket". Anyway, lets be strict. > > Does something like this make sense? i think the seperator list needs to include '\t' because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB. And i dont think you can mix "," with " \t" seperators, because otherwise "Foo Upgrade, Bar" will match. Something more is needed to parse elements of a header. >>> >>> I would reshuffle the websocket handling a bit as I don't think that >>> we need the http priv struct. We can lookup the parsed headers later. >>> >>> The attached diff moves it all into one places and introduces a new >>> function kv_find_value() that is able to to match headers with >>> multiple values (I think we might need it elsewhere. And even if not, >>> I would prefer to have this in a function intead of stuffing it into >>> relay_read_http). >>> >>> A minor question is if the lookup should be done before or after >>> applying the filters (relay_test - my diff does it afterwards, the >>> current code does it before). >>> >>> Tests? Comments? Ok? >>> >> >> I keep on replying to my own diffs... the updated one below uses >> strcasecmp instead of strcasestr in kv_find_value(). There is no need >> for substring- or fn- matching in this function. >> > > Next one... > > benno@ pointed out that the RFC allows whitespace before the comma. > We also don't have to strip \r\n as it is done by relayd's kv parser. > > OK? > > Reyk > > Index: usr.sbin/relayd/http.h > === > RCS file: /cvs/src/usr.sbin/relayd/http.h,v > retrieving revision 1.10 > diff -u -p -u -p -r1.10 http.h > --- usr.sbin/relayd/http.h4 Mar 2019 21:25:03 - 1.10 > +++ usr.sbin/relayd/http.h8 May 2019 18:34:09 - > @@ -251,10 +251,4 @@ struct http_descriptor { > struct kvtreehttp_headers; > }; > > -struct relay_http_priv { > -#define HTTP_CONNECTION_UPGRADE 0x01 > -#define HTTP_UPGRADE_WEBSOCKET 0x02 > - int http_upgrade_req; > -}; > - > #endif /* HTTP_H */ > Index: usr.sbin/relayd/relay.c > === > RCS file: /cvs/src/usr.sbin/relayd/relay.c,v > retrieving revision 1.242 > diff -u -p -u -p -r1.242 relay.c > --- usr.sbin/relayd/relay.c 4 Mar 2019 21:25:03 - 1.242 > +++ usr.sbin/relayd/relay.c 8 May 2019 18:34:09 - > @@ -1410,13 +1410,7 @@ relay_session(struct rsession *con) > return; > } > > - if (rlay->rl_proto->type == RELAY_PROTO_HTTP) { > - if (relay_http_priv_init(con) == -1) { > - relay_close(con, > - "failed to allocate http session data", 1); > - return; > - } > - } else { > + if (rlay->rl_proto->type != RELAY_PROTO_HTTP) { > if (rlay->rl_conf.fwdmode == FWD_TRANS) > relay_bindanyreq(con, 0, IPPROTO_TCP); > else if (relay_connect(con) == -1) { > Index: usr.sbin/relayd/relay_http.c > === > RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v > retrieving revision 1.72 > diff -u -p -u -p -r1.72 relay_http.c > --- usr.sbin/relayd/relay_http.c 4 Mar 2019 21:25:03 - 1.72 > +++ usr.sbin/relayd/relay_http.c 8 May 2019 18:34:09 - > @@ -110,17 +110,6 @@ relay_http_init(struct relay *rlay) > } > > int > -relay_http_priv_init(struct rsession *con) > -{ > - struct relay_http_priv *p; > - if ((p = calloc(1, sizeof(struct relay_http_priv))) == NULL) > - return (-1); > - > - con->se_priv = p; > - return (0); > -} > - > -int > relay_httpdesc_init(struct ctl_relay_event *cre) > { > struct http_descriptor *desc; > @@ -163,13 +152,13 @@ relay_read_http(struct bufferevent *bev, > struct relay*rlay = con->se_relay; > struct protocol *proto = rlay->rl_proto; > struct evbuffer *src = EVBUFFER_INPUT(bev); > - struct relay_http_priv *priv = con->se_priv; > char*line = NULL, *key, *value; >
Re: relayd websocket
On Wed, May 08, 2019 at 07:07:43PM +0200, Reyk Floeter wrote: > On Wed, May 08, 2019 at 06:26:45PM +0200, Reyk Floeter wrote: > > On Wed, Mar 06, 2019 at 05:36:32PM +0100, Sebastian Benoit wrote: > > > Rivo Nurges(rivo.nur...@smit.ee) on 2019.03.05 22:42:13 +: > > > > Hi! > > > > > > > > On 3/5/19 10:36 PM, Claudio Jeker wrote: > > > > > I guess that this would need strcasestr() instead of strcasecmp(), > > > > > since you > > > > > are looking for the substring "Upgrade" in value. Maybe more is > > > > > needed if > > > > > we want to be sure that 'Connection: Upgrade-maybe' does not match. > > > > > > > > You are correct about strcasestr. "Connection: Upgrade-maybe" would > > > > need > > > > to have correct "Upgrade: websocket". Anyway, lets be strict. > > > > > > > > Does something like this make sense? > > > > > > i think the seperator list needs to include '\t' > > > because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB. > > > > > > And i dont think you can mix "," with " \t" seperators, > > > because otherwise "Foo Upgrade, Bar" will match. > > > > > > Something more is needed to parse elements of a header. > > > > > > > I would reshuffle the websocket handling a bit as I don't think that > > we need the http priv struct. We can lookup the parsed headers later. > > > > The attached diff moves it all into one places and introduces a new > > function kv_find_value() that is able to to match headers with > > multiple values (I think we might need it elsewhere. And even if not, > > I would prefer to have this in a function intead of stuffing it into > > relay_read_http). > > > > A minor question is if the lookup should be done before or after > > applying the filters (relay_test - my diff does it afterwards, the > > current code does it before). > > > > Tests? Comments? Ok? > > > > I keep on replying to my own diffs... the updated one below uses > strcasecmp instead of strcasestr in kv_find_value(). There is no need > for substring- or fn- matching in this function. > Next one... benno@ pointed out that the RFC allows whitespace before the comma. We also don't have to strip \r\n as it is done by relayd's kv parser. OK? Reyk Index: usr.sbin/relayd/http.h === RCS file: /cvs/src/usr.sbin/relayd/http.h,v retrieving revision 1.10 diff -u -p -u -p -r1.10 http.h --- usr.sbin/relayd/http.h 4 Mar 2019 21:25:03 - 1.10 +++ usr.sbin/relayd/http.h 8 May 2019 18:34:09 - @@ -251,10 +251,4 @@ struct http_descriptor { struct kvtreehttp_headers; }; -struct relay_http_priv { -#define HTTP_CONNECTION_UPGRADE0x01 -#define HTTP_UPGRADE_WEBSOCKET 0x02 - int http_upgrade_req; -}; - #endif /* HTTP_H */ Index: usr.sbin/relayd/relay.c === RCS file: /cvs/src/usr.sbin/relayd/relay.c,v retrieving revision 1.242 diff -u -p -u -p -r1.242 relay.c --- usr.sbin/relayd/relay.c 4 Mar 2019 21:25:03 - 1.242 +++ usr.sbin/relayd/relay.c 8 May 2019 18:34:09 - @@ -1410,13 +1410,7 @@ relay_session(struct rsession *con) return; } - if (rlay->rl_proto->type == RELAY_PROTO_HTTP) { - if (relay_http_priv_init(con) == -1) { - relay_close(con, - "failed to allocate http session data", 1); - return; - } - } else { + if (rlay->rl_proto->type != RELAY_PROTO_HTTP) { if (rlay->rl_conf.fwdmode == FWD_TRANS) relay_bindanyreq(con, 0, IPPROTO_TCP); else if (relay_connect(con) == -1) { Index: usr.sbin/relayd/relay_http.c === RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v retrieving revision 1.72 diff -u -p -u -p -r1.72 relay_http.c --- usr.sbin/relayd/relay_http.c4 Mar 2019 21:25:03 - 1.72 +++ usr.sbin/relayd/relay_http.c8 May 2019 18:34:09 - @@ -110,17 +110,6 @@ relay_http_init(struct relay *rlay) } int -relay_http_priv_init(struct rsession *con) -{ - struct relay_http_priv *p; - if ((p = calloc(1, sizeof(struct relay_http_priv))) == NULL) - return (-1); - - con->se_priv = p; - return (0); -} - -int relay_httpdesc_init(struct ctl_relay_event *cre) { struct http_descriptor *desc; @@ -163,13 +152,13 @@ relay_read_http(struct bufferevent *bev, struct relay*rlay = con->se_relay; struct protocol *proto = rlay->rl_proto; struct evbuffer *src = EVBUFFER_INPUT(bev); - struct relay_http_priv *priv = con->se_priv; char*line = NULL, *key, *value; char*urlproto, *host, *path; int action, unique, ret;
Re: relayd websocket
On Wed, May 08, 2019 at 06:26:45PM +0200, Reyk Floeter wrote: > On Wed, Mar 06, 2019 at 05:36:32PM +0100, Sebastian Benoit wrote: > > Rivo Nurges(rivo.nur...@smit.ee) on 2019.03.05 22:42:13 +: > > > Hi! > > > > > > On 3/5/19 10:36 PM, Claudio Jeker wrote: > > > > I guess that this would need strcasestr() instead of strcasecmp(), > > > > since you > > > > are looking for the substring "Upgrade" in value. Maybe more is needed > > > > if > > > > we want to be sure that 'Connection: Upgrade-maybe' does not match. > > > > > > You are correct about strcasestr. "Connection: Upgrade-maybe" would need > > > to have correct "Upgrade: websocket". Anyway, lets be strict. > > > > > > Does something like this make sense? > > > > i think the seperator list needs to include '\t' > > because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB. > > > > And i dont think you can mix "," with " \t" seperators, > > because otherwise "Foo Upgrade, Bar" will match. > > > > Something more is needed to parse elements of a header. > > > > I would reshuffle the websocket handling a bit as I don't think that > we need the http priv struct. We can lookup the parsed headers later. > > The attached diff moves it all into one places and introduces a new > function kv_find_value() that is able to to match headers with > multiple values (I think we might need it elsewhere. And even if not, > I would prefer to have this in a function intead of stuffing it into > relay_read_http). > > A minor question is if the lookup should be done before or after > applying the filters (relay_test - my diff does it afterwards, the > current code does it before). > > Tests? Comments? Ok? > I keep on replying to my own diffs... the updated one below uses strcasecmp instead of strcasestr in kv_find_value(). There is no need for substring- or fn- matching in this function. Reyk Index: usr.sbin/relayd/http.h === RCS file: /cvs/src/usr.sbin/relayd/http.h,v retrieving revision 1.10 diff -u -p -u -p -r1.10 http.h --- usr.sbin/relayd/http.h 4 Mar 2019 21:25:03 - 1.10 +++ usr.sbin/relayd/http.h 8 May 2019 16:55:59 - @@ -251,10 +251,4 @@ struct http_descriptor { struct kvtreehttp_headers; }; -struct relay_http_priv { -#define HTTP_CONNECTION_UPGRADE0x01 -#define HTTP_UPGRADE_WEBSOCKET 0x02 - int http_upgrade_req; -}; - #endif /* HTTP_H */ Index: usr.sbin/relayd/relay.c === RCS file: /cvs/src/usr.sbin/relayd/relay.c,v retrieving revision 1.242 diff -u -p -u -p -r1.242 relay.c --- usr.sbin/relayd/relay.c 4 Mar 2019 21:25:03 - 1.242 +++ usr.sbin/relayd/relay.c 8 May 2019 16:56:00 - @@ -1410,13 +1410,7 @@ relay_session(struct rsession *con) return; } - if (rlay->rl_proto->type == RELAY_PROTO_HTTP) { - if (relay_http_priv_init(con) == -1) { - relay_close(con, - "failed to allocate http session data", 1); - return; - } - } else { + if (rlay->rl_proto->type != RELAY_PROTO_HTTP) { if (rlay->rl_conf.fwdmode == FWD_TRANS) relay_bindanyreq(con, 0, IPPROTO_TCP); else if (relay_connect(con) == -1) { Index: usr.sbin/relayd/relay_http.c === RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v retrieving revision 1.72 diff -u -p -u -p -r1.72 relay_http.c --- usr.sbin/relayd/relay_http.c4 Mar 2019 21:25:03 - 1.72 +++ usr.sbin/relayd/relay_http.c8 May 2019 16:56:01 - @@ -110,17 +110,6 @@ relay_http_init(struct relay *rlay) } int -relay_http_priv_init(struct rsession *con) -{ - struct relay_http_priv *p; - if ((p = calloc(1, sizeof(struct relay_http_priv))) == NULL) - return (-1); - - con->se_priv = p; - return (0); -} - -int relay_httpdesc_init(struct ctl_relay_event *cre) { struct http_descriptor *desc; @@ -163,13 +152,13 @@ relay_read_http(struct bufferevent *bev, struct relay*rlay = con->se_relay; struct protocol *proto = rlay->rl_proto; struct evbuffer *src = EVBUFFER_INPUT(bev); - struct relay_http_priv *priv = con->se_priv; char*line = NULL, *key, *value; char*urlproto, *host, *path; int action, unique, ret; const char *errstr; size_t size, linelen; struct kv *hdr = NULL; + struct kv *upgrade = NULL, *upgrade_ws = NULL; getmonotime(&con->se_tv_last); cre->timedout = 0; @@ -398,17 +387,6 @@ relay_read_http(struct buffereven
Re: relayd websocket
On Wed, Mar 06, 2019 at 05:36:32PM +0100, Sebastian Benoit wrote: > Rivo Nurges(rivo.nur...@smit.ee) on 2019.03.05 22:42:13 +: > > Hi! > > > > On 3/5/19 10:36 PM, Claudio Jeker wrote: > > > I guess that this would need strcasestr() instead of strcasecmp(), since > > > you > > > are looking for the substring "Upgrade" in value. Maybe more is needed if > > > we want to be sure that 'Connection: Upgrade-maybe' does not match. > > > > You are correct about strcasestr. "Connection: Upgrade-maybe" would need > > to have correct "Upgrade: websocket". Anyway, lets be strict. > > > > Does something like this make sense? > > i think the seperator list needs to include '\t' > because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB. > > And i dont think you can mix "," with " \t" seperators, > because otherwise "Foo Upgrade, Bar" will match. > > Something more is needed to parse elements of a header. > I would reshuffle the websocket handling a bit as I don't think that we need the http priv struct. We can lookup the parsed headers later. The attached diff moves it all into one places and introduces a new function kv_find_value() that is able to to match headers with multiple values (I think we might need it elsewhere. And even if not, I would prefer to have this in a function intead of stuffing it into relay_read_http). A minor question is if the lookup should be done before or after applying the filters (relay_test - my diff does it afterwards, the current code does it before). Tests? Comments? Ok? Reyk Index: usr.sbin/relayd/http.h === RCS file: /cvs/src/usr.sbin/relayd/http.h,v retrieving revision 1.10 diff -u -p -u -p -r1.10 http.h --- usr.sbin/relayd/http.h 4 Mar 2019 21:25:03 - 1.10 +++ usr.sbin/relayd/http.h 8 May 2019 16:17:38 - @@ -251,10 +251,4 @@ struct http_descriptor { struct kvtreehttp_headers; }; -struct relay_http_priv { -#define HTTP_CONNECTION_UPGRADE0x01 -#define HTTP_UPGRADE_WEBSOCKET 0x02 - int http_upgrade_req; -}; - #endif /* HTTP_H */ Index: usr.sbin/relayd/relay.c === RCS file: /cvs/src/usr.sbin/relayd/relay.c,v retrieving revision 1.242 diff -u -p -u -p -r1.242 relay.c --- usr.sbin/relayd/relay.c 4 Mar 2019 21:25:03 - 1.242 +++ usr.sbin/relayd/relay.c 8 May 2019 16:17:39 - @@ -1410,13 +1410,7 @@ relay_session(struct rsession *con) return; } - if (rlay->rl_proto->type == RELAY_PROTO_HTTP) { - if (relay_http_priv_init(con) == -1) { - relay_close(con, - "failed to allocate http session data", 1); - return; - } - } else { + if (rlay->rl_proto->type != RELAY_PROTO_HTTP) { if (rlay->rl_conf.fwdmode == FWD_TRANS) relay_bindanyreq(con, 0, IPPROTO_TCP); else if (relay_connect(con) == -1) { Index: usr.sbin/relayd/relay_http.c === RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v retrieving revision 1.72 diff -u -p -u -p -r1.72 relay_http.c --- usr.sbin/relayd/relay_http.c4 Mar 2019 21:25:03 - 1.72 +++ usr.sbin/relayd/relay_http.c8 May 2019 16:17:40 - @@ -110,17 +110,6 @@ relay_http_init(struct relay *rlay) } int -relay_http_priv_init(struct rsession *con) -{ - struct relay_http_priv *p; - if ((p = calloc(1, sizeof(struct relay_http_priv))) == NULL) - return (-1); - - con->se_priv = p; - return (0); -} - -int relay_httpdesc_init(struct ctl_relay_event *cre) { struct http_descriptor *desc; @@ -163,13 +152,13 @@ relay_read_http(struct bufferevent *bev, struct relay*rlay = con->se_relay; struct protocol *proto = rlay->rl_proto; struct evbuffer *src = EVBUFFER_INPUT(bev); - struct relay_http_priv *priv = con->se_priv; char*line = NULL, *key, *value; char*urlproto, *host, *path; int action, unique, ret; const char *errstr; size_t size, linelen; struct kv *hdr = NULL; + struct kv *upgrade = NULL, *upgrade_ws = NULL; getmonotime(&con->se_tv_last); cre->timedout = 0; @@ -398,17 +387,6 @@ relay_read_http(struct bufferevent *bev, unique = 0; if (cre->line != 1) { - if (cre->dir == RELAY_DIR_REQUEST) { - if (strcasecmp("Connection", key) == 0 && - strcasecmp("Upgrade", value) == 0) - priv->h
Re: relayd websocket
Hi! Anyone? Rivo On 2019-03-12 21:50, Rivo Nurges wrote: > Hi! > > Bump... > > Rivo > > On 3/6/19 10:57 PM, Rivo Nurges wrote: >> Hi! >> >> >> On 3/6/19 10:20 PM, Rivo Nurges wrote: >>> On 3/6/19 6:36 PM, Sebastian Benoit wrote: > Does something like this make sense? i think the seperator list needs to include '\t' because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB. And i dont think you can mix "," with " \t" seperators, because otherwise "Foo Upgrade, Bar" will match. Something more is needed to parse elements of a header. >>> >>> Oh yeah. I'll work on that. >> >> So here comes the next version. Works with both spaces and tabs. >> >> Index: usr.sbin/relayd/relay_http.c >> === >> RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v >> retrieving revision 1.72 >> diff -u -p -r1.72 relay_http.c >> --- usr.sbin/relayd/relay_http.c 4 Mar 2019 21:25:03 - 1.72 >> +++ usr.sbin/relayd/relay_http.c 6 Mar 2019 20:53:59 - >> @@ -36,6 +36,7 @@ >> #include >> #include >> #include >> +#include >> >> #include "relayd.h" >> #include "http.h" >> @@ -166,6 +167,7 @@ relay_read_http(struct bufferevent *bev, >> struct relay_http_priv *priv = con->se_priv; >> char*line = NULL, *key, *value; >> char*urlproto, *host, *path; >> +char*valuecopy, *valuepart; >> int action, unique, ret; >> const char *errstr; >> size_t size, linelen; >> @@ -399,10 +401,19 @@ relay_read_http(struct bufferevent *bev, >> >> if (cre->line != 1) { >> if (cre->dir == RELAY_DIR_REQUEST) { >> -if (strcasecmp("Connection", key) == 0 && >> -strcasecmp("Upgrade", value) == 0) >> -priv->http_upgrade_req |= >> -HTTP_CONNECTION_UPGRADE; >> +if (strcasecmp("Connection", key) == 0) { >> +valuecopy = strdup(value); >> +while ((valuepart = strsep(&valuecopy, >> +",")) != NULL) { >> +while (isblank(*valuepart)) >> +valuepart = &valuepart[1]; >> +if (strcasecmp("Upgrade", valuepart) >> +== 0) >> +priv->http_upgrade_req |= >> +HTTP_CONNECTION_UPGRADE; >> +} >> +free(valuecopy); >> +} >> if (strcasecmp("Upgrade", key) == 0 && >> strcasecmp("websocket", value) == 0) >> priv->http_upgrade_req |= >> >> >> begin-base64 644 websocket3.diff >> SW5kZXg6IHVzci5zYmluL3JlbGF5ZC9yZWxheV9odHRwLmMKPT09PT09PT09PT09PT09PT09PT09 >> PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQpSQ1MgZmlsZTog >> L2N2cy9zcmMvdXNyLnNiaW4vcmVsYXlkL3JlbGF5X2h0dHAuYyx2CnJldHJpZXZpbmcgcmV2aXNp >> b24gMS43MgpkaWZmIC11IC1wIC1yMS43MiByZWxheV9odHRwLmMKLS0tIHVzci5zYmluL3JlbGF5 >> ZC9yZWxheV9odHRwLmMJNCBNYXIgMjAxOSAyMToyNTowMyAtMDAwMAkxLjcyCisrKyB1c3Iuc2Jp >> bi9yZWxheWQvcmVsYXlfaHR0cC5jCTYgTWFyIDIwMTkgMjA6NTM6NTkgLTAwMDAKQEAgLTM2LDYg >> KzM2LDcgQEAKICNpbmNsdWRlIDxzaXBoYXNoLmg+CiAjaW5jbHVkZSA8aW1zZy5oPgogI2luY2x1 >> ZGUgPHVuaXN0ZC5oPgorI2luY2x1ZGUgPGN0eXBlLmg+CiAKICNpbmNsdWRlICJyZWxheWQuaCIK >> ICNpbmNsdWRlICJodHRwLmgiCkBAIC0xNjYsNiArMTY3LDcgQEAgcmVsYXlfcmVhZF9odHRwKHN0 >> cnVjdCBidWZmZXJldmVudCAqYmV2LAogCXN0cnVjdCByZWxheV9odHRwX3ByaXYJKnByaXYgPSBj >> b24tPnNlX3ByaXY7CiAJY2hhcgkJCSpsaW5lID0gTlVMTCwgKmtleSwgKnZhbHVlOwogCWNoYXIJ >> CQkqdXJscHJvdG8sICpob3N0LCAqcGF0aDsKKwljaGFyCQkJKnZhbHVlY29weSwgKnZhbHVlcGFy >> dDsKIAlpbnQJCQkgYWN0aW9uLCB1bmlxdWUsIHJldDsKIAljb25zdCBjaGFyCQkqZXJyc3RyOwog >> CXNpemVfdAkJCSBzaXplLCBsaW5lbGVuOwpAQCAtMzk5LDEwICs0MDEsMTkgQEAgcmVsYXlfcmVh >> ZF9odHRwKHN0cnVjdCBidWZmZXJldmVudCAqYmV2LAogCiAJCWlmIChjcmUtPmxpbmUgIT0gMSkg >> ewogCQkJaWYgKGNyZS0+ZGlyID09IFJFTEFZX0RJUl9SRVFVRVNUKSB7Ci0JCQkJaWYgKHN0cmNh >> c2VjbXAoIkNvbm5lY3Rpb24iLCBrZXkpID09IDAgJiYKLQkJCQkgICAgc3RyY2FzZWNtcCgiVXBn >> cmFkZSIsIHZhbHVlKSA9PSAwKQotCQkJCQlwcml2LT5odHRwX3VwZ3JhZGVfcmVxIHw9Ci0JCQkJ >> CSAgICBIVFRQX0NPTk5FQ1RJT05fVVBHUkFERTsKKwkJCQlpZiAoc3RyY2FzZWNtcCgiQ29ubmVj >> dGlvbiIsIGtleSkgPT0gMCkgeworCQkJCSAgICB2YWx1ZWNvcHkgPSBzdHJkdXAodmFsdWUpOwor >> CQkJCSAgICB3aGlsZSAoKHZhbHVlcGFydCA9IHN0cnNlcCgmdmFsdWVjb3B5LAorCQkJCQkiLCIp >> KSAhPSBOVUxMKSB7CisJCQkJCXdoaWxlIChpc2JsYW5rKCp2YWx1ZXBhcnQpKQorCQkJCQkgICAg >> dmFsdWVwYXJ0ID0gJnZhbHVlcGFydFsxXTsKKwkJCQkgICAgCWlmIChzdHJjYXNlY21wKCJVcGdy >> YW
Re: relayd websocket
Hi! Bump... Rivo On 3/6/19 10:57 PM, Rivo Nurges wrote: > Hi! > > > On 3/6/19 10:20 PM, Rivo Nurges wrote: >> On 3/6/19 6:36 PM, Sebastian Benoit wrote: Does something like this make sense? >>> >>> i think the seperator list needs to include '\t' >>> because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB. >>> >>> And i dont think you can mix "," with " \t" seperators, >>> because otherwise "Foo Upgrade, Bar" will match. >>> >>> Something more is needed to parse elements of a header. >> >> Oh yeah. I'll work on that. > > So here comes the next version. Works with both spaces and tabs. > > Index: usr.sbin/relayd/relay_http.c > === > RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v > retrieving revision 1.72 > diff -u -p -r1.72 relay_http.c > --- usr.sbin/relayd/relay_http.c 4 Mar 2019 21:25:03 - 1.72 > +++ usr.sbin/relayd/relay_http.c 6 Mar 2019 20:53:59 - > @@ -36,6 +36,7 @@ >#include >#include >#include > +#include > >#include "relayd.h" >#include "http.h" > @@ -166,6 +167,7 @@ relay_read_http(struct bufferevent *bev, > struct relay_http_priv *priv = con->se_priv; > char*line = NULL, *key, *value; > char*urlproto, *host, *path; > + char*valuecopy, *valuepart; > int action, unique, ret; > const char *errstr; > size_t size, linelen; > @@ -399,10 +401,19 @@ relay_read_http(struct bufferevent *bev, > > if (cre->line != 1) { > if (cre->dir == RELAY_DIR_REQUEST) { > - if (strcasecmp("Connection", key) == 0 && > - strcasecmp("Upgrade", value) == 0) > - priv->http_upgrade_req |= > - HTTP_CONNECTION_UPGRADE; > + if (strcasecmp("Connection", key) == 0) { > + valuecopy = strdup(value); > + while ((valuepart = strsep(&valuecopy, > + ",")) != NULL) { > + while (isblank(*valuepart)) > + valuepart = &valuepart[1]; > + if (strcasecmp("Upgrade", valuepart) > + == 0) > + priv->http_upgrade_req |= > + HTTP_CONNECTION_UPGRADE; > + } > + free(valuecopy); > + } > if (strcasecmp("Upgrade", key) == 0 && > strcasecmp("websocket", value) == 0) > priv->http_upgrade_req |= > > > begin-base64 644 websocket3.diff > SW5kZXg6IHVzci5zYmluL3JlbGF5ZC9yZWxheV9odHRwLmMKPT09PT09PT09PT09PT09PT09PT09 > PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQpSQ1MgZmlsZTog > L2N2cy9zcmMvdXNyLnNiaW4vcmVsYXlkL3JlbGF5X2h0dHAuYyx2CnJldHJpZXZpbmcgcmV2aXNp > b24gMS43MgpkaWZmIC11IC1wIC1yMS43MiByZWxheV9odHRwLmMKLS0tIHVzci5zYmluL3JlbGF5 > ZC9yZWxheV9odHRwLmMJNCBNYXIgMjAxOSAyMToyNTowMyAtMDAwMAkxLjcyCisrKyB1c3Iuc2Jp > bi9yZWxheWQvcmVsYXlfaHR0cC5jCTYgTWFyIDIwMTkgMjA6NTM6NTkgLTAwMDAKQEAgLTM2LDYg > KzM2LDcgQEAKICNpbmNsdWRlIDxzaXBoYXNoLmg+CiAjaW5jbHVkZSA8aW1zZy5oPgogI2luY2x1 > ZGUgPHVuaXN0ZC5oPgorI2luY2x1ZGUgPGN0eXBlLmg+CiAKICNpbmNsdWRlICJyZWxheWQuaCIK > ICNpbmNsdWRlICJodHRwLmgiCkBAIC0xNjYsNiArMTY3LDcgQEAgcmVsYXlfcmVhZF9odHRwKHN0 > cnVjdCBidWZmZXJldmVudCAqYmV2LAogCXN0cnVjdCByZWxheV9odHRwX3ByaXYJKnByaXYgPSBj > b24tPnNlX3ByaXY7CiAJY2hhcgkJCSpsaW5lID0gTlVMTCwgKmtleSwgKnZhbHVlOwogCWNoYXIJ > CQkqdXJscHJvdG8sICpob3N0LCAqcGF0aDsKKwljaGFyCQkJKnZhbHVlY29weSwgKnZhbHVlcGFy > dDsKIAlpbnQJCQkgYWN0aW9uLCB1bmlxdWUsIHJldDsKIAljb25zdCBjaGFyCQkqZXJyc3RyOwog > CXNpemVfdAkJCSBzaXplLCBsaW5lbGVuOwpAQCAtMzk5LDEwICs0MDEsMTkgQEAgcmVsYXlfcmVh > ZF9odHRwKHN0cnVjdCBidWZmZXJldmVudCAqYmV2LAogCiAJCWlmIChjcmUtPmxpbmUgIT0gMSkg > ewogCQkJaWYgKGNyZS0+ZGlyID09IFJFTEFZX0RJUl9SRVFVRVNUKSB7Ci0JCQkJaWYgKHN0cmNh > c2VjbXAoIkNvbm5lY3Rpb24iLCBrZXkpID09IDAgJiYKLQkJCQkgICAgc3RyY2FzZWNtcCgiVXBn > cmFkZSIsIHZhbHVlKSA9PSAwKQotCQkJCQlwcml2LT5odHRwX3VwZ3JhZGVfcmVxIHw9Ci0JCQkJ > CSAgICBIVFRQX0NPTk5FQ1RJT05fVVBHUkFERTsKKwkJCQlpZiAoc3RyY2FzZWNtcCgiQ29ubmVj > dGlvbiIsIGtleSkgPT0gMCkgeworCQkJCSAgICB2YWx1ZWNvcHkgPSBzdHJkdXAodmFsdWUpOwor > CQkJCSAgICB3aGlsZSAoKHZhbHVlcGFydCA9IHN0cnNlcCgmdmFsdWVjb3B5LAorCQkJCQkiLCIp > KSAhPSBOVUxMKSB7CisJCQkJCXdoaWxlIChpc2JsYW5rKCp2YWx1ZXBhcnQpKQorCQkJCQkgICAg > dmFsdWVwYXJ0ID0gJnZhbHVlcGFydFsxXTsKKwkJCQkgICAgCWlmIChzdHJjYXNlY21wKCJVcGdy > YWRlIiwgdmFsdWVwYXJ0KQorCQkJCQkgICAgPT0gMCkKKwkJCQkJICAgIHByaXYtPmh0dHBfdXBn > cmFkZV9yZXEgfD0KKwkJCQkJICAgIAlIVFRQX0NPTk5FQ1RJT05fVVBH
Re: relayd websocket
Hi! On 3/6/19 10:20 PM, Rivo Nurges wrote: > On 3/6/19 6:36 PM, Sebastian Benoit wrote: >>> Does something like this make sense? >> >> i think the seperator list needs to include '\t' >> because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB. >> >> And i dont think you can mix "," with " \t" seperators, >> because otherwise "Foo Upgrade, Bar" will match. >> >> Something more is needed to parse elements of a header. > > Oh yeah. I'll work on that. So here comes the next version. Works with both spaces and tabs. Index: usr.sbin/relayd/relay_http.c === RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v retrieving revision 1.72 diff -u -p -r1.72 relay_http.c --- usr.sbin/relayd/relay_http.c4 Mar 2019 21:25:03 - 1.72 +++ usr.sbin/relayd/relay_http.c6 Mar 2019 20:53:59 - @@ -36,6 +36,7 @@ #include #include #include +#include #include "relayd.h" #include "http.h" @@ -166,6 +167,7 @@ relay_read_http(struct bufferevent *bev, struct relay_http_priv *priv = con->se_priv; char*line = NULL, *key, *value; char*urlproto, *host, *path; + char*valuecopy, *valuepart; int action, unique, ret; const char *errstr; size_t size, linelen; @@ -399,10 +401,19 @@ relay_read_http(struct bufferevent *bev, if (cre->line != 1) { if (cre->dir == RELAY_DIR_REQUEST) { - if (strcasecmp("Connection", key) == 0 && - strcasecmp("Upgrade", value) == 0) - priv->http_upgrade_req |= - HTTP_CONNECTION_UPGRADE; + if (strcasecmp("Connection", key) == 0) { + valuecopy = strdup(value); + while ((valuepart = strsep(&valuecopy, + ",")) != NULL) { + while (isblank(*valuepart)) + valuepart = &valuepart[1]; + if (strcasecmp("Upgrade", valuepart) + == 0) + priv->http_upgrade_req |= + HTTP_CONNECTION_UPGRADE; + } + free(valuecopy); + } if (strcasecmp("Upgrade", key) == 0 && strcasecmp("websocket", value) == 0) priv->http_upgrade_req |= begin-base64 644 websocket3.diff SW5kZXg6IHVzci5zYmluL3JlbGF5ZC9yZWxheV9odHRwLmMKPT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQpSQ1MgZmlsZTog L2N2cy9zcmMvdXNyLnNiaW4vcmVsYXlkL3JlbGF5X2h0dHAuYyx2CnJldHJpZXZpbmcgcmV2aXNp b24gMS43MgpkaWZmIC11IC1wIC1yMS43MiByZWxheV9odHRwLmMKLS0tIHVzci5zYmluL3JlbGF5 ZC9yZWxheV9odHRwLmMJNCBNYXIgMjAxOSAyMToyNTowMyAtMDAwMAkxLjcyCisrKyB1c3Iuc2Jp bi9yZWxheWQvcmVsYXlfaHR0cC5jCTYgTWFyIDIwMTkgMjA6NTM6NTkgLTAwMDAKQEAgLTM2LDYg KzM2LDcgQEAKICNpbmNsdWRlIDxzaXBoYXNoLmg+CiAjaW5jbHVkZSA8aW1zZy5oPgogI2luY2x1 ZGUgPHVuaXN0ZC5oPgorI2luY2x1ZGUgPGN0eXBlLmg+CiAKICNpbmNsdWRlICJyZWxheWQuaCIK ICNpbmNsdWRlICJodHRwLmgiCkBAIC0xNjYsNiArMTY3LDcgQEAgcmVsYXlfcmVhZF9odHRwKHN0 cnVjdCBidWZmZXJldmVudCAqYmV2LAogCXN0cnVjdCByZWxheV9odHRwX3ByaXYJKnByaXYgPSBj b24tPnNlX3ByaXY7CiAJY2hhcgkJCSpsaW5lID0gTlVMTCwgKmtleSwgKnZhbHVlOwogCWNoYXIJ CQkqdXJscHJvdG8sICpob3N0LCAqcGF0aDsKKwljaGFyCQkJKnZhbHVlY29weSwgKnZhbHVlcGFy dDsKIAlpbnQJCQkgYWN0aW9uLCB1bmlxdWUsIHJldDsKIAljb25zdCBjaGFyCQkqZXJyc3RyOwog CXNpemVfdAkJCSBzaXplLCBsaW5lbGVuOwpAQCAtMzk5LDEwICs0MDEsMTkgQEAgcmVsYXlfcmVh ZF9odHRwKHN0cnVjdCBidWZmZXJldmVudCAqYmV2LAogCiAJCWlmIChjcmUtPmxpbmUgIT0gMSkg ewogCQkJaWYgKGNyZS0+ZGlyID09IFJFTEFZX0RJUl9SRVFVRVNUKSB7Ci0JCQkJaWYgKHN0cmNh c2VjbXAoIkNvbm5lY3Rpb24iLCBrZXkpID09IDAgJiYKLQkJCQkgICAgc3RyY2FzZWNtcCgiVXBn cmFkZSIsIHZhbHVlKSA9PSAwKQotCQkJCQlwcml2LT5odHRwX3VwZ3JhZGVfcmVxIHw9Ci0JCQkJ CSAgICBIVFRQX0NPTk5FQ1RJT05fVVBHUkFERTsKKwkJCQlpZiAoc3RyY2FzZWNtcCgiQ29ubmVj dGlvbiIsIGtleSkgPT0gMCkgeworCQkJCSAgICB2YWx1ZWNvcHkgPSBzdHJkdXAodmFsdWUpOwor CQkJCSAgICB3aGlsZSAoKHZhbHVlcGFydCA9IHN0cnNlcCgmdmFsdWVjb3B5LAorCQkJCQkiLCIp KSAhPSBOVUxMKSB7CisJCQkJCXdoaWxlIChpc2JsYW5rKCp2YWx1ZXBhcnQpKQorCQkJCQkgICAg dmFsdWVwYXJ0ID0gJnZhbHVlcGFydFsxXTsKKwkJCQkgICAgCWlmIChzdHJjYXNlY21wKCJVcGdy YWRlIiwgdmFsdWVwYXJ0KQorCQkJCQkgICAgPT0gMCkKKwkJCQkJICAgIHByaXYtPmh0dHBfdXBn cmFkZV9yZXEgfD0KKwkJCQkJICAgIAlIVFRQX0NPTk5FQ1RJT05fVVBHUkFERTsKKwkJCQkgICAg fQorCQkJCSAgICBmcmVlKHZhbHVlY29weSk7CisJCQkJfQogCQkJCWlmIChzdHJjYXNlY21wKCJV cGdyYWRlIiwga2V5KSA9PSAwICYmCiAJCQkJICAgIHN0cmNhc2VjbXAoIndlYnNvY2tldCIsIHZh bH
Re: relayd websocket
On 3/6/19 6:36 PM, Sebastian Benoit wrote: >> Does something like this make sense? > > i think the seperator list needs to include '\t' > because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB. > > And i dont think you can mix "," with " \t" seperators, > because otherwise "Foo Upgrade, Bar" will match. > > Something more is needed to parse elements of a header. Oh yeah. I'll work on that. Rivo
Re: relayd websocket
Rivo Nurges(rivo.nur...@smit.ee) on 2019.03.05 22:42:13 +: > Hi! > > On 3/5/19 10:36 PM, Claudio Jeker wrote: > > I guess that this would need strcasestr() instead of strcasecmp(), since you > > are looking for the substring "Upgrade" in value. Maybe more is needed if > > we want to be sure that 'Connection: Upgrade-maybe' does not match. > > You are correct about strcasestr. "Connection: Upgrade-maybe" would need > to have correct "Upgrade: websocket". Anyway, lets be strict. > > Does something like this make sense? i think the seperator list needs to include '\t' because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB. And i dont think you can mix "," with " \t" seperators, because otherwise "Foo Upgrade, Bar" will match. Something more is needed to parse elements of a header. > Index: usr.sbin/relayd/relay_http.c > === > RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v > retrieving revision 1.72 > diff -u -p -r1.72 relay_http.c > --- usr.sbin/relayd/relay_http.c 4 Mar 2019 21:25:03 - 1.72 > +++ usr.sbin/relayd/relay_http.c 5 Mar 2019 22:33:47 - > @@ -166,6 +166,7 @@ relay_read_http(struct bufferevent *bev, > struct relay_http_priv *priv = con->se_priv; > char*line = NULL, *key, *value; > char*urlproto, *host, *path; > + char*valuecopy, *valuepart; > int action, unique, ret; > const char *errstr; > size_t size, linelen; > @@ -399,10 +400,18 @@ relay_read_http(struct bufferevent *bev, > > if (cre->line != 1) { > if (cre->dir == RELAY_DIR_REQUEST) { > - if (strcasecmp("Connection", key) == 0 && > - strcasecmp("Upgrade", value) == 0) > - priv->http_upgrade_req |= > - HTTP_CONNECTION_UPGRADE; > + > + > + if (strcasecmp("Connection", key) == 0) { > + valuecopy = strdup(value); > + while ((valuepart = strsep(&valuecopy, ", > ")) != NULL) > + if (strcasecmp("Upgrade", valuepart) == > 0) > + priv->http_upgrade_req |= > + HTTP_CONNECTION_UPGRADE; > + free(valuecopy); > + } > + > + > if (strcasecmp("Upgrade", key) == 0 && > strcasecmp("websocket", value) == 0) > priv->http_upgrade_req |= > > > > begin-base64 644 websocket2.diff > SW5kZXg6IHVzci5zYmluL3JlbGF5ZC9yZWxheV9odHRwLmMKPT09PT09PT09PT09PT09PT09PT09 > PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQpSQ1MgZmlsZTog > L2N2cy9zcmMvdXNyLnNiaW4vcmVsYXlkL3JlbGF5X2h0dHAuYyx2CnJldHJpZXZpbmcgcmV2aXNp > b24gMS43MgpkaWZmIC11IC1wIC1yMS43MiByZWxheV9odHRwLmMKLS0tIHVzci5zYmluL3JlbGF5 > ZC9yZWxheV9odHRwLmMJNCBNYXIgMjAxOSAyMToyNTowMyAtMDAwMAkxLjcyCisrKyB1c3Iuc2Jp > bi9yZWxheWQvcmVsYXlfaHR0cC5jCTUgTWFyIDIwMTkgMjI6MzM6NDcgLTAwMDAKQEAgLTE2Niw2 > ICsxNjYsNyBAQCByZWxheV9yZWFkX2h0dHAoc3RydWN0IGJ1ZmZlcmV2ZW50ICpiZXYsCiAJc3Ry > dWN0IHJlbGF5X2h0dHBfcHJpdgkqcHJpdiA9IGNvbi0+c2VfcHJpdjsKIAljaGFyCQkJKmxpbmUg > PSBOVUxMLCAqa2V5LCAqdmFsdWU7CiAJY2hhcgkJCSp1cmxwcm90bywgKmhvc3QsICpwYXRoOwor > CWNoYXIJCQkqdmFsdWVjb3B5LCAqdmFsdWVwYXJ0OwogCWludAkJCSBhY3Rpb24sIHVuaXF1ZSwg > cmV0OwogCWNvbnN0IGNoYXIJCSplcnJzdHI7CiAJc2l6ZV90CQkJIHNpemUsIGxpbmVsZW47CkBA > IC0zOTksMTAgKzQwMCwxOCBAQCByZWxheV9yZWFkX2h0dHAoc3RydWN0IGJ1ZmZlcmV2ZW50ICpi > ZXYsCiAKIAkJaWYgKGNyZS0+bGluZSAhPSAxKSB7CiAJCQlpZiAoY3JlLT5kaXIgPT0gUkVMQVlf > RElSX1JFUVVFU1QpIHsKLQkJCQlpZiAoc3RyY2FzZWNtcCgiQ29ubmVjdGlvbiIsIGtleSkgPT0g > MCAmJgotCQkJCSAgICBzdHJjYXNlY21wKCJVcGdyYWRlIiwgdmFsdWUpID09IDApCi0JCQkJCXBy > aXYtPmh0dHBfdXBncmFkZV9yZXEgfD0KLQkJCQkJICAgIEhUVFBfQ09OTkVDVElPTl9VUEdSQURF > OworCisKKwkJCQlpZiAoc3RyY2FzZWNtcCgiQ29ubmVjdGlvbiIsIGtleSkgPT0gMCkgeworCQkJ > CSAgICB2YWx1ZWNvcHkgPSBzdHJkdXAodmFsdWUpOworCQkJCSAgICB3aGlsZSAoKHZhbHVlcGFy > dCA9IHN0cnNlcCgmdmFsdWVjb3B5LCAiLCAiKSkgIT0gTlVMTCkKKwkJCQkgICAgCWlmIChzdHJj > YXNlY21wKCJVcGdyYWRlIiwgdmFsdWVwYXJ0KSA9PSAwKQorCQkJCQkgICAgcHJpdi0+aHR0cF91 > cGdyYWRlX3JlcSB8PQorCQkJCQkgICAgCUhUVFBfQ09OTkVDVElPTl9VUEdSQURFOworCQkJCSAg > ICBmcmVlKHZhbHVlY29weSk7CisJCQkJfQorCisKIAkJCQlpZiAoc3RyY2FzZWNtcCgiVXBncmFk > ZSIsIGtleSkgPT0gMCAmJgogCQkJCSAgICBzdHJjYXNlY21wKCJ3ZWJzb2NrZXQiLCB2YWx1ZSkg > PT0gMCkKIAkJCQkJcHJpdi0+aHR0cF91cGdyYWRlX3JlcSB8PQo= > > > > > > >
Re: relayd websocket
Hi! On 3/5/19 10:36 PM, Claudio Jeker wrote: > I guess that this would need strcasestr() instead of strcasecmp(), since you > are looking for the substring "Upgrade" in value. Maybe more is needed if > we want to be sure that 'Connection: Upgrade-maybe' does not match. You are correct about strcasestr. "Connection: Upgrade-maybe" would need to have correct "Upgrade: websocket". Anyway, lets be strict. Does something like this make sense? Index: usr.sbin/relayd/relay_http.c === RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v retrieving revision 1.72 diff -u -p -r1.72 relay_http.c --- usr.sbin/relayd/relay_http.c4 Mar 2019 21:25:03 - 1.72 +++ usr.sbin/relayd/relay_http.c5 Mar 2019 22:33:47 - @@ -166,6 +166,7 @@ relay_read_http(struct bufferevent *bev, struct relay_http_priv *priv = con->se_priv; char*line = NULL, *key, *value; char*urlproto, *host, *path; + char*valuecopy, *valuepart; int action, unique, ret; const char *errstr; size_t size, linelen; @@ -399,10 +400,18 @@ relay_read_http(struct bufferevent *bev, if (cre->line != 1) { if (cre->dir == RELAY_DIR_REQUEST) { - if (strcasecmp("Connection", key) == 0 && - strcasecmp("Upgrade", value) == 0) - priv->http_upgrade_req |= - HTTP_CONNECTION_UPGRADE; + + + if (strcasecmp("Connection", key) == 0) { + valuecopy = strdup(value); + while ((valuepart = strsep(&valuecopy, ", ")) != NULL) + if (strcasecmp("Upgrade", valuepart) == 0) + priv->http_upgrade_req |= + HTTP_CONNECTION_UPGRADE; + free(valuecopy); + } + + if (strcasecmp("Upgrade", key) == 0 && strcasecmp("websocket", value) == 0) priv->http_upgrade_req |= begin-base64 644 websocket2.diff SW5kZXg6IHVzci5zYmluL3JlbGF5ZC9yZWxheV9odHRwLmMKPT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQpSQ1MgZmlsZTog L2N2cy9zcmMvdXNyLnNiaW4vcmVsYXlkL3JlbGF5X2h0dHAuYyx2CnJldHJpZXZpbmcgcmV2aXNp b24gMS43MgpkaWZmIC11IC1wIC1yMS43MiByZWxheV9odHRwLmMKLS0tIHVzci5zYmluL3JlbGF5 ZC9yZWxheV9odHRwLmMJNCBNYXIgMjAxOSAyMToyNTowMyAtMDAwMAkxLjcyCisrKyB1c3Iuc2Jp bi9yZWxheWQvcmVsYXlfaHR0cC5jCTUgTWFyIDIwMTkgMjI6MzM6NDcgLTAwMDAKQEAgLTE2Niw2 ICsxNjYsNyBAQCByZWxheV9yZWFkX2h0dHAoc3RydWN0IGJ1ZmZlcmV2ZW50ICpiZXYsCiAJc3Ry dWN0IHJlbGF5X2h0dHBfcHJpdgkqcHJpdiA9IGNvbi0+c2VfcHJpdjsKIAljaGFyCQkJKmxpbmUg PSBOVUxMLCAqa2V5LCAqdmFsdWU7CiAJY2hhcgkJCSp1cmxwcm90bywgKmhvc3QsICpwYXRoOwor CWNoYXIJCQkqdmFsdWVjb3B5LCAqdmFsdWVwYXJ0OwogCWludAkJCSBhY3Rpb24sIHVuaXF1ZSwg cmV0OwogCWNvbnN0IGNoYXIJCSplcnJzdHI7CiAJc2l6ZV90CQkJIHNpemUsIGxpbmVsZW47CkBA IC0zOTksMTAgKzQwMCwxOCBAQCByZWxheV9yZWFkX2h0dHAoc3RydWN0IGJ1ZmZlcmV2ZW50ICpi ZXYsCiAKIAkJaWYgKGNyZS0+bGluZSAhPSAxKSB7CiAJCQlpZiAoY3JlLT5kaXIgPT0gUkVMQVlf RElSX1JFUVVFU1QpIHsKLQkJCQlpZiAoc3RyY2FzZWNtcCgiQ29ubmVjdGlvbiIsIGtleSkgPT0g MCAmJgotCQkJCSAgICBzdHJjYXNlY21wKCJVcGdyYWRlIiwgdmFsdWUpID09IDApCi0JCQkJCXBy aXYtPmh0dHBfdXBncmFkZV9yZXEgfD0KLQkJCQkJICAgIEhUVFBfQ09OTkVDVElPTl9VUEdSQURF OworCisKKwkJCQlpZiAoc3RyY2FzZWNtcCgiQ29ubmVjdGlvbiIsIGtleSkgPT0gMCkgeworCQkJ CSAgICB2YWx1ZWNvcHkgPSBzdHJkdXAodmFsdWUpOworCQkJCSAgICB3aGlsZSAoKHZhbHVlcGFy dCA9IHN0cnNlcCgmdmFsdWVjb3B5LCAiLCAiKSkgIT0gTlVMTCkKKwkJCQkgICAgCWlmIChzdHJj YXNlY21wKCJVcGdyYWRlIiwgdmFsdWVwYXJ0KSA9PSAwKQorCQkJCQkgICAgcHJpdi0+aHR0cF91 cGdyYWRlX3JlcSB8PQorCQkJCQkgICAgCUhUVFBfQ09OTkVDVElPTl9VUEdSQURFOworCQkJCSAg ICBmcmVlKHZhbHVlY29weSk7CisJCQkJfQorCisKIAkJCQlpZiAoc3RyY2FzZWNtcCgiVXBncmFk ZSIsIGtleSkgPT0gMCAmJgogCQkJCSAgICBzdHJjYXNlY21wKCJ3ZWJzb2NrZXQiLCB2YWx1ZSkg PT0gMCkKIAkJCQkJcHJpdi0+aHR0cF91cGdyYWRlX3JlcSB8PQo=
Re: relayd websocket
On Tue, Mar 05, 2019 at 04:21:30PM +, Rivo Nurges wrote: > Hi! > > RFC 6455 4.2.1 states: > 4. A |Connection| header field that *includes* the token "Upgrade", > treated as an ASCII case-insensitive value. > > In my test case Firefox sends: Connection: keep-alive, Upgrade > > Relayd currently expects Connection to equal Upgrade, not include Upgrade. > > I haven't figured out how to configure Thunderbird to send proper diffs, > so I'm sending bas64 encoded version too. > > Index: usr.sbin/relayd/relay_http.c > === > RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v > retrieving revision 1.72 > diff -u -p -r1.72 relay_http.c > --- usr.sbin/relayd/relay_http.c 4 Mar 2019 21:25:03 - 1.72 > +++ usr.sbin/relayd/relay_http.c 5 Mar 2019 16:03:56 - > @@ -400,7 +400,7 @@ relay_read_http(struct bufferevent *bev, > if (cre->line != 1) { > if (cre->dir == RELAY_DIR_REQUEST) { > if (strcasecmp("Connection", key) == 0 && > - strcasecmp("Upgrade", value) == 0) > + strcasecmp("Upgrade", value) >= 0) > priv->http_upgrade_req |= > HTTP_CONNECTION_UPGRADE; > if (strcasecmp("Upgrade", key) == 0 && > I guess that this would need strcasestr() instead of strcasecmp(), since you are looking for the substring "Upgrade" in value. Maybe more is needed if we want to be sure that 'Connection: Upgrade-maybe' does not match. -- :wq Claudio