From 37d83a4e2ecd53d744dbf294f1d4080384655da3 Mon Sep 17 00:00:00 2001 From: Ben Sidhom Date: Fri, 4 Jul 2014 13:47:54 -0700 Subject: [PATCH 1/4] Continue discovery on connect errors (fixes #324) Continues trying to connect to the discovery server at regular intervals despite failure. Whether or not to retry and retry interval should be specified in configuration (not currently in this fix). --- discover/discover.go | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/discover/discover.go b/discover/discover.go index 4eb82d38..ba0289fe 100644 --- a/discover/discover.go +++ b/discover/discover.go @@ -168,16 +168,21 @@ func (d *Discoverer) sendLocalAnnouncements() { } func (d *Discoverer) sendExternalAnnouncements() { + // this should go in the Discoverer struct + errorRetryIntv := 60 * time.Second + remote, err := net.ResolveUDPAddr("udp", d.extServer) - if err != nil { - l.Warnf("Global discovery: %v; no external announcements", err) - return + for err != nil { + l.Warnf("Global discovery: %v; trying again in %v", err, errorRetryIntv) + time.Sleep(errorRetryIntv) + remote, err = net.ResolveUDPAddr("udp", d.extServer) } conn, err := net.ListenUDP("udp", nil) - if err != nil { - l.Warnf("Global discovery: %v; no external announcements", err) - return + for err != nil { + l.Warnf("Global discovery: %v; trying again in %v", err, errorRetryIntv) + time.Sleep(errorRetryIntv) + conn, err = net.ListenUDP("udp", nil) } var buf []byte @@ -198,7 +203,7 @@ func (d *Discoverer) sendExternalAnnouncements() { l.Debugf("discover: send announcement -> %v\n%s", remote, hex.Dump(buf)) } - _, err = conn.WriteTo(buf, remote) + _, err := conn.WriteTo(buf, remote) if err != nil { if debug { l.Debugln("discover: warning:", err) @@ -222,7 +227,7 @@ func (d *Discoverer) sendExternalAnnouncements() { if ok { time.Sleep(d.globalBcastIntv) } else { - time.Sleep(60 * time.Second) + time.Sleep(errorRetryIntv) } } } From a7b6e3546760a3a99d60beccf626317ac668496c Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Tue, 8 Jul 2014 11:49:11 +0200 Subject: [PATCH 2/4] incomingIndexes should not be a package variable (fixes #344) --- protocol/protocol.go | 55 ++++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 27 deletions(-) diff --git a/protocol/protocol.go b/protocol/protocol.go index e82fa756..4136b9e7 100644 --- a/protocol/protocol.go +++ b/protocol/protocol.go @@ -97,6 +97,15 @@ type rawConnection struct { outbox chan []encodable closed chan struct{} once sync.Once + + incomingIndexes chan incomingIndex +} + +type incomingIndex struct { + update bool + id string + repo string + files []FileInfo } type asyncResult struct { @@ -121,21 +130,22 @@ func NewConnection(nodeID string, reader io.Reader, writer io.Writer, receiver M wb := bufio.NewWriter(flwr) c := rawConnection{ - id: nodeID, - receiver: nativeModel{receiver}, - state: stateInitial, - reader: flrd, - cr: cr, - xr: xdr.NewReader(flrd), - writer: flwr, - cw: cw, - wb: wb, - xw: xdr.NewWriter(wb), - awaiting: make([]chan asyncResult, 0x1000), - idxSent: make(map[string]map[string]uint64), - outbox: make(chan []encodable), - nextID: make(chan int), - closed: make(chan struct{}), + id: nodeID, + receiver: nativeModel{receiver}, + state: stateInitial, + reader: flrd, + cr: cr, + xr: xdr.NewReader(flrd), + writer: flwr, + cw: cw, + wb: wb, + xw: xdr.NewWriter(wb), + awaiting: make([]chan asyncResult, 0x1000), + idxSent: make(map[string]map[string]uint64), + outbox: make(chan []encodable), + nextID: make(chan int), + closed: make(chan struct{}), + incomingIndexes: make(chan incomingIndex, 100), // should be enough for anyone, right? } go c.indexSerializerLoop() @@ -316,15 +326,6 @@ func (c *rawConnection) readerLoop() (err error) { } } -type incomingIndex struct { - update bool - id string - repo string - files []FileInfo -} - -var incomingIndexes = make(chan incomingIndex, 100) // should be enough for anyone, right? - func (c *rawConnection) indexSerializerLoop() { // We must avoid blocking the reader loop when processing large indexes. // There is otherwise a potential deadlock where both sides has the model @@ -334,7 +335,7 @@ func (c *rawConnection) indexSerializerLoop() { // routine and buffered channel. for { select { - case ii := <-incomingIndexes: + case ii := <-c.incomingIndexes: if ii.update { c.receiver.IndexUpdate(ii.id, ii.repo, ii.files) } else { @@ -360,7 +361,7 @@ func (c *rawConnection) handleIndex() error { // update and can't receive the large index update from the // other side. - incomingIndexes <- incomingIndex{false, c.id, im.Repository, im.Files} + c.incomingIndexes <- incomingIndex{false, c.id, im.Repository, im.Files} } return nil } @@ -371,7 +372,7 @@ func (c *rawConnection) handleIndexUpdate() error { if err := c.xr.Error(); err != nil { return err } else { - incomingIndexes <- incomingIndex{true, c.id, im.Repository, im.Files} + c.incomingIndexes <- incomingIndex{true, c.id, im.Repository, im.Files} } return nil } From 50b37f1366d44f7f46d23af5d5f1bfa73a7b8026 Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Tue, 8 Jul 2014 11:49:28 +0200 Subject: [PATCH 3/4] Revert "Add temporary debug logging for #344 (revert later)" This reverts commit 5353659f9fcb840142ba4c8d83f48654d43d4bdc. --- files/set.go | 5 ----- files/set_debug.go | 51 ---------------------------------------------- model/model.go | 3 --- 3 files changed, 59 deletions(-) delete mode 100644 files/set_debug.go diff --git a/files/set.go b/files/set.go index 0f0d657f..bcb7048d 100644 --- a/files/set.go +++ b/files/set.go @@ -49,7 +49,6 @@ func (m *Set) Replace(id uint, fs []scanner.File) { } m.Lock() - log("Replace", id, len(fs)) if len(fs) == 0 || !m.equals(id, fs) { m.changes[id]++ m.replace(id, fs) @@ -66,7 +65,6 @@ func (m *Set) ReplaceWithDelete(id uint, fs []scanner.File) { } m.Lock() - log("ReplaceWithDelete", id, len(fs)) if len(fs) == 0 || !m.equals(id, fs) { m.changes[id]++ @@ -104,9 +102,7 @@ func (m *Set) Update(id uint, fs []scanner.File) { if debug { l.Debugf("Update(%d, [%d])", id, len(fs)) } - m.Lock() - log("Update", id, len(fs)) m.update(id, fs) m.changes[id]++ m.Unlock() @@ -224,7 +220,6 @@ func (m *Set) equals(id uint, fs []scanner.File) bool { func (m *Set) update(cid uint, fs []scanner.File) { remFiles := m.remoteKey[cid] if remFiles == nil { - printLog() l.Fatalln("update before replace for cid", cid) } for _, f := range fs { diff --git a/files/set_debug.go b/files/set_debug.go deleted file mode 100644 index 76dac1e2..00000000 --- a/files/set_debug.go +++ /dev/null @@ -1,51 +0,0 @@ -package files - -import ( - "fmt" - "time" - - "github.com/calmh/syncthing/cid" -) - -type logEntry struct { - time time.Time - method string - cid uint - node string - nfiles int -} - -func (l logEntry) String() string { - return fmt.Sprintf("%v: %s cid:%d node:%s nfiles:%d", l.time, l.method, l.cid, l.node, l.nfiles) -} - -var ( - debugLog [10]logEntry - debugNext int - cm *cid.Map -) - -func SetCM(m *cid.Map) { - cm = m -} - -func log(method string, id uint, nfiles int) { - e := logEntry{ - time: time.Now(), - method: method, - cid: id, - nfiles: nfiles, - } - if cm != nil { - e.node = cm.Name(id) - } - debugLog[debugNext] = e - debugNext = (debugNext + 1) % len(debugLog) -} - -func printLog() { - l.Debugln("--- Consistency error ---") - for _, e := range debugLog { - l.Debugln(e) - } -} diff --git a/model/model.go b/model/model.go index 42401819..fe37eba8 100644 --- a/model/model.go +++ b/model/model.go @@ -98,9 +98,6 @@ func NewModel(indexDir string, cfg *config.Configuration, clientName, clientVers sup: suppressor{threshold: int64(cfg.Options.MaxChangeKbps)}, } - // TEMP: #344 - files.SetCM(m.cm) - var timeout = 20 * 60 // seconds if t := os.Getenv("STDEADLOCKTIMEOUT"); len(t) > 0 { it, err := strconv.Atoi(t) From 58cc108c0cfad9a7a507ace70ab4cfb3eb96c304 Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Tue, 22 Jul 2014 19:23:43 +0200 Subject: [PATCH 4/4] Handle WANPPPConnection devices (fixes #431) --- upnp/testdata/technicolor.xml | 93 +++++++++++++++++++++++++++++++++++ upnp/upnp.go | 45 ++++++++++------- upnp/upnp_test.go | 17 +++++++ 3 files changed, 138 insertions(+), 17 deletions(-) create mode 100644 upnp/testdata/technicolor.xml create mode 100644 upnp/upnp_test.go diff --git a/upnp/testdata/technicolor.xml b/upnp/testdata/technicolor.xml new file mode 100644 index 00000000..1f564441 --- /dev/null +++ b/upnp/testdata/technicolor.xml @@ -0,0 +1,93 @@ + + + +1 +0 + +http://192.168.1.254:8000 + +urn:schemas-upnp-org:device:InternetGatewayDevice:1 +Technicolor TG784n v3 (1321RAWMS) +Technicolor +http://www.technicolor.com + +Technicolor Internet Gateway Device +Technicolor TG +784n v3 +http://www.technicolor.com +1321RAWMS +uuid:UPnP_Technicolor TG784n v3-1_A4-B1-E9-D8-F4-78 +/ + + +urn:schemas-upnp-org:service:Layer3Forwarding:1 +urn:upnp-org:serviceId:L3Forwarding1 +/hou74cq4tw9/IGD/upnp/control/igd/layer3f +/hou74cq4tw9/IGD/upnp/event/igd/layer3f +/hou74cq4tw9/IGD/upnp/Layer3Forwarding.xml + + + + +urn:schemas-upnp-org:device:LANDevice:1 +LANDevice +Technicolor +Technicolor TG784n v3 +A4-B1-E9-D8-F4-78 +uuid:UPnP_Technicolor TG784n v3-1_A4-B1-E9-D8-F4-78_LD_1 + + +urn:schemas-upnp-org:service:LANHostConfigManagement:1 +urn:upnp-org:serviceId:LANHostCfg1 +/hou74cq4tw9/IGD/upnp/control/igd/lanhcm_1 + +/hou74cq4tw9/IGD/upnp/LANHostConfigManagement.xml + + + + +urn:schemas-upnp-org:device:WANDevice:1 +WANDevice +Technicolor +Technicolor TG784n v3 +A4-B1-E9-D8-F4-78 +uuid:UPnP_Technicolor TG784n v3-1_A4-B1-E9-D8-F4-78_WD_1 + + +urn:schemas-upnp-org:service:WANCommonInterfaceConfig:1 +urn:upnp-org:serviceId:WANCommonIFC1 +/hou74cq4tw9/IGD/upnp/control/igd/wancic_1 +/hou74cq4tw9/IGD/upnp/event/igd/wancic_1 +/hou74cq4tw9/IGD/upnp/WANCommonInterfaceConfig.xml + + + + +urn:schemas-upnp-org:device:WANConnectionDevice:1 +WANConnectionDevice +Technicolor +Technicolor TG784n v3 +A4-B1-E9-D8-F4-78 +uuid:UPnP_Technicolor TG784n v3-1_A4-B1-E9-D8-F4-78_WCD_1_1 + + +urn:schemas-upnp-org:service:WANDSLLinkConfig:1 +urn:upnp-org:serviceId:WANDSLLinkC1 +/hou74cq4tw9/IGD/upnp/control/igd/wandsllc_1_1 +/hou74cq4tw9/IGD/upnp/event/igd/wandsllc_1_1 +/hou74cq4tw9/IGD/upnp/WANDSLLinkConfig.xml + + +urn:schemas-upnp-org:service:WANPPPConnection:1 +urn:upnp-org:serviceId:WANPPPConn1 +/hou74cq4tw9/IGD/upnp/control/igd/wanpppc_1_1_1 +/hou74cq4tw9/IGD/upnp/event/igd/wanpppc_1_1_1 +/hou74cq4tw9/IGD/upnp/WANPPPConnection.xml + + + + + + + + diff --git a/upnp/upnp.go b/upnp/upnp.go index df9e73fd..be6560a9 100644 --- a/upnp/upnp.go +++ b/upnp/upnp.go @@ -14,6 +14,7 @@ import ( "encoding/xml" "errors" "fmt" + "io" "io/ioutil" "net" "net/http" @@ -24,6 +25,7 @@ import ( type IGD struct { serviceURL string + device string ourIP string } @@ -103,7 +105,7 @@ Mx: 3 return nil, errors.New("no location") } - serviceURL, err := getServiceURL(locURL) + serviceURL, device, err := getServiceURL(locURL) if err != nil { return nil, err } @@ -119,6 +121,7 @@ Mx: 3 igd := &IGD{ serviceURL: serviceURL, + device: device, ourIP: ourIP, } return igd, nil @@ -162,49 +165,57 @@ func getChildService(d upnpDevice, serviceType string) (upnpService, bool) { return upnpService{}, false } -func getServiceURL(rootURL string) (string, error) { +func getServiceURL(rootURL string) (string, string, error) { r, err := http.Get(rootURL) if err != nil { - return "", err + return "", "", err } defer r.Body.Close() if r.StatusCode >= 400 { - return "", errors.New(r.Status) + return "", "", errors.New(r.Status) } + return getServiceURLReader(rootURL, r.Body) +} +func getServiceURLReader(rootURL string, r io.Reader) (string, string, error) { var upnpRoot upnpRoot - err = xml.NewDecoder(r.Body).Decode(&upnpRoot) + err := xml.NewDecoder(r).Decode(&upnpRoot) if err != nil { - return "", err + return "", "", err } dev := upnpRoot.Device if dev.DeviceType != "urn:schemas-upnp-org:device:InternetGatewayDevice:1" { - return "", errors.New("No InternetGatewayDevice") + return "", "", errors.New("No InternetGatewayDevice") } dev, ok := getChildDevice(dev, "urn:schemas-upnp-org:device:WANDevice:1") if !ok { - return "", errors.New("No WANDevice") + return "", "", errors.New("No WANDevice") } dev, ok = getChildDevice(dev, "urn:schemas-upnp-org:device:WANConnectionDevice:1") if !ok { - return "", errors.New("No WANConnectionDevice") + return "", "", errors.New("No WANConnectionDevice") } - svc, ok := getChildService(dev, "urn:schemas-upnp-org:service:WANIPConnection:1") + device := "urn:schemas-upnp-org:service:WANIPConnection:1" + svc, ok := getChildService(dev, device) if !ok { - return "", errors.New("No WANIPConnection") + device = "urn:schemas-upnp-org:service:WANPPPConnection:1" + } + svc, ok = getChildService(dev, device) + if !ok { + return "", "", errors.New("No WANIPConnection nor WANPPPConnection") } if len(svc.ControlURL) == 0 { - return "", errors.New("no controlURL") + return "", "", errors.New("no controlURL") } u, _ := url.Parse(rootURL) replaceRawPath(u, svc.ControlURL) - return u.String(), nil + return u.String(), device, nil } func replaceRawPath(u *url.URL, rp string) { @@ -223,7 +234,7 @@ func replaceRawPath(u *url.URL, rp string) { u.RawQuery = q } -func soapRequest(url, function, message string) error { +func soapRequest(url, device, function, message string) error { tpl := ` %s @@ -237,7 +248,7 @@ func soapRequest(url, function, message string) error { } req.Header.Set("Content-Type", `text/xml; charset="utf-8"`) req.Header.Set("User-Agent", "syncthing/1.0") - req.Header.Set("SOAPAction", `"urn:schemas-upnp-org:service:WANIPConnection:1#`+function+`"`) + req.Header.Set("SOAPAction", fmt.Sprintf(`"%s#%s"`, device, function)) req.Header.Set("Connection", "Close") req.Header.Set("Cache-Control", "no-cache") req.Header.Set("Pragma", "no-cache") @@ -280,7 +291,7 @@ func (n *IGD) AddPortMapping(protocol Protocol, externalPort, internalPort int, ` body := fmt.Sprintf(tpl, externalPort, protocol, internalPort, n.ourIP, description, timeout) - return soapRequest(n.serviceURL, "AddPortMapping", body) + return soapRequest(n.serviceURL, n.device, "AddPortMapping", body) } func (n *IGD) DeletePortMapping(protocol Protocol, externalPort int) (err error) { @@ -292,5 +303,5 @@ func (n *IGD) DeletePortMapping(protocol Protocol, externalPort int) (err error) ` body := fmt.Sprintf(tpl, externalPort, protocol) - return soapRequest(n.serviceURL, "DeletePortMapping", body) + return soapRequest(n.serviceURL, n.device, "DeletePortMapping", body) } diff --git a/upnp/upnp_test.go b/upnp/upnp_test.go new file mode 100644 index 00000000..3d9f6e94 --- /dev/null +++ b/upnp/upnp_test.go @@ -0,0 +1,17 @@ +package upnp + +import ( + "os" + "testing" +) + +func TestGetTechnicolorRootURL(t *testing.T) { + r, _ := os.Open("testdata/technicolor.xml") + _, action, err := getServiceURLReader("http://localhost:1234/", r) + if err != nil { + t.Fatal(err) + } + if action != "urn:schemas-upnp-org:service:WANPPPConnection:1" { + t.Error("Unexpected action", action) + } +}