Bug #18480

Arv-put tries to open non-regular files

Added by Lucas Di Pentima 11 days ago. Updated 7 days ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
SDKs
Target version:
Start date:
11/26/2021
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
-
Release relationship:
Auto

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:

Subtasks

Task #18483: Review 18480-arvput-special-files-handlingResolvedLucas Di Pentima

History

#1 Updated by Tom Clegg 10 days 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.

#2 Updated by Lucas Di Pentima 10 days 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.

#3 Updated by Lucas Di Pentima 10 days ago

Updates at 087fe7a4f - branch 18480-arvput-special-files-handling
Test run: https://ci.arvados.org/job/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.

#4 Updated by Lucas Di Pentima 10 days ago

Updates at ee12db851
Test run: https://ci.arvados.org/job/developer-run-tests/2817/

  • Improves test confirming that symlinked directories aren't affected.
  • Terminates producer process before any assertion, to avoid potencial test lockups.

#5 Updated by Tom Clegg 10 days 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!

#6 Updated by Lucas Di Pentima 10 days ago

  • % Done changed from 0 to 100
  • Status changed from New to Resolved

Applied in changeset arvados-private:commit:arvados|10397a28667ac68a174c916124a80c04fb16062c.

Also available in: Atom PDF