GitLab

Currently, redundant path name handling happens when archive files have
a common prefix and an equal destination is returned from the `decide-destination`
signal. In this case, the old prefix is removed and the new one (the equal one)
is added again. Let's detect this case and prevent the redundancy.

Currently, an empty folder is created when extracting an archive over
Nautilus which contains one file with the same name as the archive.
This is because gnome-autoar always creates the top-level folder, but
the one file archive with the same name should be extracted directly
without creating additional folders. Otherwise the `conflict` signal
is emitted and Nautilus by default skips conflicting files, so only
empty folder remains after extraction. This problem have not been
unfortunately noticed by the `test-one-file-same-name` unit test due to
differences in the `decide-destination` signal handling. Let's do not
create the top-level folder at all as `g_file_make_directory_with_parents`
is used later anyway.

Fixes: #28

The Coverity tool found usage of uninitialized value, which was unfortunatelly
introduced by the recent commit c919227c. This might lead to segfaults when
compressing remote files. Let's initialize the `sparse` variable to fix this.

The Coverity tool found some defects in the testing suite. Although,
those defects don't cause any really issues, let's fix them to make
covscan happy and the code more bullet-proof.

Let's use GitLab CI to automatically build source codes and run unit
tests for each commit to be sure that new changes don't break anything.

For some reason `buildah push --creds` is needed to make it work, though
it is not clear to me why.

Fixes: #20

The .doap file currently states that gnome-autoar belongs to "apps" category.
However, gnome-autoar is a library, so "core" makes more sense here. Also
download-page and bug-database elements are missing. Let's fix the mentioned
issues.

Currently, there is no README file, so GitLab doesn't show any project
info. Let's add one with the same info which is in doap and docs already.
But also update the texts to not mention "gschemas" as they are not used
already.

Fixes: #13

The new cpio format writes the file content for hardlinks the last
time an inode is seen. To achieve this, the archive entries are
internally stored over `archive_entry_linkify` and written later.
In that case, `archive_entry_linkify` takes ownership of the
`archive_entry` struct and set the pointer to `NULL`. However,
gnome-autoar keeps using the original entry, which leads to weird
errors. Also, after all archive entries are written,
`archive_entry_linkify` has to be called in a loop to write all
deffered entries. However, it is called only once currently, which
can cause that some of the hardlinks are not written to the
archive. Let's correctly handle the entry ownership and call
`archive_entry_linkify` to fix hardlink handling for the new cpio
format.

Relates: #25

Currently, various filesystem attributes are passed to libarchive
without checks. But it may happen that not all those attributes are
available, especially for files provided over GVfs. Let's set only
those which are available for sure.

Relates: #25

Currently, all files are considered as hardlinks when compressing files provided
by GVfs (e.g. SFTP) to TAR format, so the file content is lost for those files.
This is because the `inode`, `device`, and/or `nlink` attributes are not set for
those files. Let's do not call `archive_entry_linkify` in this case to not create
broken archives.

Fixes: #25

The extraction of children from a readonly directory should not fail.
Let's add a test to verify this.

If archive contains read-only folders, the extraction of their children
fails with the "Permissions denied" error. This is because the folder
permissions are restored immediately before writing their children.
Let's do not restore unix mode before writing the children to fix
this issue.

Fixes: #10

The CVE number for #12 has been assigned after the
release, so it is not part of the NEWS file. Let's add the CVE number
additionaly at least.

(Malicious) archives can have entries with symlink in parents. Archives
entries can have absolute paths, or relative paths that points outside
of the destination. Let's add test to ensure that extraction fails with
error for symlinks in parents and tests to verify that malformed paths
are correctly sanitized and not written outside.

(Malicious) archives can have malformed paths with `..` segments so they
point outside of the destination. The `autoar_extractor_do_sanitize_pathname`
function already sanitizes those paths to be inside of the destination,
however, the code from `autoar_extractor_step_decide_destination` operates on
paths, which are not yet sanitized and fails with the following criticals:
`g_file_resolve_relative_path: assertion 'relative_path != NULL' failed`.
Let's use the `autoar_extractor_do_sanitize_pathname` also here to fix this
criticals.

Currently, it is still possible that some files are extracted outside of
the destination dir in case of malicious archives. The checks from commit
adb067e6 can be still bypassed in certain cases. See file-roller#108
for more details. After some investigation, I am convinced that it would be
best to	simply disallow symlinks in parents. For example, `tar` fails to
extract such files with the `ENOTDIR` error. Let's do the same here.

Fixes: #12

This reverts commit adb067e6.

This reverts commit cc4e8b7c.

In case of conflict, when skipping some file, the `total_size` and `total_files`
is not updated, but neither `completed_files` and `completed_size`. Let's
reduce the `total_size` and `total_files`. Same approach is used in Nautilus
when skipping.

The symlink, or hardline should be rewriten itself, not its target.
Let's add tests to verify this.

Currently, symlinks are followed when detecting conflicts. But this
is not desired as the original file caused the conflict, not its target.

