> 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