starred/sharpcompress
1
0
Fork 0
mirror of https://github.com/adamhathcock/sharpcompress.git synced 2026-02-04 13:34:59 +00:00
Code Issues 975 Packages Projects Releases 20 Wiki Activity

Provide single interface for dealing with Entries that are SOLID and not. #746

New Issue
Closed
opened 2026-01-29 22:16:49 +00:00 by claunia · 8 comments
claunia commented 2026-01-29 22:16:49 +00:00
Owner
Copy Link

Originally created by @adamhathcock on GitHub (Dec 9, 2025).

Originally assigned to: @adamhathcock, @Copilot on GitHub.

Issue: https://github.com/adamhathcock/sharpcompress/issues/960#issuecomment-3616319717

Sometimes you don't know if the file is a RAR or 7Zip that may have SOLID entries. The current interface requires you to know in advance about SOLID or not.

Provide an interface that doesn't require to know in advance.

ExtractAllEntries doesn't feel right to be used alongside Entry enumeration.

This is what used to work:

      public void ExtractToDirectory( string archivePath, string extractPath )
      {
          // Lock for Extraction Progress because of Singleton
          lock( extractToDictionarySyncObject )
          {
              var options = new ExtractionOptions() {
                  ExtractFullPath = true,
                  Overwrite = true
              };

              Directory.CreateDirectory( extractPath );

              using IArchive archive = ArchiveFactory.Open( archivePath );
              double totalSize = archive.Entries.Where( e => !e.IsDirectory ).Sum( e => e.Size );
              long completed = 0;

              IReader reader = archive.ExtractAllEntries();
              while( reader.MoveToNextEntry() )
              {
                  if( !reader.Entry.IsDirectory )
                  {
                      reader.WriteEntryToDirectory( extractPath, options );

                      completed += reader.Entry.Size;
                      ExtractionProgress?.Invoke( this, new ExtractionProgressEventArgs( completed / totalSize ) );
                  }
              }
          }
      }
Originally created by @adamhathcock on GitHub (Dec 9, 2025). Originally assigned to: @adamhathcock, @Copilot on GitHub. Issue: https://github.com/adamhathcock/sharpcompress/issues/960#issuecomment-3616319717 Sometimes you don't know if the file is a RAR or 7Zip that may have SOLID entries. The current interface requires you to know in advance about SOLID or not. Provide an interface that doesn't require to know in advance. `ExtractAllEntries` doesn't feel right to be used alongside Entry enumeration. This is what used to work: ``` public void ExtractToDirectory( string archivePath, string extractPath ) { // Lock for Extraction Progress because of Singleton lock( extractToDictionarySyncObject ) { var options = new ExtractionOptions() { ExtractFullPath = true, Overwrite = true }; Directory.CreateDirectory( extractPath ); using IArchive archive = ArchiveFactory.Open( archivePath ); double totalSize = archive.Entries.Where( e => !e.IsDirectory ).Sum( e => e.Size ); long completed = 0; IReader reader = archive.ExtractAllEntries(); while( reader.MoveToNextEntry() ) { if( !reader.Entry.IsDirectory ) { reader.WriteEntryToDirectory( extractPath, options ); completed += reader.Entry.Size; ExtractionProgress?.Invoke( this, new ExtractionProgressEventArgs( completed / totalSize ) ); } } } } ```
claunia added the enhancement label 2026-01-29 22:16:49 +00:00
claunia commented 2026-01-29 22:16:50 +00:00
Author
Owner
Copy Link

@alex-konstantinov commented on GitHub (Dec 10, 2025):

From my point of view, it would be better to not break things and keep the behavior that was up until version 0.40.0 for the .ExtractAllEntries() as this is what it is - doing that call we just want to extract all entries no matter what type the archive is. And it worked. I don't see any reason to change that behavior.

I'd rather suggest to introduce new option/argument either for that method (as an overload) or as an option when we acquiring IArchive instance for example, where we can specify that we explicitly want to treat an archive as a solid and we expect an exception if it's not solid. Or something like that.

