kqueue: avoid infinite loops from symlinks cycles

The semenatics of using kqueue are already slightly different than
inotify, specifically that inotify will give you events for the symlink
itself whereas kqueue will only give events for the target of the
symlink. This preserves that behaviour, that symlinks don't get notified
on, but ensures that circular symlinks don't recurse forever.

This change pushes the resolved filename up through the stack so that
the fileExists map can be properly populated; not doing so would mean
that deleting a cyclical symlink and then replacing it with a file of
the same name would not notify for that file's creation (or subsequent
events) because we would believe that the file already existed.

diff --git a/AUTHORS b/AUTHORS
index 05b3f3eecc9a098cade38aba0baf007b505239f6..763b853c37d523d0c19ff2e728bb42d6a081d285 100644 (file)
--- a/AUTHORS
+++ b/AUTHORS
@@ -13,6 +13,7 @@ Caleb Spare <cespare@gmail.com>
 Case Nelson <case@teammating.com>
 Chris Howey <chris@howey.me> <howeyc@gmail.com>
 Christoffer Buchholz <christoffer.buchholz@gmail.com>
+Daniel Wagner-Hall <dawagner@gmail.com>
 Dave Cheney <dave@cheney.net>
 Evan Phoenix <evan@fallingsnow.net>
 Francisco Souza <f@souza.cc>

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 208bcd740c40900b45d5f06360f05c485c77973d..4e6672702f6fce25796c207bbeb1f37de44088d7 100644 (file)
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,10 +1,11 @@
 # Changelog
 
