diff --git a/prospero-cli/src/main/java/org/wildfly/prospero/cli/RepositoryDefinition.java b/prospero-cli/src/main/java/org/wildfly/prospero/cli/RepositoryDefinition.java index 499b6debc..039b74619 100644 --- a/prospero-cli/src/main/java/org/wildfly/prospero/cli/RepositoryDefinition.java +++ b/prospero-cli/src/main/java/org/wildfly/prospero/cli/RepositoryDefinition.java @@ -17,13 +17,12 @@ package org.wildfly.prospero.cli; -import org.jboss.logging.Logger; +import org.apache.commons.lang3.StringUtils; import org.wildfly.channel.Repository; import java.net.MalformedURLException; import java.net.URI; import java.net.URISyntaxException; -import java.net.URL; import java.nio.file.Files; import java.nio.file.InvalidPathException; import java.nio.file.Path; @@ -32,8 +31,6 @@ public class RepositoryDefinition { - private static final Logger logger = Logger.getLogger(RepositoryDefinition.class.getName()); - public static List from(List repos) throws ArgumentParsingException { final ArrayList repositories = new ArrayList<>(repos.size()); for (int i = 0; i < repos.size(); i++) { @@ -41,81 +38,76 @@ public static List from(List repos) throws ArgumentParsingEx final String repoId; final String repoUri; - try { - if (repoInfo.contains("::")) { - final String[] parts = repoInfo.split("::"); - if (parts.length != 2 || parts[0].isEmpty() || parts[1].isEmpty()) { - throw CliMessages.MESSAGES.invalidRepositoryDefinition(repoInfo); - } - repoId = parts[0]; - repoUri = parseRepositoryLocation(parts[1]); - } else { - repoId = "temp-repo-" + i; - repoUri = parseRepositoryLocation(repoInfo); + if (repoInfo.contains("::")) { + final String[] parts = repoInfo.split("::"); + if (parts.length != 2 || parts[0].isEmpty() || parts[1].isEmpty()) { + throw CliMessages.MESSAGES.invalidRepositoryDefinition(repoInfo); } - repositories.add(new Repository(repoId, repoUri)); - } catch (URISyntaxException e) { - logger.error("Unable to parse repository uri + " + repoInfo, e); - throw CliMessages.MESSAGES.invalidRepositoryDefinition(repoInfo); + repoId = parts[0]; + repoUri = parseRepositoryLocation(parts[1], true); + } else { + if (StringUtils.isBlank(repoInfo)) { + throw CliMessages.MESSAGES.invalidRepositoryDefinition(repoInfo); + } + repoId = "temp-repo-" + i; + repoUri = parseRepositoryLocation(repoInfo, true); } + repositories.add(new Repository(repoId, repoUri)); } return repositories; } - private static String parseRepositoryLocation(String repoLocation) throws URISyntaxException, ArgumentParsingException { - if (!isRemoteUrl(repoLocation) && !repoLocation.isEmpty()) { - // the repoLocation contains either a file URI or a path - // we need to convert it to a valid file IR - repoLocation = getAbsoluteFileURI(repoLocation).toString(); - } - if (!isValidUrl(repoLocation)){ - throw CliMessages.MESSAGES.invalidRepositoryDefinition(repoLocation); - } - return repoLocation; - } - - private static boolean isRemoteUrl(String repoInfo) { - return repoInfo.startsWith("http://") || repoInfo.startsWith("https://"); - } - - private static boolean isValidUrl(String text) { + static String parseRepositoryLocation(String location, boolean checkLocalPathExists) throws ArgumentParsingException { + URI uri; try { - new URL(text); - return true; - } catch (MalformedURLException e) { - return false; + uri = new URI(location); + if (("file".equals(uri.getScheme()) || StringUtils.isBlank(uri.getScheme())) + && StringUtils.isBlank(uri.getHost())) { + // A "file:" URI with an empty host is assumed to be a local filesystem URL. An empty scheme would mean + // a URI that is just a path without any "proto:" part, which is still assumed a local path. + // A "file:" URI with a non-empty host would signify a remote URL, in which case we don't process it + // further. + if (!uri.isOpaque() && StringUtils.isNotBlank(uri.getScheme())) { + // The path starts with '/' character (not opaque) and has a scheme defined -> we can use + // `Path.of(uri)` to transform into a path, which gracefully handles Windows paths etc. + uri = normalizeLocalPath(Path.of(uri), checkLocalPathExists).toUri(); + } else { + // This is to handle relative URLs like "file:relative/path", which is outside of spec (URI is not + // hierarchical -> cannot use `Path.of(uri)`). Note that `uri.getSchemeSpecificPart()` rather than + // `uri.getPath()` because the URI class doesn't parse the path portion for opaque URIs. + uri = normalizeLocalPath(Path.of(uri.getSchemeSpecificPart()), checkLocalPathExists).toUri(); + } + } + + // Resulting URI must be convertible to URL. + //noinspection ResultOfMethodCallIgnored + uri.toURL(); + } catch (URISyntaxException | MalformedURLException e) { + try { + // If the location is not a valid URI / URL, try to handle it as a path. + Path path = Path.of(location); + uri = normalizeLocalPath(path, checkLocalPathExists).toUri(); + } catch (InvalidPathException e2) { + throw CliMessages.MESSAGES.invalidFilePath(location, e); + } + } catch (IllegalArgumentException e) { + throw CliMessages.MESSAGES.invalidFilePath(location, e); } - } - public static URI getAbsoluteFileURI(String repoInfo) throws ArgumentParsingException, URISyntaxException { - final Path repoPath = getPath(repoInfo).toAbsolutePath().normalize(); - if (Files.exists(repoPath)) { - return repoPath.toUri(); - } else { - throw CliMessages.MESSAGES.nonExistingFilePath(repoPath); + try { + return uri.toURL().toExternalForm(); + } catch (MalformedURLException | IllegalArgumentException e) { + throw CliMessages.MESSAGES.invalidFilePath(location, e); } } - public static Path getPath(String repoInfo) throws URISyntaxException, ArgumentParsingException { - if (repoInfo.startsWith("file:")) { - final URI inputUri = new URI(repoInfo); - if (containsAbsolutePath(inputUri)) { - return Path.of(inputUri); - } else { - return Path.of(inputUri.getSchemeSpecificPart()); - } - } else { - try { - return Path.of(repoInfo); - } catch (InvalidPathException e) { - throw CliMessages.MESSAGES.invalidFilePath(repoInfo, e); - } + private static Path normalizeLocalPath(Path path, boolean checkPathExists) throws ArgumentParsingException { + Path normalized = path.toAbsolutePath().normalize(); + if (checkPathExists && !Files.exists(path)) { + throw CliMessages.MESSAGES.nonExistingFilePath(normalized); } + return normalized; } - private static boolean containsAbsolutePath(URI inputUri) { - // absolute paths in URI (even on Windows) has to start with slash. If not we treat it as a relative path - return inputUri.getSchemeSpecificPart().startsWith("/"); - } } diff --git a/prospero-cli/src/test/java/org/wildfly/prospero/cli/RepositoryDefinitionTest.java b/prospero-cli/src/test/java/org/wildfly/prospero/cli/RepositoryDefinitionTest.java index 2f0f8cca0..977d8b5be 100644 --- a/prospero-cli/src/test/java/org/wildfly/prospero/cli/RepositoryDefinitionTest.java +++ b/prospero-cli/src/test/java/org/wildfly/prospero/cli/RepositoryDefinitionTest.java @@ -18,6 +18,7 @@ package org.wildfly.prospero.cli; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.SystemUtils; import org.assertj.core.api.Assertions; import org.junit.Before; import org.junit.Test; @@ -77,7 +78,7 @@ public void mixGeneratedAndProvidedIds() throws Exception { } @Test - public void throwsErrorIfFormatIsIncorrect() throws Exception { + public void throwsErrorIfFormatIsIncorrect() { assertThrows(ArgumentParsingException.class, ()->from(List.of("::http://test1.te"))); assertThrows(ArgumentParsingException.class, ()->from(List.of("repo-1::"))); @@ -85,9 +86,6 @@ public void throwsErrorIfFormatIsIncorrect() throws Exception { assertThrows(ArgumentParsingException.class, ()->from(List.of("repo-1:::http://test1.te"))); assertThrows(ArgumentParsingException.class, ()->from(List.of("foo::bar::http://test1.te"))); - - assertThrows(ArgumentParsingException.class, ()->from(List.of("imnoturl"))); - } @Test @@ -103,7 +101,7 @@ public void throwsErrorIfFormatIsIncorrectForFileURLorPathDoesNotExist() throws @Test public void testCorrectRelativeOrAbsolutePathForFileURL() throws Exception { - Repository repository = new Repository("temp-repo-0", tempRepoUrlEmptyHostForm); + Repository repository = new Repository("temp-repo-0", tempRepoUrlNoHostForm); List actualList = from(List.of("file:../prospero-cli")); assertNotNull(actualList); @@ -182,4 +180,61 @@ public void testNonExistingFileUri() throws Exception { "The provided path [%s] doesn't exist or is not accessible. The local repository has to an existing, readable folder.", Path.of("idontexist").toAbsolutePath())); } + + @Test + public void testNormalization() throws Exception { + String cwdPath = Path.of(System.getProperty("user.dir")).toUri().getPath(); + + assertThat(RepositoryDefinition.parseRepositoryLocation("file://" + cwdPath, true)) // file:///home/... + .isEqualTo("file:" + cwdPath); + + assertThat(RepositoryDefinition.parseRepositoryLocation("file:" + cwdPath, true)) // file:/home/... + .isEqualTo("file:" + cwdPath); + + assertThat(RepositoryDefinition.parseRepositoryLocation("file://host/some/path", true)) + .isEqualTo("file://host/some/path"); + + assertThat(RepositoryDefinition.parseRepositoryLocation("file:../prospero-cli", true)) + .isEqualTo("file:" + cwdPath); + + try { + RepositoryDefinition.parseRepositoryLocation("file://../prospero-cli", true); // This is interpreted as local absolute path "/../path". + fail("This path should fail because it doesn't exist."); + } catch (ArgumentParsingException e) { + // pass + } + + // On Linux following is interpreted as relative path, on Windows it's an absolute path + if (SystemUtils.IS_OS_WINDOWS) { + assertThat(RepositoryDefinition.parseRepositoryLocation("a:foo/bar", false)) // interpreted as local relative path + .isEqualTo("file:/A:/foo/bar"); + } else { + assertThat(RepositoryDefinition.parseRepositoryLocation("a:foo/bar", false)) // interpreted as local relative path + .isEqualTo("file:" + cwdPath + "a:foo/bar"); + } + } + + @Test + public void testNormalizationWindowsPaths() throws Exception { + String cwdPath = Path.of(System.getProperty("user.dir")).toUri().getPath(); + + assertThat(RepositoryDefinition.parseRepositoryLocation("file:/c:/some/path", false)) + .isEqualTo("file:/c:/some/path"); + + assertThat(RepositoryDefinition.parseRepositoryLocation("file:///c:/some/path", false)) + .isEqualTo("file:/c:/some/path"); + + assertThat(RepositoryDefinition.parseRepositoryLocation("file://host/c:/some/path", false)) + .isEqualTo("file://host/c:/some/path"); + + // On Linux following is interpreted as relative path, on Windows it's an absolute path + if (SystemUtils.IS_OS_WINDOWS) { + assertThat(RepositoryDefinition.parseRepositoryLocation("c:/foo/bar", false)) + .isEqualTo("file:/c:/foo/bar"); + } else { + assertThat(RepositoryDefinition.parseRepositoryLocation("c:/foo/bar", false)) + .isEqualTo("file:" + cwdPath + "c:/foo/bar"); + } + } + } \ No newline at end of file diff --git a/prospero-cli/src/test/java/org/wildfly/prospero/cli/commands/InstallCommandTest.java b/prospero-cli/src/test/java/org/wildfly/prospero/cli/commands/InstallCommandTest.java index 479251982..3872c5627 100644 --- a/prospero-cli/src/test/java/org/wildfly/prospero/cli/commands/InstallCommandTest.java +++ b/prospero-cli/src/test/java/org/wildfly/prospero/cli/commands/InstallCommandTest.java @@ -238,7 +238,7 @@ public void provisionConfigAndRemoteRepoSet() throws Exception { Path channelsFile = temporaryFolder.newFile().toPath(); File installDir = temporaryFolder.newFolder(); - String testURL = installDir.toPath().toUri().toString(); + String testURL = installDir.toPath().toUri().toURL().toExternalForm(); MetadataTestUtils.prepareChannel(channelsFile, List.of(new URL("file:some-manifest.yaml"))); @@ -260,7 +260,7 @@ public void passShadowRepositories() throws Exception { Path channelsFile = temporaryFolder.newFile().toPath(); File installDir = temporaryFolder.newFolder(); - String testURL = installDir.toPath().toUri().toString(); + String testURL = installDir.toPath().toUri().toURL().toExternalForm(); MetadataTestUtils.prepareChannel(channelsFile, List.of(new URL("file:some-manifest.yaml")));