Refactor: request-example openfile

[puellanivis]

While looking into Issue-383, I realized that there were a small bunch of corner cases not being covered properly. This ended up being a bit more than just the openfile refactor, but in the end ends up with much improved semantics when dealing with the MemFile handler.

During development a large number of tests started panicking when I broke file creation. The tests should not panic, and their assert.Nil or assert.NoError tests have been replaced with a require.NoError. All assert.Nil of error values have been replaced with either assert.NoError or require.NoError.

As such there are a heavy number of test lines changed, which do not actually change any behavior of the tests at all.

Resolved:

• O_TRUNC was being ignored on existing files.
• O_EXCL and O_CREATE were being ignored.
• in most places where symlinks should be followed, they are now followed until a non-symlink file/dir, or os.ErrNotExist
• infinite symlinks will eventually error; following 5 symlinks deep will assume a symlink loop, and error
• creating dangling symlink files should actually be allowed, mkdir dangling symlinks should fail
• O_CREATE with a dangling symlink should create the target filename, not the symlink‘s name
• a file being made in a symlink to a directory should be made under the target directory, not under the symlink
• new symlinks were not checking for an existing parent directory
• symlink should not be able to overwrite an existing file/dir/symlink
• new directories were not checking for an existing parent directory
• mkdir should not be able to overwrite an existing file/dir/symlink
• new file/dir/symlink creation has been unified to ensure proper locating at a canonical name is made
• rmdir should only work on directories
• remove should only work on files

Known issue:

• Absolutely sure there are still a ton of symlink corner-case issues. These would require a pretty complete and total redesign though.

[drakkan]

Hi @puellanivis this seems a great work, thank you!

[puellanivis]

OK, I think all that is left, is a bit of verification of SSH vs POSIX semantics (like rename does not allow overwrite in SSH, but does in POSIX) but I think there’s a bit of deeper problems with compliance on that, i.e. the normal _RENAME packet actually calls os.Rename(‌…) which will actually replace existing files, unilike SSH standard. 🙃

(also an escape hatch for too many symlinks followed)

[puellanivis]

Alright, getting any more compliance of out this fake system would definitely require significantly more effort, and a pretty complete overhaul. So, I’m going to leave it here, and we can attack potential further non-compliance issues as they come, or reimplement the in-memory-filesystem more properly in another PR.

I have taken the time to expand support for a PosixRenameFileCmder in the style of the OpenFileFileWriter and LstatFileLister so that the proper semantics of SSH_FXP_RENAME not allowing for replacing existing files can be maintained at least for our in-memory-filesystem.

[drakkan]

It looks good to me, thank you!