-## v1.2.4 / 2015-10-17
+## v1.2.5 / 2015-10-17
 
 * inotify: use epoll_create1 for arm64 support (requires Linux 2.6.27 or later) [#100](https://github.com/go-fsnotify/fsnotify/pull/100) (thanks @suihkulokki)
 * inotify: fix path leaks [#73](https://github.com/go-fsnotify/fsnotify/pull/73) (thanks @chamaken)
 * kqueue: watch for rename events on subdirectories [#83](https://github.com/go-fsnotify/fsnotify/pull/83) (thanks @guotie)
+* kqueue: avoid infinite loops from symlinks cycles [#101](https://github.com/go-fsnotify/fsnotify/pull/101) (thanks @illicitonion)
 
 ## v1.2.1 / 2015-10-14
 

diff --git a/integration_test.go b/integration_test.go
index 59169c6afa8819ca9bd065ec2b2acbbedd07d4da..49e48ff1ca7da214c06b001cbac5ecf426825f83 100644 (file)
--- a/integration_test.go
+++ b/integration_test.go
@@ -10,6 +10,7 @@ import (
        "io/ioutil"
        "os"
        "os/exec"
+       "path"
        "path/filepath"
        "runtime"
        "sync/atomic"
@@ -1065,6 +1066,53 @@ func TestFsnotifyFakeSymlink(t *testing.T) {
        watcher.Close()
 }
 
+func TestCyclicSymlink(t *testing.T) {
+       if runtime.GOOS == "windows" {
+               t.Skip("symlinks don't work on Windows.")
+       }
+
+       watcher := newWatcher(t)
+
+       testDir := tempMkdir(t)
+       defer os.RemoveAll(testDir)
+
+       link := path.Join(testDir, "link")
+       if err := os.Symlink(".", link); err != nil {
+               t.Fatalf("could not make symlink: %v", err)
+       }
+       addWatch(t, watcher, testDir)
+
+       var createEventsReceived counter
+       go func() {
+               for ev := range watcher.Events {
+                       if ev.Op&Create == Create {
+                               createEventsReceived.increment()
+                       }
+               }
+       }()
+
+       if err := os.Remove(link); err != nil {
+               t.Fatalf("Error removing link: %v", err)
+       }
+
+       // It would be nice to be able to expect a delete event here, but kqueue has
+       // no way for us to get events on symlinks themselves, because opening them
+       // opens an fd to the file to which they point.
+
+       if err := ioutil.WriteFile(link, []byte("foo"), 0700); err != nil {
+               t.Fatalf("could not make symlink: %v", err)
+       }
+
+       // We expect this event to be received almost immediately, but let's wait 500 ms to be sure
+       time.Sleep(500 * time.Millisecond)
+
+       if got := createEventsReceived.value(); got == 0 {
+               t.Errorf("want at least 1 create event got %v", got)
+       }
+
+       watcher.Close()
+}
+
 // TestConcurrentRemovalOfWatch tests that concurrent calls to RemoveWatch do not race.
 // See https://codereview.appspot.com/103300045/
 // go test -test.run=TestConcurrentRemovalOfWatch -test.cpu=1,1,1,1,1 -race

diff --git a/kqueue.go b/kqueue.go
index 821955f2885024246b2da2df807624ad179061f0..9662a50a84d2aa87ec361c85cedf99b7158872b5 100644 (file)
--- a/kqueue.go
+++ b/kqueue.go
@@ -94,7 +94,8 @@ func (w *Watcher) Add(name string) error {
        w.mu.Lock()
        w.externalWatches[name] = true
        w.mu.Unlock()
-       return w.addWatch(name, noteAllEvents)
+       _, err := w.addWatch(name, noteAllEvents)
+       return err
 }
 
 // Remove stops watching the the named file or directory (non-recursively).
@@ -153,7 +154,8 @@ var keventWaitTime = durationToTimespec(100 * time.Millisecond)
 
 // addWatch adds name to the watched file set.
 // The flags are interpreted as described in kevent(2).
-func (w *Watcher) addWatch(name string, flags uint32) error {
+// Returns the real path to the file which was added, if any, which may be different from the one passed in the case of symlinks.
+func (w *Watcher) addWatch(name string, flags uint32) (string, error) {
        var isDir bool
        // Make ./name and name equivalent
        name = filepath.Clean(name)
@@ -161,7 +163,7 @@ func (w *Watcher) addWatch(name string, flags uint32) error {
        w.mu.Lock()
        if w.isClosed {
                w.mu.Unlock()
-               return errors.New("kevent instance already closed")
+               return "", errors.New("kevent instance already closed")
        }
        watchfd, alreadyWatching := w.watches[name]
        // We already have a watch, but we can still override flags.
@@ -173,17 +175,17 @@ func (w *Watcher) addWatch(name string, flags uint32) error {
        if !alreadyWatching {
                fi, err := os.Lstat(name)
                if err != nil {
-                       return err
+                       return "", err
                }
 
                // Don't watch sockets.
                if fi.Mode()&os.ModeSocket == os.ModeSocket {
-                       return nil
+                       return "", nil
                }
 
                // Don't watch named pipes.
                if fi.Mode()&os.ModeNamedPipe == os.ModeNamedPipe {
-                       return nil
+                       return "", nil
                }
 
                // Follow Symlinks
@@ -195,18 +197,26 @@ func (w *Watcher) addWatch(name string, flags uint32) error {
                if fi.Mode()&os.ModeSymlink == os.ModeSymlink {
                        name, err = filepath.EvalSymlinks(name)
                        if err != nil {
-                               return nil
+                               return "", nil
+                       }
+
+                       w.mu.Lock()
+                       _, alreadyWatching = w.watches[name]
+                       w.mu.Unlock()
+
+                       if alreadyWatching {
+                               return name, nil
                        }
 
                        fi, err = os.Lstat(name)
                        if err != nil {
-                               return nil
+                               return "", nil
                        }
                }
 
                watchfd, err = syscall.Open(name, openMode, 0700)
                if watchfd == -1 {
-                       return err
+                       return "", err
                }
 
                isDir = fi.IsDir()
@@ -215,7 +225,7 @@ func (w *Watcher) addWatch(name string, flags uint32) error {
        const registerAdd = syscall.EV_ADD | syscall.EV_CLEAR | syscall.EV_ENABLE
        if err := register(w.kq, []int{watchfd}, registerAdd, flags); err != nil {
                syscall.Close(watchfd)
-               return err
+               return "", err
        }
 
        if !alreadyWatching {
@@ -229,6 +239,7 @@ func (w *Watcher) addWatch(name string, flags uint32) error {
                // Watch the directory if it has not been watched before,
                // or if it was watched before, but perhaps only a NOTE_DELETE (watchDirectoryFiles)
                w.mu.Lock()
+
                watchDir := (flags&syscall.NOTE_WRITE) == syscall.NOTE_WRITE &&
                        (!alreadyWatching || (w.dirFlags[name]&syscall.NOTE_WRITE) != syscall.NOTE_WRITE)
                // Store flags so this watch can be updated later
@@ -237,11 +248,11 @@ func (w *Watcher) addWatch(name string, flags uint32) error {
 
                if watchDir {
                        if err := w.watchDirectoryFiles(name); err != nil {
-                               return err
+                               return "", err
                        }
                }
        }
-       return nil
+       return name, nil
 }
 
 // readEvents reads from kqueue and converts the received kevents into
@@ -364,7 +375,8 @@ func (w *Watcher) watchDirectoryFiles(dirPath string) error {
 
        for _, fileInfo := range files {
                filePath := filepath.Join(dirPath, fileInfo.Name())
-               if err := w.internalWatch(filePath, fileInfo); err != nil {
+               filePath, err = w.internalWatch(filePath, fileInfo)
+               if err != nil {
                        return err
                }
 
@@ -399,7 +411,8 @@ func (w *Watcher) sendDirectoryChangeEvents(dirPath string) {
                }
 
                // like watchDirectoryFiles (but without doing another ReadDir)
-               if err := w.internalWatch(filePath, fileInfo); err != nil {
+               filePath, err = w.internalWatch(filePath, fileInfo)
+               if err != nil {
                        return
                }
 
@@ -409,7 +422,7 @@ func (w *Watcher) sendDirectoryChangeEvents(dirPath string) {
        }
 }
 
-func (w *Watcher) internalWatch(name string, fileInfo os.FileInfo) error {
+func (w *Watcher) internalWatch(name string, fileInfo os.FileInfo) (string, error) {
        if fileInfo.IsDir() {
                // mimic Linux providing delete events for subdirectories
                // but preserve the flags used if currently watching subdirectory