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

kqueue: don't set up watchers on unreadable files #479

Merged
merged 1 commit into from Aug 6, 2022
Merged

Conversation

arp242
Copy link
Member

@arp242 arp242 commented Jul 31, 2022

Watching an entire directory would fail if it contains a file we can't
read due to permission errors. That's obviously not ideal, so just skip
the file.

Also add a test for trying to watch a directory where the permissions
are denied, and test the error is what's expected; this prevents
regressions like in #393.

Fixes #439

@arp242
Copy link
Member Author

arp242 commented Jul 31, 2022

Not sure why the BSD tests are failing; for example for FreeBSD:

--- FAIL: TestWatch (0.00s)
	--- FAIL: TestWatch/file_in_directory_is_not_readable (0.98s)
		helpers_test.go:33: 
			have:
				WRITE                "/file"
				REMOVE               "/file"
				REMOVE               "/file-unreadable"
			want:
				WRITE                "/file"
				REMOVE               "/file"
--- FAIL: TestAdd (0.00s)
	--- FAIL: TestAdd/permission_denied (0.22s)
		integration_test.go:504: error is nil

But if I test locally it's fine 🤔

[~cg/fsnotify](kqueue-unreadable)% go-on freebsd go test -race
go-on: freebsd already started; nothing to do
FreeBSD 13.1-RELEASE
PASS
ok      github.com/fsnotify/fsnotify    9.030s

I think the chmod is not running properly for some reason.

@arp242
Copy link
Member Author

arp242 commented Aug 1, 2022

Oh, the NetBSD failure has a clue:

Pseudo-terminal will not be allocated because stdin is not a terminal.
We recommend that you create a non-root account and use su(1) for root access.

So I guess the tests are always run as root, and root can always access everything :-/

I fixed it for FreeBSD; need to look at the others.

@arp242 arp242 requested a review from a team August 1, 2022 11:06
@arp242 arp242 force-pushed the kqueue-unreadable branch 2 times, most recently from 9d9283e to fc8c806 Compare August 6, 2022 16:18
Watching an entire directory would fail if it contains a file we can't
read due to permission errors. That's obviously not ideal, so just skip
the file.

Also add a test for trying to watch a directory where the permissions
are denied, and test the error is what's expected; this prevents
regressions like in #393.

Fixes #439
@arp242 arp242 merged commit 8ab3b84 into main Aug 6, 2022
@arp242 arp242 deleted the kqueue-unreadable branch August 6, 2022 16:40
@shogo82148 shogo82148 mentioned this pull request Mar 6, 2024
25 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Directory partially watched if a file fails to open during addWatch()
1 participant