Update RAR5 code to report encryption #2096
Conversation
| /* fallthrough */ | ||
| case EX_SUBDATA: | ||
| /* fallthrough */ | ||
| default: | ||
| /* Skip unsupported entry. */ | ||
| return consume(a, extra_data_size); | ||
| extra_data_size -= extra_field_size; | ||
| if (ARCHIVE_OK != consume(a, extra_field_size)) { |
There was a problem hiding this comment.
I didn't actually encounter any errors specific to this code-path, however it made me felt uneasy - e.g. skipping an earlier unsupported field may cause us to skip identifying an encryption field.
| if (rar->has_encrypted_entries == ARCHIVE_READ_FORMAT_ENCRYPTION_DONT_KNOW) { | ||
| rar->has_encrypted_entries = 0; | ||
| } |
There was a problem hiding this comment.
This is something I didn't necessarily like doing, but I copied from other places. Encryption might not necessarily be detected when reading the first header (e.g. if the Nth entry is the first encrypted one), however there's not much difference in practice between perpetually returning ARCHIVE_READ_FORMAT_ENCRYPTION_DONT_KNOW and returning 0.
| DEFINE_TEST(test_read_format_rar4_solid_encrypted) | ||
| { | ||
| /* TODO: If solid RAR4 support is ever added, the following should pass */ | ||
| #if 0 | ||
| test_encrypted_rar_archive("test_read_format_rar4_solid_encrypted.rar", 0, 1); | ||
| #else | ||
| skipping("RAR4 solid archive support not currently available"); | ||
| #endif | ||
| } |
There was a problem hiding this comment.
I figure that at least having this here will make someone's life easier if RAR4 solid support is ever added. I can remove if folks don't want currently dead code laying around.
| #ifdef HAVE_ZLIB_H | ||
| int zip_version = 20; | ||
| #else | ||
| int zip_version = 10; | ||
| #endif |
There was a problem hiding this comment.
Something I hit running the tests locally. I should probably look into how to ensure that I build with zlib, but this can otherwise alleviate pain for anyone else who runs into this.
With data in comp_state, this info is no longer needed on the file
|
Fixes #1661 Edit - oops, I need this in the description |
Currently, the RAR5 code always reports
ARCHIVE_READ_FORMAT_ENCRYPTION_UNSUPPORTEDforarchive_read_has_encrypted_entries, nor does it set any of the entry-specific properties, even though it has enough information to properly report this information. Accurate reporting of encryption is super useful for applications because reporting an error message such as "the archive is encrypted, but we don't currently support encryption" is a lot better than a not generally usefulerrnovalue and a non-localizable error string with a confusing and unpredictable error message.Fixes #1661