Jens-G opened a new pull request, #3836:
URL: https://github.com/apache/thrift/pull/3836

   ## The defect
   
   `TServerSocket::listen()` and `TNonblockingServerSocket::listen()` resolve 
the bind address with `AI_PASSIVE|AI_V4MAPPED`; `TSocket::open()` resolves the 
connect address with `AI_PASSIVE|AI_ADDRCONFIG`.
   
   `AI_ADDRCONFIG` does not count a loopback address as a configured one. So on 
a host that carries `::1` on `lo` and has no other IPv6 address — a default 
Docker container, for instance — the two disagree about what `"localhost"` 
means:
   
   | Caller | Flags | First address |
   |---|---|---|
   | `TServerSocket::listen()` | `AI_PASSIVE\|AI_V4MAPPED` | `::1`, and the 
bind succeeds |
   | `TSocket::open()` | `AI_PASSIVE\|AI_ADDRCONFIG` | `127.0.0.1` |
   
   A C++ server on `"localhost"` therefore listens on `::1` while a C++ client 
on `"localhost"` dials `127.0.0.1`, and the connect fails with `ECONNREFUSED`. 
The `IPV6_V6ONLY=0` the server sets does not rescue it: the bind is to `::1` 
specifically, not to `::`.
   
   Measured in such a container, resolving with each flag set:
   
   ```
   --- named host: localhost ---
   server today  (AI_PASSIVE|AI_V4MAPPED)         -> ::1 127.0.0.1
   client today  (AI_PASSIVE|AI_ADDRCONFIG)       -> 127.0.0.1
   server after  (+AI_ADDRCONFIG)                 -> 127.0.0.1
   
   --- numeric ::1 ---
   server today  (AI_PASSIVE|AI_V4MAPPED)         -> ::1
   server after  (+AI_ADDRCONFIG)                 -> ERROR EAI_ADDRFAMILY   <- 
why the fallback exists
   ```
   
   ## When it changed
   
   Last worked in 0.13.0. THRIFT-5186 (`9b9567b23`, first released in 0.14.0) 
removed `AI_ADDRCONFIG` from the server sockets so a host with no configured 
address could still resolve localhost — correct on its own terms. That same 
commit handled the client differently: it kept the flag on `TSocket::open()` 
and instead extended the Windows-only retry-without-it to POSIX. THRIFT-5880 
(`25202e1b0`) later widened that retry to `EAI_ADDRFAMILY`, but it is 
conditional on the first resolution *failing* and does not fire here, because 
it succeeds.
   
   CI never caught it: GitHub runners have `::1` on `lo`, so `AI_ADDRCONFIG` 
returns IPv6 for the client too and both sides agree again.
   
   ## The change
   
   Give the servers the same shape `TSocket::open()` already has: resolve with 
`AI_ADDRCONFIG`, and fall back to resolving without it when that leaves nothing 
to bind. Both sides then agree, and THRIFT-5186's case still works via the 
fallback.
   
   The fallback is load-bearing rather than defensive — as the measurement 
above shows, an explicit `"::1"` fails outright with `EAI_ADDRFAMILY` under 
`AI_ADDRCONFIG` on such a host.
   
   The `#ifdef ANDROID` branch becomes redundant and goes away. Android already 
passed `AI_ADDRCONFIG` and only gains the fallback; `AI_V4MAPPED` is ignored 
for the `AF_UNSPEC` queries this helper makes, so it is not a behaviour change 
there.
   
   The wildcard bind is unaffected: with an empty address the resolver already 
returns `0.0.0.0` ahead of `::`, and where IPv6 *is* configured `AI_ADDRCONFIG` 
does not remove it.
   
   ## Tests
   
   - `TServerSocketTest/test_bind_to_address` already covered this for 
`TServerSocket` and fails in such a container.
   - `TNonblockingServerSocket` carried the identical line with no equivalent 
coverage; `TNonblockingServerTest/bind_and_connect_agree_on_hostname` adds it.
   
   Both were confirmed to fail against the unmodified library before the fix, 
with `connect() failed: Connection refused`, and pass after it.
   
   Full C++ suite in `thrift:jammy`: **39/39 `ctest` pass**. `UnitTests` was 
red in that container before this change and is green now. Changed lines are 
`clang-format` clean.
   
   Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to