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

READ (only) default mode in SftpFileSystemProvider.newFileChannel() #372

Merged
merged 1 commit into from
Jun 3, 2023

Conversation

hannibal218bc
Copy link
Contributor

According to upstream javadoc, the default mode is READ (only) when there is no mode specified:

The READ and WRITE options determine if the file should be opened for reading and/or writing. If neither option (or the APPEND option) is contained in the array then the file is opened for reading

SftpFileSystemProvider used READ, WRITE which leads to Permission denied errors when trying to access read-only files (see this Stackoverflow thread).

This MR suggests to use a standard-complaint setting; however, this might cause regressions...

@tomaswolf
Copy link
Member

Thanks for this fix. Unfortunately tests are failing.

A quick fix might be to open the channel for writing in that failing test method.

But it seems that the locking implementation doesn't map exceptions correctly when channels are opened for reading only (server just returns a "General Failure"); surely that could be improved. And moreover, the javadoc on FileChannel.lock() says a NonWritableChannelException should be thrown if the channel wasn't writeable. It should be possible to check that on the client side before even making any remote call.

We have no tests for other combinations (read lock on a write-only channel).

@tomaswolf
Copy link
Member

Created bug #383 for this.

According to upstream javadoc, the default mode is READ (only) when there is no mode specified.
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.

2 participants