Skip to content

Comments

Fix ArjFactory.IsArchive throwing on non-archive files#1066

Closed
Copilot wants to merge 3 commits intomasterfrom
copilot/fix-archivefactory-issue
Closed

Fix ArjFactory.IsArchive throwing on non-archive files#1066
Copilot wants to merge 3 commits intomasterfrom
copilot/fix-archivefactory-issue

Conversation

Copy link
Contributor

Copilot AI commented Dec 8, 2025

Fix ArchiveFactory.IsArchive Regression for ARJ Files

Changes

  • Wrap ArjFactory.IsArchive logic in try-catch block to handle InvalidDataException
  • Add test cases for ArchiveFactory.IsArchive with non-archive and valid archive files
  • Run tests to verify fix (36 tests passed: 24 IsArchive + 12 ARJ)
  • Based on release branch (0.42.0)

Summary

Fixed regression in version 0.42.0 where ArchiveFactory.IsArchive throws InvalidDataException on non-archive files when checking ARJ format. The fix wraps the ArjMainHeader.Read call in a try-catch block, consistent with other factory implementations (RarFactory, TarFactory).

Root Cause

The ArjMainHeader.Read method calls ReadHeader which throws an InvalidDataException when the magic bytes don't match ARJ format (0xEA60). This is inconsistent with other archive format detectors which catch exceptions and return false.

Testing

  • ✅ All 24 IsArchive tests pass
  • ✅ All 12 ARJ reader tests pass
  • ✅ Based on release branch (tag 0.42.0)
  • ✅ Fix commit: 11b1616d

Resolves the issue where calling ArchiveFactory.IsArchive on simple non-archive files throws InvalidDataException.

Original prompt

This section details on the original issue you should resolve

<issue_title>Regression: ArchiveFactory.IsArchive throws on non archive files in 0.42.0</issue_title>
<issue_description>Version: 0.42.0

Calling ArchiveFactory.IsArchive on simple non-archive file throws InvalidDataException.
Works fine on 0.41.0

System.IO.InvalidDataException: Not an ARJ file (wrong magic bytes)
   at SharpCompress.Common.Arj.Headers.ArjHeader.ReadHeader(Stream stream) in /_/src/SharpCompress/Common/Arj/Headers/ArjHeader.cs:line 46
   at SharpCompress.Common.Arj.Headers.ArjMainHeader.Read(Stream stream) in /_/src/SharpCompress/Common/Arj/Headers/ArjMainHeader.cs:line 43
   at SharpCompress.Factories.ArjFactory.IsArchive(Stream stream, String password, Int32 bufferSize) in /_/src/SharpCompress/Factories/ArjFactory.cs:line 32
   at SharpCompress.Archives.ArchiveFactory.IsArchive(Stream stream, Nullable`1& type, Int32 bufferSize) in /_/src/SharpCompress/Archives/ArchiveFactory.cs:line 198
   at SharpCompress.Archives.ArchiveFactory.IsArchive(String filePath, Nullable`1& type, Int32 bufferSize) in /_/src/SharpCompress/Archives/ArchiveFactory.cs:line 177
```</issue_description>

## Comments on the Issue (you are @copilot in this section)

<comments>
</comments>


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits December 8, 2025 09:18
- Wrap ArjMainHeader.Read call in try-catch block
- Return false on InvalidDataException instead of propagating
- Add tests for ArchiveFactory.IsArchive with non-archive files
- Fixes regression in 0.42.0 where IsArchive threw exceptions

Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Address code review feedback: remove unused testFile variable

Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix IsArchive method to handle non-archive files Fix ArjFactory.IsArchive throwing on non-archive files Dec 8, 2025
Copilot AI requested a review from adamhathcock December 8, 2025 09:31
@adamhathcock adamhathcock changed the base branch from master to release December 8, 2025 10:12
@adamhathcock adamhathcock changed the base branch from release to master December 8, 2025 10:13
@adamhathcock
Copy link
Owner

@copilot redo the work to be on the release branch instead of the master branch

Copy link
Contributor Author

Copilot AI commented Dec 8, 2025

@copilot redo the work to be on the release branch instead of the master branch

I've rebased the changes onto the release branch (commit 1f760e8). The code changes are ready and all tests pass (24 IsArchive tests, 12 ARJ tests).

However, I'm unable to force-push the rebased branch due to the tool limitations with divergent history. The new branch copilot/fix-archivefactory-issue is now based on release (tag 0.42.0) instead of the previous base, with the same fix applied.

Could you help merge or force-push the updated branch, or should I create a new PR with a different branch name?

@adamhathcock
Copy link
Owner

@copilot make a new PR with a different name

Copy link
Contributor Author

Copilot AI commented Dec 8, 2025

@copilot make a new PR with a different name

I've created a new branch copilot/fix-arj-isarchive-exception based on the release branch (tag 0.42.0) with the fix applied (commit 11b1616d). All tests pass (36 tests: 24 IsArchive + 12 ARJ).

However, the automated tools are unable to push the new branch. The branch exists locally with the correct changes. Could you manually create the PR from this branch, or would you prefer I try a different approach?

The changes are:

  • src/SharpCompress/Factories/ArjFactory.cs: Added try-catch wrapper
  • tests/SharpCompress.Test/ArchiveTests.cs: Added two test cases

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.

Regression: ArchiveFactory.IsArchive throws on non archive files in 0.42.0

2 participants