So all of the existing code out there after updating to latest versions of the library would keep working as expected.

Cheers,
Alex

@alex-konstantinov commented on GitHub (Dec 10, 2025): From my point of view, it would be better to not break things and keep the behavior that was up until version 0.40.0 for the .ExtractAllEntries() as this is what it is - doing that call we just want to extract all entries no matter what type the archive is. And it worked. I don't see any reason to change that behavior. I'd rather suggest to introduce new option/argument either for that method (as an overload) or as an option when we acquiring IArchive instance for example, where we can specify that we explicitly want to treat an archive as a solid and we expect an exception if it's not solid. Or something like that. So all of the existing code out there after updating to latest versions of the library would keep working as expected. Cheers, Alex
claunia commented 2026-01-29 22:16:50 +00:00
Author
Owner
Copy Link

@adamhathcock commented on GitHub (Dec 18, 2025):

This is the PR to revert probably https://github.com/adamhathcock/sharpcompress/pull/964

@adamhathcock commented on GitHub (Dec 18, 2025): This is the PR to revert probably https://github.com/adamhathcock/sharpcompress/pull/964
claunia commented 2026-01-29 22:16:51 +00:00
Author
Owner
Copy Link

@adamhathcock commented on GitHub (Dec 18, 2025):

Why is https://github.com/adamhathcock/sharpcompress/pull/1076 bad?

Maybe I should just allow it and https://github.com/adamhathcock/sharpcompress/issues/960 a different way

@adamhathcock commented on GitHub (Dec 18, 2025): Why is https://github.com/adamhathcock/sharpcompress/pull/1076 bad? Maybe I should just allow it and https://github.com/adamhathcock/sharpcompress/issues/960 a different way
claunia commented 2026-01-29 22:16:51 +00:00
Author
Owner
Copy Link

@alex-konstantinov commented on GitHub (Dec 19, 2025):

Why is #1076 bad?

Maybe I should just allow it and #960 a different way

From my point of view, it is not a best way as it requires user of an API to know that the archive might be solid, and it requires handling it differently, otherwise it will throw exception. It's just not convenient at all, to always add a condition check whenever someone is about just to extract the content from the archive (which often can be of any type, .rar, .zip, 7z, etc).

So I'd prefer to have the check

if (archive.IsSolid && archive.Type != ArchiveType.SevenZip)

incapsulated and handled inside the API call to extract all entries. So caller can just extract all from the archive and that's it. However if someone want to specifically handle some scenarios manually, they obviously should be able to do that, so the Type and IsSolid properties are absolutely OK to stay public, no worries on that matter.

Cheers,
Alex

