8000 gh-116931: Add fileobj parameter check for Tarfile.addfile by lyc8503 · Pull Request #117988 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content

gh-116931: Add fileobj parameter check for Tarfile.addfile #117988

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

Merged
merged 2 commits into from
Apr 19, 2024

Conversation

lyc8503
Copy link
Contributor
@lyc8503 lyc8503 commented Apr 17, 2024

Related to #116931

Added additional checks for the addfile method in Tarfile.
It now throws an ValueError when the user passes in a non-zero size tarinfo but does not provide a fileobj.
This prevents users from accidentally creating a header without writing anything to it, and avoids affecting subsequent writes.


📚 Documentation preview 📚: https://cpython-previews--117988.org.readthedocs.build/

Added additional checks for the addfile method in Tarfile.
It now throws an ValueError when the user passes in a non-zero size tarinfo but does not provide a fileobj,
instead of writing an incomplete entry.
@lyc8503 lyc8503 requested a review from ethanfurman as a code owner April 17, 2024 13:51
@ghost
Copy link
ghost commented Apr 17, 2024

All commit authors signed the Contributor License Agreement.
CLA signed

@encukou encukou enabled auto-merge (squash) April 18, 2024 13:47
@encukou encukou merged commit 15b3555 into python:main Apr 19, 2024
33 checks passed
@lyc8503 lyc8503 deleted the gh-116931 branch April 19, 2024 12:28
pbo-linaro pushed a commit to pbo-linaro/qemu-ci that referenced this pull request Feb 28, 2025
This test depends on TarFile.addfile() to add tar member header without
writing the member data, which we write ourself using qemu-nbd. Python
3.13 changed the function in a backward incompatible way[1] to require a
file object for tarinfo with non-zero size, breaking the test:

     -[{"name": "vm.ovf", "offset": 512, "size": 6}, {"name": "disk", "offset": 1536, "size": 393216}]
     +Traceback (most recent call last):
     +  File "/home/stefanha/qemu/tests/qemu-iotests/302", line 118, in <module>
     +    tar.addfile(disk)
     +    ~~~~~~~~~~~^^^^^^
     +  File "/usr/lib64/python3.13/tarfile.py", line 2262, in addfile
     +    raise ValueError("fileobj not provided for non zero-size regular file")
     +ValueError: fileobj not provided for non zero-size regular file

The new behavior makes sense for most users, but breaks our unusual
usage. Fix the test to add the member header directly using public but
undocumented attributes. This is more fragile but the test works again.

This also fixes a bug in the previous code - when calling addfile()
without a fileobject, tar.offset points to the start of the member data
instead of the end.

[1] python/cpython#117988

Signed-off-by: Nir Soffer <nirsof@gmail.com>
pbo-linaro pushed a commit to pbo-linaro/qemu-ci that referenced this pull request Feb 28, 2025
https://lore.kernel.org/qemu-devel/20250228195708.48035-1-nirsof@gmail.com

---

From: Nir Soffer <nirsof@gmail.com>
To: qemu-devel@nongnu.org
Cc: qemu-block@nongnu.org, Hanna Reitz <hreitz@redhat.com>,
 Stefan Hajnoczi <stefanha@redhat.com>, Kevin Wolf <kwolf@redhat.com>,
 Nir Soffer <nirsof@gmail.com>
Subject: [PATCH v2] iotest: Unbreak 302 with python 3.13
Date: Fri, 28 Feb 2025 21:57:08 +0200
Message-Id: <20250228195708.48035-1-nirsof@gmail.com>
X-Mailer: git-send-email 2.39.5 (Apple Git-154)
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Received-SPF: pass client-ip=2a00:1450:4864:20::430;
 envelope-from=nirsof@gmail.com; helo=mail-wr1-x430.google.com
X-Spam_score_int: -20
X-Spam_score: -2.1
X-Spam_bar: --
X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1,
 DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF
8000
=-0.1, FREEMAIL_FROM=0.001,
 RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001,
 SPF_PASS=-0.001 autolearn=ham autolearn_force=no
X-Spam_action: no action
X-BeenThere: qemu-devel@nongnu.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
 <mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <https://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
 <mailto:qemu-devel-request@nongnu.org?subject=subscribe>
Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org
Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org

This test depends on TarFile.addfile() to add tar member header without
writing the member data, which we write ourself using qemu-nbd. Python
3.13 changed the function in a backward incompatible way[1] to require a
file object for tarinfo with non-zero size, breaking the test:

     -[{"name": "vm.ovf", "offset": 512, "size": 6}, {"name": "disk", "offset": 1536, "size": 393216}]
     +Traceback (most recent call last):
     +  File "/home/stefanha/qemu/tests/qemu-iotests/302", line 118, in <module>
     +    tar.addfile(disk)
     +    ~~~~~~~~~~~^^^^^^
     +  File "/usr/lib64/python3.13/tarfile.py", line 2262, in addfile
     +    raise ValueError("fileobj not provided for non zero-size regular file")
     +ValueError: fileobj not provided for non zero-size regular file

