Skip to content

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

Merged
arp242 merged 1 commit intomainfrom
kqueue-unreadable
Aug 6, 2022
Merged

kqueue: don't set up watchers on unreadable files#479
arp242 merged 1 commit intomainfrom
kqueue-unreadable

Conversation

@arp242
Copy link
Copy Markdown
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
Copy Markdown
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 arp242 force-pushed the kqueue-unreadable branch from 982d3eb to b75b4a0 Compare July 31, 2022 23:59
@arp242
Copy link
Copy Markdown
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
@arp242
kqueue: don't set up watchers on unreadable files
555c656 
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 force-pushed the kqueue-unreadable branch from fc8c806 to 555c656 Compare August 6, 2022 16:19
@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
Open
25 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

1 participant

@arp242