On Tue, Jan 17, 2023 at 04:01:39PM +0400, Sergey Kandaurov wrote:
>
> > On 17 Jan 2023, at 14:44, Roman Arutyunyan <arut@nginx.com> wrote:
> >
> >
> >
> >> On 16 Jan 2023, at 17:27, Sergey Kandaurov <pluknet@nginx.com> wrote:
> >>
> >> # HG changeset patch
> >> # User Sergey Kandaurov <pluknet@nginx.com>
> >> # Date 1673875616 -14400
> >> # Mon Jan 16 17:26:56 2023 +0400
> >> # Branch quic
> >> # Node ID f7c7cabe232898db5b16a142163de71964cebcfd
> >> # Parent 6bb884dc72916dc675df65d02abee0c9cfabc916
> >> QUIC: defer setting the active flag for client stream events.
> >>
> >> Specifically, now it is kept unset until streams are initialized.
> >> Notably, this unbreaks OCSP with client certificates after 35e27117b593.
> >> Previously, the read event could be posted prematurely in ngx_quic_set_event()
> >> e.g., as part of handling a STREAM frame.
> >>
> >> diff --git a/src/event/quic/ngx_event_quic_streams.c b/src/event/quic/ngx_event_quic_streams.c
> >> --- a/src/event/quic/ngx_event_quic_streams.c
> >> +++ b/src/event/quic/ngx_event_quic_streams.c
> >> @@ -106,6 +106,13 @@ ngx_quic_open_stream(ngx_connection_t *c
> >> return NULL;
> >> }
> >>
> >> + nqs->connection->write->active = 1;
> >> + nqs->connection->write->ready = 1;
> >> +
> >> + if (!bidi) {
> >
> > if (bidi) ?
>
> Sure, tnx.
>
> >
> >> + nqs->connection->read->active = 1;
> >> + }
> >> +
> >> return nqs->connection;
> >
> > This could've been simplified by adding a variable for nqs->connection, YMMV.
>
> Agree, and it looks similar to other places.
>
> While here, renamed nqs to just a qs, the only one after 6434160b4b78.
>
> # HG changeset patch
> # User Sergey Kandaurov <pluknet@nginx.com>
> # Date 1673956193 -14400
> # Tue Jan 17 15:49:53 2023 +0400
> # Branch quic
> # Node ID fa3018f0102247215953126515974c76d2b02e53
> # Parent 6bb884dc72916dc675df65d02abee0c9cfabc916
> QUIC: defer setting the active flag for client stream events.
>
> Specifically, now it is kept unset until streams are initialized.
> Notably, this unbreaks OCSP with client certificates after 35e27117b593.
> Previously, the read event could be posted prematurely via ngx_quic_set_event()
> e.g., as part of handling a STREAM frame.
>
> diff --git a/src/event/quic/ngx_event_quic_streams.c b/src/event/quic/ngx_event_quic_streams.c
> --- a/src/event/quic/ngx_event_quic_streams.c
> +++ b/src/event/quic/ngx_event_quic_streams.c
> @@ -46,8 +46,8 @@ ngx_connection_t *
> ngx_quic_open_stream(ngx_connection_t *c, ngx_uint_t bidi)
> {
> uint64_t id;
> - ngx_connection_t *pc;
> - ngx_quic_stream_t *nqs;
> + ngx_connection_t *pc, *sc;
> + ngx_quic_stream_t *qs;
> ngx_quic_connection_t *qc;
>
> pc = c->quic ? c->quic->parent : c;
> @@ -101,12 +101,21 @@ ngx_quic_open_stream(ngx_connection_t *c
> qc->streams.server_streams_uni++;
> }
>
> - nqs = ngx_quic_create_stream(pc, id);
> - if (nqs == NULL) {
> + qs = ngx_quic_create_stream(pc, id);
> + if (qs == NULL) {
> return NULL;
> }
>
> - return nqs->connection;
> + sc = qs->connection;
> +
> + sc->write->active = 1;
> + sc->write->ready = 1;
> +
> + if (bidi) {
> + sc->read->active = 1;
> + }
> +
> + return sc;
> }
>
>
> @@ -534,6 +543,13 @@ ngx_quic_init_stream_handler(ngx_event_t
>
> ngx_log_debug0(NGX_LOG_DEBUG_EVENT, c->log, 0, "quic init stream");
>
> + if ((qs->id & NGX_QUIC_STREAM_UNIDIRECTIONAL) == 0) {
> + c->write->active = 1;
> + c->write->ready = 1;
> + }
> +
> + c->read->active = 1;
> +
> ngx_queue_remove(&qs->queue);
>
> c->listening->handler(c);
> @@ -704,19 +720,6 @@ ngx_quic_create_stream(ngx_connection_t
>
> log->connection = sc->number;
>
> - if ((id & NGX_QUIC_STREAM_UNIDIRECTIONAL) == 0
> - || (id & NGX_QUIC_STREAM_SERVER_INITIATED))
> - {
> - sc->write->active = 1;
> - sc->write->ready = 1;
> - }
> -
> - if ((id & NGX_QUIC_STREAM_UNIDIRECTIONAL) == 0
> - || (id & NGX_QUIC_STREAM_SERVER_INITIATED) == 0)
> - {
> - sc->read->active = 1;
> - }
> -
> if (id & NGX_QUIC_STREAM_UNIDIRECTIONAL) {
> if (id & NGX_QUIC_STREAM_SERVER_INITIATED) {
> qs->send_max_data = qc->ctp.initial_max_stream_data_uni;
>
>
> --
> Sergey Kandaurov
> _______________________________________________
> nginx-devel mailing list
> nginx-devel@nginx.org
> https://mailman.nginx.org/mailman/listinfo/nginx-devel

Looks fine

--
Roman Arutyunyan
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel