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

{dest-path} ending in '/' result in registered file named ' ' #70

Open
BrianJKoopman opened this issue Feb 3, 2020 · 2 comments
Open

Comments

@BrianJKoopman
Copy link

I realize I'm probably breaking the rules in my file naming during manual testing and that this case might not come up when programmatically uploading directories, but in testing I've successfully uploading directories with {local-path} containing a trailing slash:

$ librarian upload --null-obsid TestUser directory_test/ foo/directory_test

However, I find that if I have a trailing slash on the {dest-path} (might be easy to do manually if tab completing a directory and not giving the destination any parent directory), things go wrong:

$ librarian upload --null-obsid TestUser directory_test/ directory_test11/
error: upload failed: RPC call {'store_name': 'host2store', 'staging_dir': 'staging.uC98iT', 'dest_store_path': 'directory_test11/', 'meta_mode': 'infer', 'deletion_policy': 'disallowed', 'staging_was_known': False, 'null_obsid': True, 'authenticator': 'I am a bot'} failed: cannot move upload to its destination (is there already a file there, unknown to this Librarian?): RPC call ['ssh', 'Host2', "mkdir -p '/tmp/librarian/directory_test11' && chmod u+w '/tmp/librarian/staging.uC98iT/' && mv -nT '/tmp/librarian/staging.uC98iT/' '/tmp/librarian/directory_test11/' && test ! -e '/tmp/librarian/staging.uC98iT/' && chmod -R 'ugoa-w' '/tmp/librarian/directory_test11/'"] failed: exit code 1; stdout:

''

stderr:

''

The librarian webpage then reports a file existing with the name of ' ':

Name Created Observation Type Source Size
details 2020-02-03 17:14:52 null uC98iT/ TestUser 36 Bytes

I understand why, given the rules for naming, this might error out, but I think the ' ' file registering might be a bug.

@pkgw
Copy link
Contributor

pkgw commented Feb 4, 2020

Definitely a bug, the interfaces should sanitize file paths for things like trailing slashes. Other fun things to think about are trailing /., internal /../, etc.

@plaplant
Copy link
Member

plaplant commented Feb 4, 2020

We can probably handle most of these issues in a self-consistent way by using pathlib to canonicalize file/directory paths as part of uploading. I can try to take a stab at this in the next ~week or so.

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

No branches or pull requests

3 participants