Skip to content

Conversation

@garydgregory
Copy link
Member

No description provided.

garydgregory and others added 30 commits October 2, 2025 10:30
* Improve `FileUtils.forceDelete()` tests on Windows

On Windows, the `DeleteFile` Win32 API has a little quirk: it refuses to delete files with the legacy **DOS read-only attribute** set. (Because apparently 99% of Windows users don’t realize that “deleting a file” is actually an operation on the *directory*, not the file itself 😉).

So the usual drill is: clear the read-only flag first, then delete.

* Until JDK 25, `File.delete()` did this for you behind the scenes: it quietly stripped the flag before calling into `DeleteFile`. That meant your file might be left behind (with the flag missing) if the *real* ACLs didn’t allow deletion.
* From JDK 25 onward, `File.delete()` doesn’t touch the flag anymore. If the bit is set, `DeleteFile` fails, end of story.
* `FileUtils.forceDelete()` already knows how to juggle the flag itself, so its behavior didn’t change.

This PR:

* Updates two tests that were (unfairly) comparing `File.delete` with `FileUtils.forceDelete`. With JDK 25, their expectations diverged.
* Adds a new test to confirm that `FileUtils.forceDelete` restores the read-only flag if the actual deletion fails.

> [!WARNING]
> I didn’t develop this on a Windows box, so I couldn’t test it locally. Leaving this as a **draft PR** until CI tells us whether Windows agrees with me.

* fix: always clear read-only bit

* fix: check for `IOException`, not `AccessDeniedException`

* feat: add holder for file and parent attributes

Introduce a helper that snapshots file and parent directory attributes before `setReadOnly` is applied.
If a deletion attempt fails, the holder can restore the original attributes to keep the filesystem state consistent.

* fix: Spotbugs failure

* fix: prefer POSIX to DOS attributes when both are available

On Linux/Unix, both POSIX and DOS attribute views may be supported,
while on Windows only DOS attributes are available.
Check for POSIX first to ensure the correct view is used across platforms.

