On Wed, 3 Jun 2026 14:34:38 GMT, Ashay Rane <[email protected]> wrote:

> Prior to this change, the DirPermissionDenied.java test used chmod to
> change directory permissions, which can fail on Windows either because
> chmod is not a native Windows program/command or because MSys2's
> implementation of chmod does not work well with Access Control Entries
> in Windows.  Consequently, the DirPermissionDenied.java test fails on
> Windows, except in specific restrictive cases such as when the test is
> run using the most recent version of Cygwin.
> 
> This patch updates the test so that instead of shelling out to chmod,
> the test now uses Java APIs to change the directory permission.  Key to
> this change is that depending on whether the filesystem supports POSIX
> or Access Control Lists, the test decides whether to use the Unix-style
> file permissions or ACL entries.  In the unlikely case that the test is
> unable to change the directory access as is required by the test, the
> test bails out with a `SkippedException`.  This is required on older
> versions of Windows, where Administrator accounts implictly have
> unconditional access to all files, even when there is an attempt to deny
> them access.
> 
> I've validated that this test now passes on macOS, Windows 11, and
> Windows Server 2022 Datacenter.
> 
> ---------
> - [x] I confirm that I make this contribution in accordance with the [OpenJDK 
> Interim AI Policy](https://openjdk.org/legal/ai).

Thanks for having a look at this.
Looks generally reasonable but we will have to test it in our CI. Does the test 
now passes on the systems where it used to fail?

test/jdk/sun/net/www/protocol/file/DirPermissionDenied.java line 29:

> 27:  * @summary NPE from FileURLConnection.connect
> 28:  * @library /test/lib
> 29:  * @build DirPermissionDenied jdk.test.lib.util.FileUtils 
> jtreg.SkippedException

If this is using junit then jtreg.SkippedException is not needed and should not 
be used. JUnuit `Assumptions` API should be used instead.

test/jdk/sun/net/www/protocol/file/DirPermissionDenied.java line 52:

> 50: 
> 51: import jdk.test.lib.util.FileUtils;
> 52: import jtreg.SkippedException;

Please revert that line (see previous comment)

test/jdk/sun/net/www/protocol/file/DirPermissionDenied.java line 92:

> 90:             makeDirectoryUnreadable();
> 91:             if (TEST_DIR.toFile().list() != null) {
> 92:                 throw new SkippedException("Could not make directory 
> inaccessible");

Use JUnit `Assumptions` here instead.

-------------

PR Review: https://git.openjdk.org/jdk/pull/31372#pullrequestreview-4449742553
PR Review Comment: https://git.openjdk.org/jdk/pull/31372#discussion_r3373424900
PR Review Comment: https://git.openjdk.org/jdk/pull/31372#discussion_r3373429444
PR Review Comment: https://git.openjdk.org/jdk/pull/31372#discussion_r3373434501

Reply via email to