Skip to content

fix decompression without dir #962

Open
valoq wants to merge 13 commits intoouch-org:mainfrom
valoq:decompression-fix
Open

fix decompression without dir #962
valoq wants to merge 13 commits intoouch-org:mainfrom
valoq:decompression-fix

Conversation

@valoq
Copy link
Copy Markdown
Collaborator

@valoq valoq commented Apr 25, 2026

Addresses #958

Sorry for the duplicate PR, git hates me today

@valoq
Copy link
Copy Markdown
Collaborator Author

valoq commented Apr 25, 2026

I changed the decompression behavior that was added in 0.7.1 back to the one in 0.6.1 where archived decompressed in the current directory are written into a new directory with the archive name.

Why extracting the a new dir is better:

  • it provides a way better user experience that does not accidentally mess up the cwd
  • it matches user expectations from previous version
  • it avoid breaking scripts that rely on the previous expectations
  • is is a crucial dependency for moving forward with DRAFT: Add landlock support #723: by using an extraction directory, we can 1. create the new dir, 2. restrict write to cwd and parents and only whitelist the extraction dir, 3. extract the archive (dangerous)

Edit:
I realized that this behavior was provided by smart unpack which was removed. Going open an issue to discuss this specific issue for a future change

This PR now only fixes the issue with the missing --dir option

Edit2: added the solution discussed in #963

@valoq valoq force-pushed the decompression-fix branch from 596f892 to ebc24be Compare April 25, 2026 18:04
@valoq valoq mentioned this pull request Apr 25, 2026
@marcospb19 marcospb19 linked an issue Apr 27, 2026 that may be closed by this pull request
@valoq
Copy link
Copy Markdown
Collaborator Author

valoq commented Apr 27, 2026

This PR now implements the decompression as discussed in #963

@valoq
Copy link
Copy Markdown
Collaborator Author

valoq commented Apr 27, 2026

I will add some more tests to ensure this does not overlook any edge cases

@valoq valoq marked this pull request as draft April 27, 2026 15:21
@marcospb19
Copy link
Copy Markdown
Member

Alright 👍, when you're done maybe I'll add a test too.

@marcospb19 marcospb19 linked an issue Apr 27, 2026 that may be closed by this pull request
@valoq valoq marked this pull request as ready for review April 27, 2026 22:09
@valoq valoq marked this pull request as draft April 27, 2026 22:11
@valoq valoq force-pushed the decompression-fix branch from cbf6300 to 3363c77 Compare April 27, 2026 22:13
@valoq valoq marked this pull request as ready for review April 27, 2026 22:13
@valoq
Copy link
Copy Markdown
Collaborator Author

valoq commented Apr 27, 2026

Ok, the tests are now all successful. This should be ready to be merged

@marcospb19
Copy link
Copy Markdown
Member

marcospb19 commented Apr 28, 2026

You also fixed ouch decompress file.tar --dir .

majestic...

I'll add a test for that to ensure it's not broken in the future.

@marcospb19
Copy link
Copy Markdown
Member

@valoq I pushed 2 new tests I'd like you to take a look and review in the last commit.

We can't cover all the scenarios nicely, now when --dir points to CWD, it'll also overwrite without asking, and it is a little inconsistent when compared to --dir pointing to other folders, that's not a big problem, just something I thought I'd add a test to ensure the behavior isn't changed unexpectedly.

To entirely solve this, we'd need to check for conflicts on every single file extracted, that's possible and I was working on that on another branch but it sounds simpler than it really is, there is a lot of emergent complexity for merging files on conflicts and correctly decompressing files even when a parent folder was renamed, by taking over the responsibility of conflict handling we also have to add more code to check for vulnerabilities that crates check for us.


Well anyways, please review the extra code and tell me if you see any problem, I think it's great for the scope of this PR, and the rest can be addressed properly later.

Signed: a929d4c4e

valoq and others added 13 commits April 28, 2026 20:23
to fullfil this promise:

```
// Each step leaves the filesystem in a consistent state, and a failure midway
// leaves the user with valid extracted content (just nested one level).
```
This reverts commit ec497cf.

That crate is unix-only but we want these tests to pass on Windows too

oof
the tests evidence that there is some inconsistency today between how
'--dir' works when pointing to CWD or not

when it points to CWD, it overwrites files inside of it without asking,
that is a side-effect of this PR fixing it asking to remove CWD entirely
@marcospb19
Copy link
Copy Markdown
Member

Just did a rebase to solve conflicts.

@valoq
Copy link
Copy Markdown
Collaborator Author

valoq commented Apr 29, 2026

Well anyways, please review the extra code and tell me if you see any problem, I think it's great for the scope of this PR, and the rest can be addressed properly later.

LGTM

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

Successfully merging this pull request may close these issues.

optional smart unpack Ouch 0.7.1 breaks decompression

2 participants