* fix: revert read-only related changes
Bumps [ossf/scorecard-action](https://github.com/ossf/scorecard-action) from 2.4.2 to 2.4.3.
- [Release notes](https://github.com/ossf/scorecard-action/releases)
- [Changelog](https://github.com/ossf/scorecard-action/blob/main/RELEASE.md)
- [Commits](ossf/scorecard-action@05b42c6...4eaacf0)

---
updated-dependencies:
- dependency-name: ossf/scorecard-action
  dependency-version: 2.4.3
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.30.4 to 3.30.6.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@303c0ae...64d10c1)

---
updated-dependencies:
- dependency-name: github/codeql-action
  dependency-version: 3.30.6
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/dependency-review-action](https://github.com/actions/dependency-review-action) from 4.7.3 to 4.8.0.
- [Release notes](https://github.com/actions/dependency-review-action/releases)
- [Commits](actions/dependency-review-action@595b5ae...56339e5)

---
updated-dependencies:
- dependency-name: actions/dependency-review-action
  dependency-version: 4.8.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Add missing `read` argument validation in `IOUtils`

In #790 I introduced `IOUtils#checkIndexFromLength` calls to validate arguments across the codebase. Ironically, the `IOUtils` class itself was left out.

This PR addresses that omission by adding argument validation to `IOUtils#read` and `IOUtils#readFully`.

Key points:

* Ensures consistency with the rest of Commons IO by validating `offset` and `length`.
* Fixes inconsistent exception behavior:

  * Previously, `length < 0` resulted in an `IllegalArgumentException`.
  * `offset < 0` did not trigger validation and failed later with an `IndexOutOfBoundsException`.
* With this change, both invalid cases are handled consistently and upfront.

* fix: failing tests
The implementation of `IOUtils.toByteArray(InputStream, int, int)` added in #776 throws different exceptions depending on the requested size:

* For request sizes larger than the internal chunk size, it correctly throws an `EOFException`.
* For smaller requests, it incorrectly throws a generic `IOException`.

This PR makes the behavior consistent by always throwing an `EOFException` when the stream ends prematurely.

Note: This also affects `RandomAccessFiles.read`. Its previous truncation behavior was undocumented and inconsistent with `RandomAccessFile.read` (which reads as much as possible). The new behavior is not explicitly documented here either, since it is unclear whether throwing on truncation is actually desirable.
Better unit test messages
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.30.6 to 4.30.7.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@64d10c1...e296a93)

---
updated-dependencies:
- dependency-name: github/codeql-action
  dependency-version: 4.30.7
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps `commons.bytebuddy.version` from 1.17.7 to 1.17.8.

Updates `net.bytebuddy:byte-buddy` from 1.17.7 to 1.17.8
- [Release notes](https://github.com/raphw/byte-buddy/releases)
- [Changelog](https://github.com/raphw/byte-buddy/blob/master/release-notes.md)
- [Commits](raphw/byte-buddy@byte-buddy-1.17.7...byte-buddy-1.17.8)

Updates `net.bytebuddy:byte-buddy-agent` from 1.17.7 to 1.17.8
- [Release notes](https://github.com/raphw/byte-buddy/releases)
- [Changelog](https://github.com/raphw/byte-buddy/blob/master/release-notes.md)
- [Commits](raphw/byte-buddy@byte-buddy-1.17.7...byte-buddy-1.17.8)

---
updated-dependencies:
- dependency-name: net.bytebuddy:byte-buddy
  dependency-version: 1.17.8
  dependency-type: direct:development
  update-type: version-update:semver-patch
- dependency-name: net.bytebuddy:byte-buddy-agent
  dependency-version: 1.17.8
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
category/java/errorprone.xml/UselessOperationOnImmutable as it is
scheduled for removal from PMD.

PMD 8.0.0 will remove support for this Rule.
This PR is split from #799.

The `CloseShieldChannel` implementation only inspects interfaces **directly** implemented by the given channel’s class, ignoring those inherited from its superclasses.
As a result, proxies for types such as `FileChannel` does not expose any of the interfaces declared on `FileChannel` itself.
* Fixes issues in `CloseShieldChannel`

Two bugs in the `CloseShieldChannel` helper make it unreliable in practice:

1. **Type-erasure bug in `T wrap(T)`**
   The method signature only works correctly when `T` is an **interface** extending `Channel`.
   Since Java’s type system doesn’t allow constraining `T` to “interface types only,” this could lead to unexpected runtime `ClassCastException`s even though the code compiles successfully.

2. **Incomplete interface discovery**
   The implementation only inspected interfaces **directly** implemented by the given channel’s class, ignoring those inherited from its superclasses.
   As a result, proxies for types such as `FileChannel` did not expose any of the interfaces declared on `FileChannel` itself.

#### Fixes

This PR addresses both issues:

* **Reworks the API signature**

  * Replaces `T wrap(T)` with its erasure: `Channel wrap(Channel)`.
  * Introduces a new overload: `T wrap(T, Class<T>)`, which allows callers to explicitly specify the interface type they expect.
    This version fails fast with a clear `IllegalArgumentException` if the provided type is not an interface, instead of allowing a `ClassCastException` later.

* **Improves interface collection logic**

  * Updates the implementation to include interfaces declared on superclasses, ensuring all relevant `Channel` interfaces are correctly proxied.

* Fixes interface discovery in `CloseShieldChannel`

This PR is split from #799.

The `CloseShieldChannel` implementation only inspects interfaces **directly** implemented by the given channel’s class, ignoring those inherited from its superclasses.
As a result, proxies for types such as `FileChannel` does not expose any of the interfaces declared on `FileChannel` itself.

* fix: add overloads for commons channel types

* fix: add `ByteChannel` overload to resolve ambiguity

* fix: Limit interfaces to those verified.

* fix: rollback previous test

* fix: Restore generic method
Bumps [actions/dependency-review-action](https://github.com/actions/dependency-review-action) from 4.8.0 to 4.8.1.
- [Release notes](https://github.com/actions/dependency-review-action/releases)
- [Commits](actions/dependency-review-action@56339e5...40c09b7)

---
updated-dependencies:
- dependency-name: actions/dependency-review-action
  dependency-version: 4.8.1
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 4.30.7 to 4.30.8.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@e296a93...f443b60)

---
updated-dependencies:
- dependency-name: github/codeql-action
  dependency-version: 4.30.8
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Fix concurrency issue in `IOUtils.skip`

This patch addresses a concurrency problem in `IOUtils.skip`, as reported in [COMPRESS-666](https://issues.apache.org/jira/browse/COMPRESS-666) and [COMPRESS-697](https://issues.apache.org/jira/browse/COMPRESS-697).

Previously, `IOUtils.skip` relied on `InputStream#read` to skip bytes, using a buffer shared across **all** threads. Although `IOUtils.skip` itself does not consume the data read, certain `InputStream` implementations (e.g. `ChecksumInputStream`) may process that data internally.

In concurrent scenarios, this shared buffer could be overwritten by another thread between the `read` and the subsequent internal processing (such as checksum calculation), leading to incorrect behavior.

This change reverts commit c12eaff and restores the use of a **per-thread buffer** in `IOUtils.skip`, ensuring thread safety and correct behavior in concurrent environments.

* Adds a reentrancy guard to the thread-local pool

* Apply suggestion from @Copilot (1)

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Apply suggestions from @Copilot (2)

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@garydgregory garydgregory marked this pull request as draft December 14, 2025 15:29
garydgregory and others added 21 commits December 17, 2025 07:50
- ~2.6x speedup on StringReader input
- ~10% speed on file resource as InputStreamReader

Benchmark
Mode  Cnt          Score         Error  Units
IOUtilsContentEqualsReadersBenchmark_2_22_0.testFileCurrent
avgt    5     105274.452 ±    1466.048  ns/op
IOUtilsContentEqualsReadersBenchmark_2_22_0.testFileRelease2_22_0
avgt    5     107500.847 ±    1752.422  ns/op
IOUtilsContentEqualsReadersBenchmark_2_22_0.testFile_2_21_0
avgt    5     115720.416 ±    1209.652  ns/op
IOUtilsContentEqualsReadersBenchmark_2_22_0.testStringCurrent
avgt    5  113330719.330 ± 1187191.151  ns/op
IOUtilsContentEqualsReadersBenchmark_2_22_0.testStringRelease2_22_0
avgt    5  110389392.582 ±  785367.455  ns/op
IOUtilsContentEqualsReadersBenchmark_2_22_0.testString_2_21_0
avgt    5  284939866.619 ± 9969793.485  ns/op

Apache Maven 3.9.12 (848fbb4bf2d427b72bdb2471c22fced7ebd9a7a1)
Maven home: /opt/homebrew/Cellar/maven/3.9.12/libexec
Java version: 21.0.9, vendor: Homebrew, runtime:
/opt/homebrew/Cellar/openjdk@21/21.0.9/libexec/openjdk.jdk/Contents/Home
Default locale: en_US, platform encoding: UTF-8
OS name: "mac os x", version: "26.2", arch: "aarch64", family: "mac"
- Add missing comment.
- Close HTML tags.
- Typos
Bumps `commons.bytebuddy.version` from 1.18.2 to 1.18.3.

Updates `net.bytebuddy:byte-buddy` from 1.18.2 to 1.18.3
- [Release notes](https://github.com/raphw/byte-buddy/releases)
- [Changelog](https://github.com/raphw/byte-buddy/blob/master/release-notes.md)
- [Commits](raphw/byte-buddy@byte-buddy-1.18.2...byte-buddy-1.18.3)

Updates `net.bytebuddy:byte-buddy-agent` from 1.18.2 to 1.18.3
- [Release notes](https://github.com/raphw/byte-buddy/releases)
- [Changelog](https://github.com/raphw/byte-buddy/blob/master/release-notes.md)
- [Commits](raphw/byte-buddy@byte-buddy-1.18.2...byte-buddy-1.18.3)

---
updated-dependencies:
- dependency-name: net.bytebuddy:byte-buddy
  dependency-version: 1.18.3
  dependency-type: direct:development
  update-type: version-update:semver-patch
- dependency-name: net.bytebuddy:byte-buddy-agent
  dependency-version: 1.18.3
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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.

4 participants