The new behavior makes sense for most users, but breaks our unusual
usage. Fix the test to add the member header directly using public but
undocumented attributes. This is more fragile but the test works again.

This also fixes a bug in the previous code - when calling addfile()
without a fileobject, tar.offset points to the start of the member data
instead of the end.

[1] python/cpython#117988

Signed-off-by: Nir Soffer <nirsof@gmail.com>
---
 tests/qemu-iotests/302 | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/302 b/tests/qemu-iotests/302
index a6d79e7..e980ec5 100755
--- a/tests/qemu-iotests/302
+++ b/tests/qemu-iotests/302
@@ -115,13 +115,22 @@ with tarfile.open(tar_file, "w") as tar:

     disk = tarfile.TarInfo("disk")
     disk.size = actual_size
-    tar.addfile(disk)

-    # 6. Shrink the tar to the actual size, aligned to 512 bytes.
+    # Since python 3.13 we cannot use addfile() to create the member header.
+    # Add the tarinfo directly using public but undocumented attributes.

-    tar_size = offset + (disk.size + 511) & ~511
-    tar.fileobj.seek(tar_size)
-    tar.fileobj.truncate(tar_size)
+    buf = disk.tobuf(tar.format, tar.encoding, tar.errors)
+    tar.fileobj.write(buf)
+    tar.members.append(disk)
+
+    # Update the offset and position to the location of the next member.
+
+    tar.offset = offset + (disk.size + 511) & ~511
+    tar.fileobj.seek(tar.offset)
+
+    # 6. Shrink the tar to the actual size.
+
+    tar.fileobj.truncate(tar.offset)

 with tarfile.open(tar_file) as tar:
     members = [{"name": m.name, "size": m.size, "offset": m.offset_data}
--
2.39.5 (Apple Git-154)

Signed-off-by: GitHub Actions Bot <bot@github.com>
pbo-linaro pushed a commit to pbo-linaro/qemu-ci that referenced this pull request Mar 5, 2025
This test depends on TarFile.addfile() to add tar member header without
writing the member data, which we write ourself using qemu-nbd. Python
3.13 changed the function in a backward incompatible way[1] to require a
file object for tarinfo with non-zero size, breaking the test:

     -[{"name": "vm.ovf", "offset": 512, "size": 6}, {"name": "disk", "offset": 1536, "size": 393216}]
     +Traceback (most recent call last):
     +  File "/home/stefanha/qemu/tests/qemu-iotests/302", line 118, in <module>
     +    tar.addfile(disk)
     +    ~~~~~~~~~~~^^^^^^
     +  File "/usr/lib64/python3.13/tarfile.py", line 2262, in addfile
     +    raise ValueError("fileobj not provided for non zero-size regular file")
     +ValueError: fileobj not provided for non zero-size regular file

The new behavior makes sense for most users, but breaks our unusual
usage. Fix the test to add the member header directly using public but
undocumented attributes. This is more fragile but the test works again.

This also fixes a bug in the previous code - when calling addfile()
without a fileobject, tar.offset points to the start of the member data
instead of the end.

[1] python/cpython#117988

Signed-off-by: Nir Soffer <nirsof@gmail.com>
Message-ID: <20250228195708.48035-1-nirsof@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
stsquad pushed a commit to qemu/qemu that referenced this pull request Mar 6, 2025
This test depends on TarFile.addfile() to add tar member header without
writing the member data, which we write ourself using qemu-nbd. Python
3.13 changed the function in a backward incompatible way[1] to require a
file object for tarinfo with non-zero size, breaking the test:

     -[{"name": "vm.ovf", "offset": 512, "size": 6}, {"name": "disk", "offset": 1536, "size": 393216}]
     +Traceback (most recent call last):
     +  File "/home/stefanha/qemu/tests/qemu-iotests/302", line 118, in <module>
     +    tar.addfile(disk)
     +    ~~~~~~~~~~~^^^^^^
     +  File "/usr/lib64/python3.13/tarfile.py", line 2262, in addfile
     +    raise ValueError("fileobj not provided for non zero-size regular file")
     +ValueError: fileobj not provided for non zero-size regular file

The new behavior makes sense for most users, but breaks our unusual
usage. Fix the test to add the member header directly using public but
undocumented attributes. This is more fragile but the test works again.

This also fixes a bug in the previous code - when calling addfile()
without a fileobject, tar.offset points to the start of the member data
instead of the end.

[1] python/cpython#117988

Signed-off-by: Nir Soffer <nirsof@gmail.com>
Message-ID: <20250228195708.48035-1-nirsof@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
< 3F6B create-branch data-default-repo="python/cpython" data-selected-nwo="python/cpython" data-default-source-branch="main" data-sidebar-url="/python/cpython/issues/closing_references/partials/sidebar?source_id=2248357433&source_type=ISSUE" class="discussion-sidebar-item d-block">
Development

Successfully merging this pull request may close these issues.

2 participants
0