Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert Add AddRaw to not follow symlinks #394

Merged
merged 1 commit into from Aug 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 1 addition & 12 deletions README.md
Expand Up @@ -72,18 +72,10 @@ func main() {
}
}()

// if this is a link, it will follow all the links and watch the file pointed to
err = watcher.Add("/tmp/foo")
if err != nil {
log.Fatal(err)
}

// this will watch the link, rather than the file it points to
err = watcher.AddRaw("/tmp/link")
if err != nil {
log.Fatal(err)
}

<-done
}
```
Expand All @@ -98,10 +90,6 @@ See [example_test.go](https://github.com/fsnotify/fsnotify/blob/master/example_t

## FAQ

**Are symlinks resolved?**
Symlinks are implicitly resolved by [`filepath.EvalSymlinks(path)`](https://golang.org/pkg/path/filepath/#EvalSymlinks) when `watcher.Add(name)` is used. If that is not desired, you can use `watcher.AddRaw(name)` to not follow any symlinks before watching. See [example_test.go](https://github.com/fsnotify/fsnotify/blob/master/example_test.go).


**When a file is moved to another directory is it still being watched?**

No (it shouldn't be, unless you are watching where it was moved to).
Expand Down Expand Up @@ -139,3 +127,4 @@ fsnotify requires support from underlying OS to work. The current NFS protocol d

* [notify](https://github.com/rjeczalik/notify)
* [fsevents](https://github.com/fsnotify/fsevents)

4 changes: 2 additions & 2 deletions fen.go
Expand Up @@ -27,8 +27,8 @@ func (w *Watcher) Close() error {
return nil
}

// AddRaw starts watching the named file or directory (non-recursively). Symlinks are not implicitly resolved.
func (w *Watcher) AddRaw(name string) error {
// Add starts watching the named file or directory (non-recursively).
func (w *Watcher) Add(name string) error {
return nil
}

Expand Down
17 changes: 0 additions & 17 deletions fsnotify.go
Expand Up @@ -12,7 +12,6 @@ import (
"bytes"
"errors"
"fmt"
"path/filepath"
)

// Event represents a single file system notification.
Expand Down Expand Up @@ -68,19 +67,3 @@ func (e Event) String() string {
var (
ErrEventOverflow = errors.New("fsnotify queue overflow")
)

// Add starts watching the named file or directory (non-recursively). Symlinks are implicitly resolved.
func (w *Watcher) Add(name string) error {
rawPath, err := filepath.EvalSymlinks(name)
if err != nil {
return fmt.Errorf("error resolving %#v: %s", name, err)
}
err = w.AddRaw(rawPath)
if err != nil {
if name != rawPath {
return fmt.Errorf("error adding %#v for %#v: %s", rawPath, name, err)
}
return err
}
return nil
}
2 changes: 1 addition & 1 deletion fsnotify_test.go
Expand Up @@ -52,7 +52,7 @@ func TestWatcherClose(t *testing.T) {

name := tempMkFile(t, "")
w := newWatcher(t)
err := w.AddRaw(name)
err := w.Add(name)
if err != nil {
t.Fatal(err)
}
Expand Down
6 changes: 3 additions & 3 deletions inotify.go
Expand Up @@ -88,16 +88,16 @@ func (w *Watcher) Close() error {
return nil
}

// AddRaw starts watching the named file or directory (non-recursively). Symlinks are not implicitly resolved.
func (w *Watcher) AddRaw(name string) error {
// Add starts watching the named file or directory (non-recursively).
func (w *Watcher) Add(name string) error {
name = filepath.Clean(name)
if w.isClosed() {
return errors.New("inotify instance already closed")
}

const agnosticEvents = unix.IN_MOVED_TO | unix.IN_MOVED_FROM |
unix.IN_CREATE | unix.IN_ATTRIB | unix.IN_MODIFY |
unix.IN_MOVE_SELF | unix.IN_DELETE | unix.IN_DELETE_SELF | unix.IN_DONT_FOLLOW
unix.IN_MOVE_SELF | unix.IN_DELETE | unix.IN_DELETE_SELF

var flags uint32 = agnosticEvents

Expand Down
16 changes: 8 additions & 8 deletions inotify_test.go
Expand Up @@ -54,7 +54,7 @@ func TestInotifyCloseSlightlyLaterWithWatch(t *testing.T) {
if err != nil {
t.Fatalf("Failed to create watcher")
}
w.AddRaw(testDir)
w.Add(testDir)

// Wait until readEvents has reached unix.Read, and Close.
<-time.After(50 * time.Millisecond)
Expand All @@ -74,7 +74,7 @@ func TestInotifyCloseAfterRead(t *testing.T) {
t.Fatalf("Failed to create watcher")
}

err = w.AddRaw(testDir)
err = w.Add(testDir)
if err != nil {
t.Fatalf("Failed to add .")
}
Expand Down Expand Up @@ -121,7 +121,7 @@ func TestInotifyCloseCreate(t *testing.T) {
}
defer w.Close()

err = w.AddRaw(testDir)
err = w.Add(testDir)
if err != nil {
t.Fatalf("Failed to add testDir: %v", err)
}
Expand Down Expand Up @@ -149,7 +149,7 @@ func TestInotifyCloseCreate(t *testing.T) {
}

<-time.After(50 * time.Millisecond)
err = w.AddRaw(testDir)
err = w.Add(testDir)
if err != nil {
t.Fatalf("Error adding testDir again: %v", err)
}
Expand All @@ -170,7 +170,7 @@ func TestInotifyStress(t *testing.T) {
}
defer w.Close()

err = w.AddRaw(testDir)
err = w.Add(testDir)
if err != nil {
t.Fatalf("Failed to add testDir: %v", err)
}
Expand Down Expand Up @@ -290,7 +290,7 @@ func TestInotifyRemoveTwice(t *testing.T) {
}
defer w.Close()

err = w.AddRaw(testFile)
err = w.Add(testFile)
if err != nil {
t.Fatalf("Failed to add testFile: %v", err)
}
Expand Down Expand Up @@ -332,7 +332,7 @@ func TestInotifyInnerMapLength(t *testing.T) {
}
defer w.Close()

err = w.AddRaw(testFile)
err = w.Add(testFile)
if err != nil {
t.Fatalf("Failed to add testFile: %v", err)
}
Expand Down Expand Up @@ -384,7 +384,7 @@ func TestInotifyOverflow(t *testing.T) {
t.Fatalf("Cannot create subdir: %v", err)
}

err = w.AddRaw(testSubdir)
err = w.Add(testSubdir)
if err != nil {
t.Fatalf("Failed to add subdir: %v", err)
}
Expand Down
73 changes: 5 additions & 68 deletions integration_test.go
Expand Up @@ -66,8 +66,8 @@ func newWatcher(t *testing.T) *Watcher {

// addWatch adds a watch for a directory
func addWatch(t *testing.T, watcher *Watcher, dir string) {
if err := watcher.AddRaw(dir); err != nil {
t.Fatalf("watcher.AddRaw(%q) failed: %s", dir, err)
if err := watcher.Add(dir); err != nil {
t.Fatalf("watcher.Add(%q) failed: %s", dir, err)
}
}

Expand Down Expand Up @@ -1008,74 +1008,11 @@ func TestFsnotifyClose(t *testing.T) {
testDir := tempMkdir(t)
defer os.RemoveAll(testDir)

if err := watcher.AddRaw(testDir); err == nil {
if err := watcher.Add(testDir); err == nil {
t.Fatal("expected error on Watch() after Close(), got nil")
}
}

func TestSymlinkNotResolved(t *testing.T) {
testDir := tempMkdir(t)
file1 := filepath.Join(testDir, "file1")
file2 := filepath.Join(testDir, "file2")
link := filepath.Join(testDir, "link")

f1, err := os.Create(file1)
if err != nil {
t.Fatalf("Failed to create file1: %s", err)
}
defer f1.Close()
if _, err := os.Create(file2); err != nil {
t.Fatalf("Failed to create file2: %s", err)
}

// symlink works for Windows too
if err := os.Symlink(file1, link); err != nil {
t.Fatalf("Failed to create symlink: %s", err)
}

w, err := NewWatcher()
if err != nil {
t.Fatalf("Failed to create watcher")
}

err = w.AddRaw(link)
if err != nil {
t.Fatalf("Failed to add link: %s", err)
}

// change file 1 - no event
f1.Write([]byte("Hello"))
f1.Sync()
// XXX(Code0x58): doing a create here shows a CHMOD event on mac - is that an issue?

select {
case event := <-w.Events:
t.Fatalf("Event from watcher: %v", event)
case err := <-w.Errors:
t.Fatalf("Error from watcher: %v", err)
case <-time.After(50 * time.Millisecond):
}

// ~atomic link change event
tmpLink := filepath.Join(testDir, "tmp-link")
if err := os.Symlink(file2, tmpLink); err != nil {
t.Fatalf("Failed to create symlink: %s", err)
}

if err := os.Rename(tmpLink, link); err != nil {
t.Fatalf("Failed to replace symlink: %s", err)
}

select {
case _ = <-w.Events:
case err := <-w.Errors:
t.Fatalf("Error from watcher: %v", err)
case <-time.After(50 * time.Millisecond):
t.Fatalf("Took too long to wait for event")
}

}

func TestFsnotifyFakeSymlink(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("symlinks don't work on Windows.")
Expand Down Expand Up @@ -1237,7 +1174,7 @@ func TestClose(t *testing.T) {
defer os.RemoveAll(testDir)

watcher := newWatcher(t)
if err := watcher.AddRaw(testDir); err != nil {
if err := watcher.Add(testDir); err != nil {
t.Fatalf("Expected no error on Add, got %v", err)
}
err := watcher.Close()
Expand All @@ -1258,7 +1195,7 @@ func TestRemoveWithClose(t *testing.T) {
tempFiles = append(tempFiles, tempMkFile(t, testDir))
}
watcher := newWatcher(t)
if err := watcher.AddRaw(testDir); err != nil {
if err := watcher.Add(testDir); err != nil {
t.Fatalf("Expected no error on Add, got %v", err)
}
startC, stopC := make(chan struct{}), make(chan struct{})
Expand Down
32 changes: 29 additions & 3 deletions kqueue.go
Expand Up @@ -91,8 +91,8 @@ func (w *Watcher) Close() error {
return nil
}

// AddRaw starts watching the named file or directory (non-recursively). Symlinks are not implicitly resolved.
func (w *Watcher) AddRaw(name string) error {
// Add starts watching the named file or directory (non-recursively).
func (w *Watcher) Add(name string) error {
w.mu.Lock()
w.externalWatches[name] = true
w.mu.Unlock()
Expand Down Expand Up @@ -190,7 +190,33 @@ func (w *Watcher) addWatch(name string, flags uint32) (string, error) {
return "", nil
}

watchfd, err = unix.Open(name, openMode|unix.O_SYMLINK, 0700)
// Follow Symlinks
// Unfortunately, Linux can add bogus symlinks to watch list without
// issue, and Windows can't do symlinks period (AFAIK). To maintain
// consistency, we will act like everything is fine. There will simply
// be no file events for broken symlinks.
// Hence the returns of nil on errors.
if fi.Mode()&os.ModeSymlink == os.ModeSymlink {
name, err = filepath.EvalSymlinks(name)
if err != 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
}
}

watchfd, err = unix.Open(name, openMode, 0700)
if watchfd == -1 {
return "", err
}
Expand Down
4 changes: 2 additions & 2 deletions windows.go
Expand Up @@ -64,8 +64,8 @@ func (w *Watcher) Close() error {
return <-ch
}

// AddRaw starts watching the named file or directory (non-recursively). Symlinks are not implicitly resolved.
func (w *Watcher) AddRaw(name string) error {
// Add starts watching the named file or directory (non-recursively).
func (w *Watcher) Add(name string) error {
if w.isClosed {
return errors.New("watcher already closed")
}
Expand Down