To be honest, it is not really clear to me what is purpose of this test.
As per the name, it should verify that error is returned when overwriting
file over directory. However, I think that it is totally fine to overwrite
empty directory. Anyway, the overwrite action is not explicitely chosen,
so the skip action is used instead. Consequently, the test verifies that
`error` is not set. So it looks to me that the test is tottaly wrong. Let's
modify and rename the test, so it really checks that error is returned when
somebody tries to overwrite non-empty directory.

Current logic doesn't detect conflics when extracting directory. This
is ok, but only for the case when the conflic is caused by directory.
Otherwise, the conflic should be detected and AutoarExtractor should
try to delete the file before creating new directory.

Currently, `g_file_replace` is used to write files. However, it uses
`G_FILE_CREATE_NONE` which keeps old permissions. It should rather use
`G_FILE_CREATE_REPLACE_DESTINATION` instead to not keep any old permissions
as it is among others used by File Roller. However, there is bug in
`G_FILE_CREATE_REPLACE_DESTINATION` implementaion, see glib#2325.
Let's explicitely delete that file and use `g_file_create` instead.
This will also fix problems when overwriting file by directory and
ensures that hardlinks will be replaced and not just modified.

From the code, it was not really clear what is the default action
for conflicts. Let's add test which verifies that conflicting files
are skipped by default.

The tests for conflicts contains several bugs and also are not able to
distingues between skip and overwrite actions. Let's modify the test so
they can really verify whether the files are skipped, or overwritten.

The test for conflict contains `test-one-file-` prefix which doesn't make
much sense here and just makes the name too long. Let's use just `test-`
prefix instead.

The `AUTOAR_CONFLICT_OVERWRITE` is set as default value for the action
variable when conflict occured. However, `g_signal_emit` clears that
variable to `0` if the signal is unhandled. But `0` is currently mapped
to `AUTOAR_CONFLICT_SKIP`. So the code is a little bit confusing. I think
that overwrite is the right thing in most cases and also this is the
default behavior of `tar` as an archive may contain several versions of
some file and the last one is the newest. However, gnome-autoar allows
extraction in the non-empty folders and has conflict API, so it would be
really safer to use the skip action by default. Let's add the
`AUTOAR_CONFLICT_UNHANDLED` action for better control and use the
`AUTOAR_CONFLICT_SKIP` action by default.

A test for the `output-is-dest` property is missing currently. Let's add
one to be sure that extra directory is not created for an archive with
a file with a different name.

If the `output-is-dest` property is `TRUE`, the `prefix` is not cleared and is
passed to `decide-destination` signal. This looks unexpected because it allows
to change even the prefix which doesn't match archive name, which is not allowed
even if `output-is-dest` is `FALSE`. I am conviced that it should not be allowed
to change the `prefix` at all in this case. Let's clear the `prefix` variable to
avoid that.

At the beginning, the file list is printed in the debug output. However,
it doesn't contain targets of symlinks and hardlinks. Let's print them
as well. Also print symlink target when writing it on the disk similary
to hardlinks.

`g_file_make_directory_with_parents` is called to create `self->destination_dir`
directory before extraction. However, the files may be written to completely
different dir later if the they have common prefix and the prefix is consequently
changed over `decide-destination` signal. Let's use `self->prefix_new` if it is
set to prevent creation of unrelated directories.

The returned value from `g_file_get_path` is not consequently freed.
Let's use `g_file_peek_path` instead to fix the leak.

The documentation refers to non-existing functions and properties.
It also contains misleading info about behavior of some functions,
or their parameters. Let's try to make the documentation clearer.
This also fixes some typos.

Ondrej Holy authored Feb 22, 2021 and Felipe Borges committed Mar 09, 2021

Recursive delete has been added by commit 58ac8fc5 to remove already created
directories when extraction fails because of an invalid password. In fact,
it deletes the whole `destination_dir` also in case of other failures, which
is maybe not the best approach, but ok. However, a problem is that
gnome-autoar allows extraction in non-empty destination, so this might remove
also files which were not initially created by gnome-autoar. Fortunately,
nautilus and gnome-shell currently always extracts in an extra directory. But
what is worse is the fact, that if the files in the archive have a common
prefix, then the `destination_dir` is actually a parent of that extra directory
in the case of nautilus and gnome-shell (but API allows to set completely
unrelated path)! So this can easily cause huge data loss! It would probably be
better to create parent directories only when `archive_read_data_block`
succeeds instead of deleting them later. Alternatively, gnome-autoar could
track which files were written and deletes just those on that list. But for now,
let's just remove the code for recursive delete and do not care about leftover
files...

Matthias authored Mar 05, 2021 and Ondrej Holy committed Mar 05, 2021

The commit 66cf03f9 dropped support for RAR archives because they were not well
supported by libarchive. But a lot of changed since then and libarchive 3.4.0
came with RAR 5 support. Let's add back support for RAR and bump the libarchive
dependency accordingly.

Fixes: #2

Currently, it is not possible to extract archives that don't explicitly
contain parent folders. This is unintentional regression caused by commit
adb067e6. Let's simply ignore G_IO_ERROR_NOT_FOUND errors when looking
for symlinks to fix this.

Fixes: https://gitlab.gnome.org/GNOME/gnome-autoar/-/issues/11