open with SSH_FXF_TRUNC without SSH_FXF_CREAT violates SFTP spec, but everyone does it anyway

[bj0]

It apparrently is against the SFTP spec (but not the POSIX) to open a file with SSH_FXF_TRUNC without including SSH_FXF_CREAT.

Programs like OpenSSH server and sshfs ignore this little detail, though, and programs like "cp" fail if it is followed.

workaround: SFTP frontend should accept FXF_WRITE | FXF_TRUNC.

[davidsarah]

See ​http://tools.ietf.org/html/draft-ietf-secsh-filexfer-02#section-6.3 for the relevant specification of FXF_TRUNC and FXF_CREAT, and ​http://www.opengroup.org/onlinepubs/000095399/functions/open.html for the POSIX open call.

sshfs appears to just pass the POSIX flags along with the obvious mappings.

[bj0]

the SFTP frontend now accepts WRITE | TRUNC, so

cp file1 file2

works.

but doing:

cat file1 > newfile

or

cat file1 >> newfile

when newfile doesn't already exist fails with a:

bash: newfile: No such file or directory

it does create a 'newfile' file, but it has zero length. (and repeating the command will succeed on the existing file)

from strace, it appears to be opening 'newfile' with the flags WRITE | CREAT | TRUNC.

[davidsarah]

The problem in comment:2 is due to the shell assuming that newfile already exists just after the open call. It does a stat which causes a getAttrs request, which fails with NoSuchChildError (FX_NO_SUCH_FILE). Then newfile is closed, but nothing has been written to it, so it is zero-length.

The ticket1037 branch has been changed to keep track of which paths correspond to open files. This allows us to return fake attributes from the getAttrs request (without having to write a zero-length file on the open call). zooko: please review ​this changeset, and bj0: please pull and re-test.

[davidsarah]

Please also test 'touch newfile' before and after pulling the changes. Based on ​https://bugzilla.gnome.org/show_bug.cgi?id=522532 , I think it might have the same issue.

[bj0]

I just checked and

touch newfile

Does work before the latest patches, although it takes like 20s before the new file appears in ls

After the patch, it shows up in ls instantly. Also:

cat test2 > newfile

works.

[davidsarah]

Thanks for the testing.

The current code is not quite right: it indexes the global_open_files dict by canonical path, but this does not include the root node. So, paths for different user accounts could be confused. It should instead index by the directory entry, i.e. parent cap and child name.

The test coverage also needs to be improved to cover cases where the same directory entry is opened more than once, and where files are still open when a connection is logged out.

[davidsarah]

Replying to davidsarah:

The current code is not quite right: it indexes the global_open_files dict by canonical path, but this does not include the root node. So, paths for different user accounts could be confused. It should instead index by the directory entry, i.e. parent cap and child name.

This is now fixed. Also, we now try to support renaming immutable files that have been opened for writing or creation, while they are still open. Many apps depend on this: they create a temporary file, rename it to the destination file, and then close it.

bj0: please re-test both the previous tests, and saving files with gedit and OpenOffice?.

[bj0]

I tried, and got no errors with: cp mv cat > cat >> gedit OpenOffice?

gedit and OO both saved fine it seemed.

One odd thing i noticed was that when i open a file with vi, then close it, the .swp file isn't deleted. I took a quick look at the flog and it looks like it called removeFile and got a SUCCESS, but the file is still there.

[davidsarah]

Assigning to bj0 to check whether the remaining bug in comment:9 still exists. (It should have been fixed by the changes to handle remove/close race conditions.)

[davidsarah]

Fixed according to bj0 on IRC.