From 9d79859ba691b6f1fce9f3f7936bc184a1499f3f Mon Sep 17 00:00:00 2001 From: Caleb Callaway Date: Sat, 18 Oct 2014 19:21:02 -0700 Subject: [PATCH 1/5] More verbose debug logging of UPnP SOAP requests --- internal/upnp/upnp.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/upnp/upnp.go b/internal/upnp/upnp.go index 0553960f7..74205c0f9 100644 --- a/internal/upnp/upnp.go +++ b/internal/upnp/upnp.go @@ -448,7 +448,8 @@ func soapRequest(url, device, function, message string) ([]byte, error) { req.Header.Set("Pragma", "no-cache") if debug { - l.Debugln(req.Header.Get("SOAPAction")) + l.Debugln("SOAP Request URL: " + url) + l.Debugln("SOAP Action: " + req.Header.Get("SOAPAction")) l.Debugln("SOAP Request:\n\n" + body) } From 87b9e8fbaf5b498374d708547cddadc3da0ccad6 Mon Sep 17 00:00:00 2001 From: Caleb Callaway Date: Sun, 19 Oct 2014 10:23:12 -0700 Subject: [PATCH 2/5] Parse UPnP service ID from root description and expose it to consumers --- internal/upnp/upnp.go | 50 ++++++++++++++++++++++++------------------- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/internal/upnp/upnp.go b/internal/upnp/upnp.go index 74205c0f9..710d2483f 100644 --- a/internal/upnp/upnp.go +++ b/internal/upnp/upnp.go @@ -44,12 +44,37 @@ type IGD struct { localIPAddress string } +// The InternetGatewayDevice's UUID. +func (n *IGD) UUID() string { + return n.uuid +} + +// The InternetGatewayDevice's friendly name. +func (n *IGD) FriendlyName() string { + return n.friendlyName +} + +// The InternetGatewayDevice's friendly identifier (friendly name + IP address). +func (n *IGD) FriendlyIdentifier() string { + return "'" + n.FriendlyName() + "' (" + strings.Split(n.URL().Host, ":")[0] + ")" +} + +// The URL of the InternetGatewayDevice's root device description. +func (n *IGD) URL() *url.URL { + return n.url +} + // A container for relevant properties of a UPnP service of an IGD. type IGDService struct { + serviceID string serviceURL string serviceURN string } +func (s *IGDService) ID() string { + return s.serviceID +} + type Protocol string const ( @@ -58,6 +83,7 @@ const ( ) type upnpService struct { + ServiceID string `xml:"serviceId"` ServiceType string `xml:"serviceType"` ControlURL string `xml:"controlURL"` } @@ -94,7 +120,7 @@ func Discover() []*IGD { l.Debugln("[" + resultDevice.uuid + "]") for _, resultService := range resultDevice.services { - l.Debugln("* " + resultService.serviceURL) + l.Debugln("* [" + resultService.serviceID + "] " + resultService.serviceURL) } } } @@ -398,7 +424,7 @@ func getIGDServices(rootURL string, device upnpDevice, wanDeviceURN string, wanC l.Debugln("[" + rootURL + "] Found " + service.ServiceType + " with URL " + u.String()) } - service := IGDService{serviceURL: u.String(), serviceURN: service.ServiceType} + service := IGDService{serviceID: service.ServiceID, serviceURL: u.String(), serviceURN: service.ServiceType} result = append(result, service) } @@ -498,26 +524,6 @@ func (n *IGD) DeletePortMapping(protocol Protocol, externalPort int) error { return nil } -// The InternetGatewayDevice's UUID. -func (n *IGD) UUID() string { - return n.uuid -} - -// The InternetGatewayDevice's friendly name. -func (n *IGD) FriendlyName() string { - return n.friendlyName -} - -// The InternetGatewayDevice's friendly identifier (friendly name + IP address). -func (n *IGD) FriendlyIdentifier() string { - return "'" + n.FriendlyName() + "' (" + strings.Split(n.URL().Host, ":")[0] + ")" -} - -// The URL of the InternetGatewayDevice's root device description. -func (n *IGD) URL() *url.URL { - return n.url -} - type soapGetExternalIPAddressResponseEnvelope struct { XMLName xml.Name Body soapGetExternalIPAddressResponseBody `xml:"Body"` From 27448bde20395335fee208931927ae6760e70810 Mon Sep 17 00:00:00 2001 From: Caleb Callaway Date: Sun, 19 Oct 2014 10:38:12 -0700 Subject: [PATCH 3/5] Variable naming clarification --- internal/upnp/upnp.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/upnp/upnp.go b/internal/upnp/upnp.go index 710d2483f..8d2746b6f 100644 --- a/internal/upnp/upnp.go +++ b/internal/upnp/upnp.go @@ -452,7 +452,7 @@ func replaceRawPath(u *url.URL, rp string) { u.RawQuery = q } -func soapRequest(url, device, function, message string) ([]byte, error) { +func soapRequest(url, service, function, message string) ([]byte, error) { tpl := ` %s @@ -468,7 +468,7 @@ func soapRequest(url, device, function, message string) ([]byte, error) { } req.Header.Set("Content-Type", `text/xml; charset="utf-8"`) req.Header.Set("User-Agent", "syncthing/1.0") - req.Header.Set("SOAPAction", fmt.Sprintf(`"%s#%s"`, device, function)) + req.Header.Set("SOAPAction", fmt.Sprintf(`"%s#%s"`, service, function)) req.Header.Set("Connection", "Close") req.Header.Set("Cache-Control", "no-cache") req.Header.Set("Pragma", "no-cache") From 4183044e960889423c8cb73b7129ec4ed9ae36bd Mon Sep 17 00:00:00 2001 From: Caleb Callaway Date: Wed, 22 Oct 2014 18:44:13 -0700 Subject: [PATCH 4/5] Fix UPnP log spam on networks without UPnP IGDs (see #893) We should only run the UPnP port mapping renewal routine if the initial discovery and configuration succeed. This commit applies that logic. --- cmd/syncthing/main.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/syncthing/main.go b/cmd/syncthing/main.go index fcb042a92..921587289 100644 --- a/cmd/syncthing/main.go +++ b/cmd/syncthing/main.go @@ -712,11 +712,11 @@ func setupUPnP() { l.Warnln("Failed to create UPnP port mapping") } else { l.Infof("Created UPnP port mapping for external port %d on UPnP device %s.", externalPort, igd.FriendlyIdentifier()) - } - } - if opts.UPnPRenewal > 0 { - go renewUPnP(port) + if opts.UPnPRenewal > 0 { + go renewUPnP(port) + } + } } } } else { From b7bb3bfee2861bfb459e66228bd2d9c0fe735149 Mon Sep 17 00:00:00 2001 From: Caleb Callaway Date: Wed, 22 Oct 2014 19:09:17 -0700 Subject: [PATCH 5/5] UPnP discovery fix for devices that send multiple response packets Fix UPnP discovery and port mapping issues reported in #896 --- internal/upnp/upnp.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/internal/upnp/upnp.go b/internal/upnp/upnp.go index 8d2746b6f..959e26cef 100644 --- a/internal/upnp/upnp.go +++ b/internal/upnp/upnp.go @@ -210,6 +210,17 @@ Mx: %d // Collect our results from the result handlers using the result channel for result := range resultChannel { + // Check for existing results (some routers send multiple response packets) + for _, existingResult := range results { + if existingResult.uuid == result.uuid { + if debug { + l.Debugln("Already processed device with UUID", existingResult.uuid, "continuing...") + } + continue + } + } + + // No existing results, okay to append results = append(results, result) }