-
Notifications
You must be signed in to change notification settings - Fork 709
Experiments #819
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
garydgregory
wants to merge
136
commits into
fix/read-only
Choose a base branch
from
master
base: fix/read-only
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Experiments #819
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* 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
Reduce vertical space
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.
Sort members
* 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>
- 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>
This reverts commit 4cf10f9.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.