Clearly the current behaviour with two Connection headers is wrong and this may be a correct diff, but the description accompanying it only considers "how does this fix the problem seen" rather than "what are the implications of this, does it do anything bad?".
I'm only making very minimal use of relayd and IIRC not with relay_http, but the thing this makes me wonder about is, what is the "Connection: close" for? If it's there to prevent clients from smuggling a follow-on request past relayd to the server without going through the filter rules then we need to be sure that dropping the "Connection: close" in this case doesn't bypass that. On 2021/02/16 14:25, Franz Bettag wrote: > At least someone besides me cares about correct protocols in base daemons :] > > Sent from my iPhone > > > On 14 Feb 2021, at 12:06, Marcus MERIGHI <mcmer-open...@tor.at> wrote: > > > > Another month has passed, another friendly bump... > > > > patch against -current attached, for convenience... > > > > Marcus > > > > Index: relay_http.c > > =================================================================== > > RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v > > retrieving revision 1.80 > > diff -u -p -u -r1.80 relay_http.c > > --- relay_http.c 9 Jan 2021 08:53:58 -0000 1.80 > > +++ relay_http.c 14 Feb 2021 11:03:12 -0000 > > @@ -527,7 +527,7 @@ relay_read_http(struct bufferevent *bev, > > * Ask the server to close the connection after this request > > * since we don't read any further request headers. > > */ > > - if (cre->toread == TOREAD_UNLIMITED) > > + if (cre->toread == TOREAD_UNLIMITED && upgrade == NULL) > > if (kv_add(&desc->http_headers, "Connection", > > "close", 0) == NULL) > > goto fail; > > > > mcmer-open...@tor.at (Marcus MERIGHI), 2021.01.04 (Mon) 12:59 (CET): > >> One month has passed, this is just a friendly ping... > >> > >> Marcus > >> > >> mcmer-open...@tor.at (Marcus MERIGHI), 2020.12.04 (Fri) 14:18 (CET): > >>> This patch wasn't commited and not discussed (publicly). > >>> > >>> It lets me use relayd as a front-end to the mattermost-server. > >>> > >>> @franz: Thank you! > >>> > >>> fr...@bett.ag (Franz Bettag), 2020.03.04 (Wed) 17:52 (CET): > >>>> After migrating my home setup from nginx reverse proxying to relayd, i > >>>> noticed my iOS devices having issues connecting through Websockets. > >>>> After debugging, i noticed that relayd adds the "Connection: close" > >>>> regardless of upgrade being requested. > >>>> > >>>> This issue is also reported on a blog-post using relayd as a Websocket > >>>> Client. https://deftly.net/posts/2019-10-23-websockets-with-relayd.html > >>>> > >>>> This resulted in the response having two Connection Headers, one > >>>> "Connection: upgrade" and one "Connection: close", which iOS strict > >>>> implementation does not allow. > >>>> > >>>> I have fixed the if condition that leads to this issue. > >>>> > >>>> The fix has been tested with Apple iOS 13 and the problem can be > >>>> observed using Firefox and just connecting to something Websocket over > >>>> relayd. Both "Connection:" headers will appear. > >>>> > >>>> best regards > >>>> > >>>> Franz > >>>> > >>>> > >>>> diff --git usr.sbin/relayd/relay_http.c usr.sbin/relayd/relay_http.c > >>>> index 960d4c54a..3a6678790 100644 > >>>> --- usr.sbin/relayd/relay_http.c > >>>> +++ usr.sbin/relayd/relay_http.c > >>>> @@ -524,9 +524,11 @@ relay_read_http(struct bufferevent *bev, void *arg) > >>>> > >>>> /* > >>>> * Ask the server to close the connection after this request > >>>> - * since we don't read any further request headers. > >>>> + * since we don't read any further request headers, unless > >>>> + * an Upgrade is requested, in which case we do NOT want to add > >>>> + * this header. > >>>> */ > >>>> - if (cre->toread == TOREAD_UNLIMITED) > >>>> + if (cre->toread == TOREAD_UNLIMITED && upgrade == NULL) > >>>> if (kv_add(&desc->http_headers, "Connection", > >>>> "close", 0) == NULL) > >>>> goto fail; > > <relayd.relay_http.c.diff> >
