diff --git a/cmd/syncthing/main.go b/cmd/syncthing/main.go index 0e43d2593..d563a7d04 100644 --- a/cmd/syncthing/main.go +++ b/cmd/syncthing/main.go @@ -727,7 +727,7 @@ func setupUPnP() { } else { // Set up incoming port forwarding, if necessary and possible port, _ := strconv.Atoi(portStr) - igds := upnp.Discover() + igds := upnp.Discover(time.Duration(cfg.Options().UPnPTimeoutS) * time.Second) if len(igds) > 0 { // Configure the first discovered IGD only. This is a work-around until we have a better mechanism // for handling multiple IGDs, which will require changes to the global discovery service @@ -769,13 +769,15 @@ func renewUPnP(port int) { for { opts := cfg.Options() time.Sleep(time.Duration(opts.UPnPRenewal) * time.Minute) + // Some values might have changed while we were sleeping + opts = cfg.Options() // Make sure our IGD reference isn't nil if igd == nil { if debugNet { l.Debugln("Undefined IGD during UPnP port renewal. Re-discovering...") } - igds := upnp.Discover() + igds := upnp.Discover(time.Duration(opts.UPnPTimeoutS) * time.Second) if len(igds) > 0 { // Configure the first discovered IGD only. This is a work-around until we have a better mechanism // for handling multiple IGDs, which will require changes to the global discovery service diff --git a/internal/config/config.go b/internal/config/config.go index cb1d8b0a3..5724d81fb 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -229,6 +229,7 @@ type OptionsConfiguration struct { UPnPEnabled bool `xml:"upnpEnabled" json:"upnpEnabled" default:"true"` UPnPLease int `xml:"upnpLeaseMinutes" json:"upnpLeaseMinutes" default:"0"` UPnPRenewal int `xml:"upnpRenewalMinutes" json:"upnpRenewalMinutes" default:"30"` + UPnPTimeoutS int `xml:"upnpTimeoutSeconds" json:"upnpTimeoutSeconds" default:"3"` URAccepted int `xml:"urAccepted" json:"urAccepted"` // Accepted usage reporting version; 0 for off (undecided), -1 for off (permanently) URUniqueID string `xml:"urUniqueID" json:"urUniqueId"` // Unique ID for reporting purposes, regenerated when UR is turned on. RestartOnWakeup bool `xml:"restartOnWakeup" json:"restartOnWakeup" default:"true"` diff --git a/internal/config/config_test.go b/internal/config/config_test.go index f5194979b..813cdd0c5 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -44,6 +44,7 @@ func TestDefaultValues(t *testing.T) { UPnPEnabled: true, UPnPLease: 0, UPnPRenewal: 30, + UPnPTimeoutS: 3, RestartOnWakeup: true, AutoUpgradeIntervalH: 12, KeepTemporariesH: 24, @@ -149,6 +150,7 @@ func TestOverriddenValues(t *testing.T) { UPnPEnabled: false, UPnPLease: 60, UPnPRenewal: 15, + UPnPTimeoutS: 15, RestartOnWakeup: false, AutoUpgradeIntervalH: 24, KeepTemporariesH: 48, diff --git a/internal/config/testdata/overridenvalues.xml b/internal/config/testdata/overridenvalues.xml index c2f72eec0..1730a795d 100755 --- a/internal/config/testdata/overridenvalues.xml +++ b/internal/config/testdata/overridenvalues.xml @@ -15,6 +15,7 @@ false 60 15 + 15 false 24 48 diff --git a/internal/upnp/upnp.go b/internal/upnp/upnp.go index c6675e7b6..a5cff8f3a 100644 --- a/internal/upnp/upnp.go +++ b/internal/upnp/upnp.go @@ -92,12 +92,10 @@ type upnpRoot struct { // Discover discovers UPnP InternetGatewayDevices. // The order in which the devices appear in the result list is not deterministic. -func Discover() []IGD { +func Discover(timeout time.Duration) []IGD { var result []IGD l.Infoln("Starting UPnP discovery...") - timeout := 3 - // Search for InternetGatewayDevice:2 devices result = append(result, discover("urn:schemas-upnp-org:device:InternetGatewayDevice:2", timeout, result)...) @@ -128,7 +126,7 @@ func Discover() []IGD { // Search for UPnP InternetGatewayDevices for seconds, ignoring responses from any devices listed in knownDevices. // The order in which the devices appear in the result list is not deterministic -func discover(deviceType string, timeout int, knownDevices []IGD) []IGD { +func discover(deviceType string, timeout time.Duration, knownDevices []IGD) []IGD { ssdp := &net.UDPAddr{IP: []byte{239, 255, 255, 250}, Port: 1900} tpl := `M-SEARCH * HTTP/1.1 @@ -138,7 +136,7 @@ Man: "ssdp:discover" Mx: %d ` - searchStr := fmt.Sprintf(tpl, deviceType, timeout) + searchStr := fmt.Sprintf(tpl, deviceType, timeout/time.Second) search := []byte(strings.Replace(searchStr, "\n", "\r\n", -1)) @@ -156,7 +154,7 @@ Mx: %d } defer socket.Close() // Make sure our socket gets closed - err = socket.SetDeadline(time.Now().Add(time.Duration(timeout) * time.Second)) + err = socket.SetDeadline(time.Now().Add(timeout)) if err != nil { l.Infoln(err) return results