Skip to content

Issue #14068 - Cleanup IO.asFile on Windows #14124

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.

Sign up for GitHub

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

Open
joakime wants to merge 1 commit into
base: jetty-12.1.x
Choose a base branch
from
Open

Issue #14068 - Cleanup IO.asFile on Windows #14124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/IOTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,14 @@
import org.eclipse.jetty.util.resource.ResourceFactory;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.EnabledOnOs;
import org.junit.jupiter.api.condition.OS;
import org.junit.jupiter.api.extension.ExtendWith;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.startsWith;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
Expand Down Expand Up @@ -644,4 +646,43 @@ public void testSymbolicLinkDir(WorkDir workDir) throws Exception
assertFalse(Files.isSymbolicLink(linkPath));
}

@Test
@EnabledOnOs(OS.WINDOWS)
public void testAsFileTempDirWindowsStringLinuxSlashes(WorkDir workDir) throws IOException
{
Path testDir = workDir.getEmptyPathDir().resolve("test");
FS.ensureDirExists(testDir);

String testDirStr = testDir.toString().replaceAll("\\\\", "/");
File file = IO.asFile(testDirStr);

assertTrue(Files.isSameFile(file.toPath(), testDir));
}

@Test
@EnabledOnOs(OS.WINDOWS)
public void testAsFileTempDirWindowsStringWindowsSlashes(WorkDir workDir) throws IOException
{
Path testDir = workDir.getEmptyPathDir().resolve("test");
FS.ensureDirExists(testDir);

String testDirStr = testDir.toString();
File file = IO.asFile(testDirStr);

assertTrue(Files.isSameFile(file.toPath(), testDir));
}

@Test
@EnabledOnOs(OS.WINDOWS)
public void testAsFileTempDirWindowsStringFileUri(WorkDir workDir) throws IOException
{
Path testDir = workDir.getEmptyPathDir().resolve("test");
FS.ensureDirExists(testDir);

String testDirStr = testDir.toUri().toASCIIString();
assertThat(testDirStr, startsWith("file:"));
File file = IO.asFile(testDirStr);

assertTrue(Files.isSameFile(file.toPath(), testDir));
}
}
53 changes: 34 additions & 19 deletions jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/IO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -735,36 +735,33 @@ public static long write(GatheringByteChannel out, ByteBuffer[] buffers, int off

/**
* <p>
* Convert an object to a {@link File} if possible.
* Convert an object to a {@link File} if possible.
* </p>
*
* @param fileObject A {@link File}, {@link Path}, {@link String} (supporting absolute, relative, partial, and other URI syntaxes), to be converted into a {@link File}.
* {@code null} will result in a {@code null} return.
* @return A {@link File} representation of the passed argument or {@code null}.
* @param pathObject A {@link File}, {@link Path}, {@link String} (supporting absolute, relative, partial, and other URI syntaxes), to be converted into a {@link File}.
* {@code null} will result in a {@code null} return.
* @return A {@link Path} representation of the passed argument or {@code null}.
*/
public static File asFile(Object fileObject)
public static Path asPath(Object pathObject)
{
if (fileObject == null)
if (pathObject == null)
return null;
if (fileObject instanceof File file)
return file;
if (fileObject instanceof Path path)
return path.toFile();
if (fileObject instanceof String str)
if (pathObject instanceof File file)
return file.toPath();
if (pathObject instanceof Path path)
return path;
if (pathObject instanceof String str)
{
// attempt to support absolute, relative, partial, and URI syntaxes.
if (URIUtil.hasScheme(str))
{
try
{
URI uri = new URI(str);
if (uri.isAbsolute() && !uri.getScheme().equalsIgnoreCase("file"))
if (uri.isAbsolute() && uri.getScheme().equalsIgnoreCase("file"))
{
if (LOG.isDebugEnabled())
LOG.debug("Not a local file system: {}", str);
return null;
return Path.of(uri);
}
return Path.of(uri).toFile();
}
catch (URISyntaxException e)
{
Expand All @@ -780,8 +777,7 @@ public static File asFile(Object fileObject)

try
{
Path path = Path.of(str);
return path.toFile();
return Path.of(str);
}
catch (InvalidPathException x)
{
Expand All @@ -792,10 +788,29 @@ public static File asFile(Object fileObject)
}

if (LOG.isDebugEnabled())
LOG.debug("Not able to be converted to a File object: ({}) {}", fileObject.getClass().getName(), fileObject);
LOG.debug("Not able to be converted to a Path object: ({}) {}", pathObject.getClass().getName(), pathObject);
Copy link
Contributor

@sbordet sbordet Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use LOG.atDebug().

Copy link
Contributor Author

@joakime joakime Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no ambiguity here with arguments and Throwable.

Copy link
Contributor

@sbordet sbordet Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we standardized on that idiom for every case, not just ambiguities with Throwables.

Copy link
Contributor Author

@joakime joakime Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It only applies to Throwables, as it eliminates the ambiguity on the last arg not having an associated referenced parameter ({}). If that Throwable happens to be null, then things to sideways with the currently buggy log4j2 implementation. (no other implementations of slf4j).
This is a recognized and accepted bug on log4j2.

return null;
}

/**
* <p>
* Convert an object to a {@link File} if possible.
* </p>
*
* @param fileObject A {@link File}, {@link Path}, {@link String} (supporting absolute, relative, partial, and other URI syntaxes), to be converted into a {@link File}.
* {@code null} will result in a {@code null} return.
* @return A {@link File} representation of the passed argument or {@code null}.
* @deprecated use {@link #asPath(Object)} instead to have proper conversion of various path syntaxes across JVMs, Architectures, and OSs.
*/
@Deprecated(since = "12.1.6", forRemoval = true)
public static File asFile(Object fileObject)
{
Path path = asPath(fileObject);
if (path == null)
return null;
return path.toFile();
}

private IO()
{
// prevent instantiation
Expand Down