Repository: syncthing/syncthing (language: go)

Issue:
Discovery Returns <Nil> IP
## Does your log mention database corruption?
No

## Include required information
On Discovery Client Announcements, the `X-Forwarded-For`  header may container multiple IP Addresses, for instance when sitting behind multiple proxies.    
The syntax in this case is a string of IP Addresses separated by commas. 
[For reference](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For#syntax)

Currently, Discovery assumes `X-Forwarded-For` is a single IP, which results to a `nil` IP

## Syncthing Version
v1.24.0

## What happens
When a list of IPs is provided in `X-Forwarded-For`, Discovery treats the header value as a string with a single IP; 
[Link to line of code](https://github.com/syncthing/syncthing/blob/main/cmd/stdiscosrv/apisrv.go#L139)
This result to a `nil` IP; i.e. `tcp://<nil>:22000` when `listenAddress = tcp://0.0.0.0:22000 | tcp://:22000`

## What is expected
We should parse `X-Forwarded-For` by splitting the string by commas and taking the originating client IP (the leftmost value)

## Steps to Reproduce
A simple example to replicate this entails running Discovery with HTTP, use Nginx as a reverse proxy and hardcode (as an example) a list of IPs in the `X-Forwarded-For` header.  
    1. Send an Announcement with `tcp://0.0.0.0:<some-port>`
    2. Query the DeviceID
    3. Observe the returned IP Address is `tcp://<nil>:<some-port>`



Discussion / hints:

Fix patch:
diff --git a/cmd/stdiscosrv/apisrv.go b/cmd/stdiscosrv/apisrv.go
index c9c8344b3fa..d4bfd490b87 100644
--- a/cmd/stdiscosrv/apisrv.go
+++ b/cmd/stdiscosrv/apisrv.go
@@ -136,7 +136,12 @@ func (s *apiSrv) handler(w http.ResponseWriter, req *http.Request) {
 	}
 
 	if s.useHTTP {
-		remoteAddr.IP = net.ParseIP(req.Header.Get("X-Forwarded-For"))
+		// X-Forwarded-For can have multiple client IPs; split using the comma separator
+		forwardIP, _, _ := strings.Cut(req.Header.Get("X-Forwarded-For"), ",")
+
+		// net.ParseIP will return nil if leading/trailing whitespace exists; use strings.TrimSpace()
+		remoteAddr.IP = net.ParseIP(strings.TrimSpace(forwardIP))
+
 		if parsedPort, err := strconv.ParseInt(req.Header.Get("X-Client-Port"), 10, 0); err == nil {
 			remoteAddr.Port = int(parsedPort)
 		}
@@ -410,13 +415,13 @@ func fixupAddresses(remote *net.TCPAddr, addresses []string) []string {
 			continue
 		}
 
-		if remote != nil {
-			if host == "" || ip.IsUnspecified() {
+		if host == "" || ip.IsUnspecified() {
+			if remote != nil {
 				// Replace the unspecified IP with the request source.
 
 				// ... unless the request source is the loopback address or
 				// multicast/unspecified (can't happen, really).
-				if remote.IP.IsLoopback() || remote.IP.IsMulticast() || remote.IP.IsUnspecified() {
+				if remote.IP == nil || remote.IP.IsLoopback() || remote.IP.IsMulticast() || remote.IP.IsUnspecified() {
 					continue
 				}
 
@@ -432,11 +437,22 @@ func fixupAddresses(remote *net.TCPAddr, addresses []string) []string {
 				}
 
 				host = remote.IP.String()
+
+			} else {
+				// remote is nil, unable to determine host IP
+				continue
 			}
 
-			// If zero port was specified, use remote port.
-			if port == "0" && remote.Port > 0 {
+		}
+
+		// If zero port was specified, use remote port.
+		if port == "0" {
+			if remote != nil && remote.Port > 0 {
+				// use remote port
 				port = strconv.Itoa(remote.Port)
+			} else {
+				// unable to determine remote port
+				continue
 			}
 		}
 


Test patch:
diff --git a/cmd/stdiscosrv/apisrv_test.go b/cmd/stdiscosrv/apisrv_test.go
index 3514994a3ec..457ed404f95 100644
--- a/cmd/stdiscosrv/apisrv_test.go
+++ b/cmd/stdiscosrv/apisrv_test.go
@@ -69,6 +69,14 @@ func TestFixupAddresses(t *testing.T) {
 			remote: addr("123.123.123.123", 9000),
 			in:     []string{"tcp://44.44.44.44:0"},
 			out:    []string{"tcp://44.44.44.44:9000"},
+		}, { // remote ip nil
+			remote: addr("", 9000),
+			in:     []string{"tcp://:22000", "tcp://44.44.44.44:9000"},
+			out:    []string{"tcp://44.44.44.44:9000"},
+		}, { // remote port 0
+			remote: addr("123.123.123.123", 0),
+			in:     []string{"tcp://:22000", "tcp://44.44.44.44"},
+			out:    []string{"tcp://123.123.123.123:22000"},
 		},
 	}