Skip to content

Use File(URI).toPath() to fix Windows drive letter handling#6993

Merged
pdelagrave merged 1 commit intomainfrom
pstreef/file-uri-windows-fix
Mar 16, 2026
Merged

Use File(URI).toPath() to fix Windows drive letter handling#6993
pdelagrave merged 1 commit intomainfrom
pstreef/file-uri-windows-fix

Conversation

@pstreef
Copy link
Contributor

@pstreef pstreef commented Mar 16, 2026

Problem

v8.75.6 changed Paths.get(URI.create(...)) to Paths.get(URI.create(...).getPath()) to fix percent-encoded non-ASCII characters in file URIs. However, URI.getPath() on file:///C:/Users/... returns /C:/Users/... — the leading / before the drive letter causes Paths.get(String) to fail on Windows.

Solution

Replace Paths.get(URI.create(...).getPath()) with new File(URI.create(...)).toPath() in all three locations. new File(URI) internally calls URI.getPath() for percent-decoding and then fs.fromURIPath() which strips the leading / on Windows drive letters — handling both non-ASCII characters and Windows paths correctly.

URI.getPath() on file:///C:/... returns /C:/... which Paths.get(String)
cannot handle on Windows. new File(URI).toPath() correctly decodes
percent-encoded non-ASCII characters AND handles Windows drive letters.
@github-project-automation github-project-automation bot moved this to In Progress in OpenRewrite Mar 16, 2026
@pdelagrave pdelagrave marked this pull request as ready for review March 16, 2026 16:48
@github-project-automation github-project-automation bot moved this from In Progress to Ready to Review in OpenRewrite Mar 16, 2026
if ("file".equals(scheme)) {
// A maven repository can be expressed as a URI with a file scheme
Path path = Paths.get(URI.create(baseUri + "maven-metadata-local.xml").getPath());
Path path = new File(URI.create(baseUri + "maven-metadata-local.xml")).toPath();
Copy link
Contributor

Choose a reason for hiding this comment

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

If you drop the getPath on the URI that should fix this.

ie.

Paths.get(URI.create(baseUri + "maven-metadata-local.xml"))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But that also just reverts the original change to unescape non ascii chars like ü.

Copy link
Contributor

Choose a reason for hiding this comment

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

I saw that late sorry (CC was waiting for the workflow to go green before merging, and had no instruction to check for new comments before merging, updated CLAUDE.md now).

That's what we used to do until this PR last week: #6960

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm observing that Java NIO Path does unescape the URI string correctly using the original code, so I think we're still missing something that's not being addressed by either of these two PRs.

Copy link
Contributor Author

@pstreef pstreef Mar 17, 2026

Choose a reason for hiding this comment

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

@Test
void fileUriWithNonAsciiPath(@TempDir Path tempDir) throws Exception {
    Path repo = tempDir.resolve("müller/.m2/repository");
    String uri = "file://" + repo.toAbsolutePath() + "/";

    assertThatThrownBy(() -> Paths.get(URI.create(uri)))
      .isInstanceOf(IllegalArgumentException.class)
      .hasMessageContaining("Bad escape");
}

@Test
void fileUriWithNonAsciiPath(@TempDir Path tempDir) throws Exception {
    Path repo = tempDir.resolve("müller/.m2/repository");
    String uri = "file://" + repo.toAbsolutePath() + "/";

    assertDoesNotThrow(() -> new File(URI.create(uri)).toPath());
}

@pdelagrave pdelagrave merged commit 457fcc1 into main Mar 16, 2026
1 check passed
@pdelagrave pdelagrave deleted the pstreef/file-uri-windows-fix branch March 16, 2026 17:02
@github-project-automation github-project-automation bot moved this from Ready to Review to Done in OpenRewrite Mar 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants