From 710bff8937286d88cda10a9144f1eae16fccba09 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Tue, 18 Aug 2026 21:35:12 +0000 Subject: [PATCH] Fix cp/mv -I dropping all stdin paths after the first two Since 5.36, `gsutil cp -I` (and `mv -I`, which shares the code path) copies only the first two paths from stdin and exits 0, silently ignoring the rest. Regression from 13606f78 (Add Python 3.14 support), which wrapped combined_src_urls in list(itertools.chain(...)) because itertools objects are no longer picklable on 3.14. With -I, src_url_str is a one-shot StdinIteratorCls already handed to the NameExpansionIterator built a few lines earlier, whose plurality check buffers up to two entries. The list() call then drains the rest of stdin, so the copy loop only ever sees those two. combined_src_urls only feeds SeekAheadNameExpansionIterator, which RunCommand already skips for stdin sources, so skip populating it in that case and leave the stdin iterator alone. Add a unit test copying five files via -I; it fails on the previous code with only two copied. The existing integration test used two files, which the bug cannot drop. Fixes #1908 --- gslib/commands/cp.py | 11 +++++++++-- gslib/tests/test_cp.py | 27 +++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/gslib/commands/cp.py b/gslib/commands/cp.py index 5001036b6..a0bb9cb46 100644 --- a/gslib/commands/cp.py +++ b/gslib/commands/cp.py @@ -1043,8 +1043,15 @@ def _ConstructNameExpansionIteratorDstTupleIterator(self, src_url_strs_iter, self.has_file_dst = self.has_file_dst or exp_dst_url.IsFileUrl() self.has_cloud_dst = self.has_cloud_dst or exp_dst_url.IsCloudUrl() self.provider_types.add(exp_dst_url.scheme) - self.combined_src_urls = list(itertools.chain(self.combined_src_urls, - src_url_str)) + # combined_src_urls is only consumed by SeekAheadNameExpansionIterator, + # which is not used when reading sources from stdin (-I). Do not touch + # src_url_str in that case: it is a one-shot stdin iterator that is + # already being consumed by the NameExpansionIterator above, and + # materializing it here would silently drop every path that has not + # yet been read by the copy loop. + if not copy_helper_opts.read_args_from_stdin: + self.combined_src_urls = list( + itertools.chain(self.combined_src_urls, src_url_str)) yield name_expansion_iterator_dst_tuple diff --git a/gslib/tests/test_cp.py b/gslib/tests/test_cp.py index bc2ad8596..56a4773ba 100644 --- a/gslib/tests/test_cp.py +++ b/gslib/tests/test_cp.py @@ -24,6 +24,7 @@ import binascii import datetime import gzip +import io import logging import os import pickle @@ -5117,6 +5118,32 @@ def test_storage_class_on_local_destination_fails(self): suri(bucket_uri, 'object'), 'local_file' ]) + def test_read_args_from_stdin_copies_all_paths(self): + """Tests that cp -I copies every path from stdin, not just the first two. + + Regression test for a bug where the stdin iterator was materialized into a + list while the name expansion iterator (which had already buffered two + entries for its plurality check) still held a reference to it, causing + all paths after the second one to be silently dropped. + """ + src_dir = self.CreateTempDir() + dst_dir = self.CreateTempDir() + num_files = 5 + fpaths = [ + self.CreateTempFile(tmpdir=src_dir, + file_name='f%d' % i, + contents=('data%d' % i).encode('ascii')) + for i in range(num_files) + ] + stdin_lines = '\n'.join(fpaths) + '\n' + with mock.patch('sys.stdin', io.StringIO(stdin_lines)): + self.RunCommand('cp', ['-I', dst_dir]) + copied = sorted(os.listdir(dst_dir)) + self.assertEqual(copied, ['f%d' % i for i in range(num_files)]) + for i in range(num_files): + with open(os.path.join(dst_dir, 'f%d' % i), 'rb') as f: + self.assertEqual(f.read(), ('data%d' % i).encode('ascii')) + def test_read_args_from_stdin_with_source_urls_fails(self): bucket_uri = self.CreateBucket() with self.assertRaisesRegex(