Hi,

On Thu, May 11, 2023 at 02:26:41PM +0400, Sergey Kandaurov wrote:
>
> > On 10 May 2023, at 21:46, Roman Arutyunyan <arut@nginx.com> wrote:
> >
> > Hi,
> >
> > On Tue, May 02, 2023 at 04:34:15PM +0400, Roman Arutyunyan wrote:
> >> # HG changeset patch
> >> # User Roman Arutyunyan <arut@nginx.com>
> >> # Date 1682679819 -14400
> >> # Fri Apr 28 15:03:39 2023 +0400
> >> # Branch quic
> >> # Node ID 43f0ceffa227a33e5c5ceb35b77f9a1f86dd2481
> >> # Parent cdc41ec778ffae822fefce639e67f2f57e3667f0
> >> QUIC: keep stream sockaddr and addr_text constant.
> >>
> >> HTTP and Stream variables $remote_addr and $binary_remote_addr rely on
> >> constant client address, particularly because they are cacheable.
> >> However, QUIC client may migrate to a new address. While there's no perfect
> >> way to handle this, the proposed solution is to copy client address to QUIC
> >> stream at stream creation. Previously, the address was only referenced, which
> >> could result in changing it while stream was active, which in turn would lead
> >> to broken cached variables values, since address length is cached as well.
> >
> > While testing this, it was found that $remote_addr truncation happens at the
> > QUIC level since the addr_text string is copied by value and retains the old
> > length after migration. The new commit log:
> >
> > QUIC: keep stream sockaddr and addr_text constant.
> >
> > HTTP and Stream variables $remote_addr and $binary_remote_addr rely on
> > constant client address, particularly because they are cacheable.
> > However, QUIC client may migrate to a new address. While there's no perfect
> > way to handle this, the proposed solution is to copy client address to QUIC
> > stream at stream creation.
> >
> > The change also fixes truncated $remote_addr if migration happened while the
> > stream was active. The reason is addr_text string was copied to stream by
> > value.
> >
> > [..]
> >
>
> All series looks good for me.

Discussed patch #3 again. Decided to add the following, just to be on the safe
side:

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
@@ -716,6 +716,7 @@ ngx_quic_create_stream(ngx_connection_t

} else {
addr_text.len = 0;
+ addr_text.data = NULL;
}

reusable = c->reusable;

[..]

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