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

tarfile-write issue with file/folder permissions #797

Open
ubidefeo opened this issue Feb 3, 2024 · 3 comments
Open

tarfile-write issue with file/folder permissions #797

ubidefeo opened this issue Feb 3, 2024 · 3 comments

Comments

@ubidefeo
Copy link
Contributor

ubidefeo commented Feb 3, 2024

Context

I've been playing with tarfile for a few days and yesterday started trying to create my own archives using tarfile-write installed via mip.

Adding a folder to it works recursively, but transferring the file to the PC via mpremote and expanding it yields all files and folders with --- --- --- permissions (0x000).

Discovery

I investigated and found the source of the issue is related to how the TarInfo object is composed in these lines

in particular on line 102 we find mode to be set to the os.stat(FILE)[0]

tarinfo.mode = stat[0]

stat[0] is either 0x8000 (file) or 0x4000 (folder).
And the subsequently applied masking yields 0x000 as permissions set while adding it to the archive.
I think that mode should be safely set to 0x1a4 > 644 > rw- r-- r--

I have tested by manually composing a TarInfo object to be handed to addfile() rather than patching add()

How to reproduce:

import tarfile
import os

os.mkdir('test_folder')
f = open('test_folder/test_file_01.txt', 'w')
f.write('file 01')
f.close()

os.mkdir('test_folder/sub_folder')

f = open('test_folder/sub_folder/test_file_02.txt', 'w')
f.write('file 02')
f.close()

archive = tarfile.TarFile('test_archive.tar', 'w')
archive.add('test_folder')
archive.close()

Copy the archive from the board to the PC via mpremote cp :test_archive.tar test_archive.tar

Expand the archive and verify the permissions for test_folder.
From Mac os I was not able to chmod 644 test_folder, but using the UI I could unrestrict access.
CleanShot 2024-02-03 at 21 50 23@2x

Looking at the folder content you can verify the same permission issue happens with sub_folder and contained file(s)
CleanShot 2024-02-03 at 21 51 27@2x

Environment:
I run a custom build for ESP32-S3, but this issue is not related to changes I have applied
(name='micropython', version=(1, 23, 0, 'preview'), _machine='LilyGo T-QT Pro with ESP32S3', _mpy=10758)

MicroPython v1.23.0-preview.48.g076516d88.dirty on 2024-01-21; LilyGo T-QT Pro with ESP32S3
Board: ESP32-S3
TarFile version: '0.4.1'

Hope someone can take a look :)
u.

@ubidefeo
Copy link
Contributor Author

ubidefeo commented Feb 5, 2024

upon further digging, the .mode attribute seems to be doing its thing when it comes to TarInfo, and the issue lies into how the Tar header is composed.
Dissecting this line

hdr.mode[:] = b"%07o\0" % (tarinfo.mode & 0o7777)

the permissions always come out as 000

The fix I have tested is to replace that line with

hdr.mode[:] = b"%07o\0" % (0x1ed & 0o7777)

This yields tar files which can be expanded on the host computer, but of course all the files will be 755, which might have some implications if one wants to be picky

@ubidefeo
Copy link
Contributor Author

ubidefeo commented Feb 5, 2024

Moreover, the way the initial mode is set in the TarInfo init is wrong, since it characterises the mode based on the presence of a / at the end of the resource name.
The correct way to do this is to check for os.stat(name)[0]

-        self.mode = _S_IFDIR if self.name[-1] == "/" else _S_IFREG
+        self.mode = _S_IFDIR if os.stat(name)[0] == 0x4000 else _S_IFREG

@projectgus
Copy link
Contributor

I left a review on your PR, but I wanted to say thanks for a very comprehensive and clear bug report and analysis!

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

2 participants