Bug #18480
closed
Arv-put tries to open non-regular files
100%
Description
A customer reported an issue when trying to use arv-put on paths that include special files (like named pipes), and already provided a patch for it:
--- a/sdk/python/arvados/commands/put.py
+++ b/sdk/python/arvados/commands/put.py
@@ -576,6 +576,9 @@
files.sort()
for f in files:
filepath = os.path.join(root, f)
+ if not os.path.isfile(filepath):
+ self.logger.warning("Skipping non-regular file '{}'".format(filepath))
+ continue
# Add its size to the total bytes count (if applicable)
if self.follow_links or (not os.path.islink(filepath)):
if self.bytes_expected is not None:
Updated by Tom Clegg over 1 year ago
Might need to check isdir() too -- looks like the provided patch will skip symlinks to directories even when follow-symlinks is enabled. But I think skipping special files like fifos and char/block devices would be an improvement. Hard to imagine a use case for actually trying to copy them. Seems like "cat fifo | arv-put -" would always be a better way to do that, if someone actually wants to.
Updated by Lucas Di Pentima over 1 year ago
I think dirs aren't included in that loop, because this part of the code is inside a os.walk() call loop, that handles directories separately.
Updated by Lucas Di Pentima over 1 year ago
Updates at 087fe7a4f - branch 18480-arvput-special-files-handling
Test run: developer-run-tests: #2816
- Adds test exposing the bug.
- Applies suggested changes, fixing the problem.
The symlinked directories aren't affected and we already have a test that confirms that: test_arv_put.ArvPutUploadJobTest.test_symlinks_are_followed_by_default.
Updated by Lucas Di Pentima over 1 year ago
Updates at ee12db851
Test run: developer-run-tests: #2817
- Improves test confirming that symlinked directories aren't affected.
- Terminates producer process before any assertion, to avoid potencial test lockups.
Updated by Tom Clegg over 1 year ago
Aha, yes, dirs are returned separately by os.walk so I didn't need to worry about the symlink-to-dir case.
LGTM, thanks!
Updated by Lucas Di Pentima over 1 year ago
- % Done changed from 0 to 100
- Status changed from New to Resolved
Applied in changeset arvados-private:commit:arvados|10397a28667ac68a174c916124a80c04fb16062c.