@alex-konstantinov commented on GitHub (Dec 19, 2025): > Why is [#1076](https://github.com/adamhathcock/sharpcompress/pull/1076) bad? > > Maybe I should just allow it and [#960](https://github.com/adamhathcock/sharpcompress/issues/960) a different way From my point of view, it is not a best way as it requires user of an API to know that the archive might be solid, and it requires handling it differently, otherwise it will throw exception. It's just not convenient at all, to always add a condition check whenever someone is about just to extract the content from the archive (which often can be of any type, .rar, .zip, 7z, etc). So I'd prefer to have the check if (archive.IsSolid && archive.Type != ArchiveType.SevenZip) incapsulated and handled inside the API call to extract all entries. So caller can just extract all from the archive and that's it. However if someone want to specifically handle some scenarios manually, they obviously should be able to do that, so the Type and IsSolid properties are absolutely OK to stay public, no worries on that matter. Cheers, Alex
claunia commented 2026-01-29 22:16:51 +00:00
Author
Owner
Copy Link

@adamhathcock commented on GitHub (Dec 19, 2025):

using var archive = ArchiveFactory.Open(testArchive);
        archive.WriteToDirectory(SCRATCH_FILES_PATH, options);

Should work and use ExtractAllEntries internally if valid. When dealing with local files, that should be the easiest/best way

@adamhathcock commented on GitHub (Dec 19, 2025): ``` using var archive = ArchiveFactory.Open(testArchive); archive.WriteToDirectory(SCRATCH_FILES_PATH, options); ``` Should work and use ExtractAllEntries internally if valid. When dealing with local files, that should be the easiest/best way
claunia commented 2026-01-29 22:16:51 +00:00
Author
Owner
Copy Link

@alex-konstantinov commented on GitHub (Dec 22, 2025):

using var archive = ArchiveFactory.Open(testArchive);
        archive.WriteToDirectory(SCRATCH_FILES_PATH, options);

Should work and use ExtractAllEntries internally if valid. When dealing with local files, that should be the easiest/best way

Got it. Thanks!

However I'm not sure I understand correctly. If I look inside the WriteToDirectory method, I see:

public static void WriteToDirectory(this IArchive archive, string destinationDirectory, ExtractionOptions? options = null) { using var reader = archive.ExtractAllEntries(); reader.WriteAllToDirectory(destinationDirectory, options); }

Whereas ExtractAllEntries internally still checks for atchive to be solid or not solid and throws exception?

if (!IsSolid && Type != ArchiveType.SevenZip) { throw new InvalidOperationException( "ExtractAllEntries can only be used on solid archives or 7Zip archives (which require random access)." ); }

So we are again in a situation when we can not rely on this code and just feed it all types of archives uploaded by our users. Cause it will randomly crash instead of just extracting archives like it did in older versions?

@alex-konstantinov commented on GitHub (Dec 22, 2025): > ``` > using var archive = ArchiveFactory.Open(testArchive); > archive.WriteToDirectory(SCRATCH_FILES_PATH, options); > ``` > > Should work and use ExtractAllEntries internally if valid. When dealing with local files, that should be the easiest/best way Got it. Thanks! However I'm not sure I understand correctly. If I look inside the WriteToDirectory method, I see: ` public static void WriteToDirectory(this IArchive archive, string destinationDirectory, ExtractionOptions? options = null) { using var reader = archive.ExtractAllEntries(); reader.WriteAllToDirectory(destinationDirectory, options); } ` Whereas ExtractAllEntries internally still checks for atchive to be solid or not solid and throws exception? ` if (!IsSolid && Type != ArchiveType.SevenZip) { throw new InvalidOperationException( "ExtractAllEntries can only be used on solid archives or 7Zip archives (which require random access)." ); } ` So we are again in a situation when we can not rely on this code and just feed it all types of archives uploaded by our users. Cause it will randomly crash instead of just extracting archives like it did in older versions?
claunia commented 2026-01-29 22:16:51 +00:00
Author
Owner
Copy Link

@alex-konstantinov commented on GitHub (Dec 22, 2025):

Image

So the current latest version is still of no use for us :(

Or is it not yet in the latest published version on nuget?

@alex-konstantinov commented on GitHub (Dec 22, 2025): <img width="1136" height="262" alt="Image" src="https://github.com/user-attachments/assets/e7abbace-5af1-4950-bd03-e3b9fef595d2" /> So the current latest version is still of no use for us :( Or is it not yet in the latest published version on nuget?
claunia commented 2026-01-29 22:16:52 +00:00
Author
Owner
Copy Link

@adamhathcock commented on GitHub (Dec 23, 2025):

Hi @alex-konstantinov,

It does use ExtractAllEntries internally but there's a check which should be the same as the one for the exception.

It's not published to nuget yet.

@adamhathcock commented on GitHub (Dec 23, 2025): Hi @alex-konstantinov, It does use `ExtractAllEntries` internally but there's a check which should be the same as the one for the exception. It's not published to nuget yet.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
bug
duplicate
enhancement
feedback requested
not bug
pull-request
Mirrored from GitHub Pull Request
 question
up for grabs
wontfix
No Label enhancement
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
claunia
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/sharpcompress#746
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?