[PATCH 3 of 3] QUIC: keep stream sockaddr and addr_text constant

[Roman Arutyunyan]

Hi,

On Thu, May 11, 2023 at 02:26:41PM +0400, Sergey Kandaurov wrote:
> 
> > On 10 May 2023, at 21:46, Roman Arutyunyan <arut at nginx.com> wrote:
> > 
> > Hi,
> > 
> > On Tue, May 02, 2023 at 04:34:15PM +0400, Roman Arutyunyan wrote:
> >> # HG changeset patch
> >> # User Roman Arutyunyan <arut at 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