Skip to content

Commit

Permalink
JBEAP-27450 Normalize URL notation via URI.toURL().toExternalForm()
Browse files Browse the repository at this point in the history 
Also tried so simplify the RepositoryDefinition class.
  • Loading branch information
@TomasHofman
TomasHofman committed Jul 24, 2024
1 parent 483e29d commit fbac3af
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 69 deletions.
119 changes: 54 additions & 65 deletions prospero-cli/src/main/java/org/wildfly/prospero/cli/RepositoryDefinition.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -32,90 +31,80 @@

public class RepositoryDefinition {

private static final Logger logger = Logger.getLogger(RepositoryDefinition.class.getName());

public static List<Repository> from(List<String> repos) throws ArgumentParsingException {
final ArrayList<Repository> repositories = new ArrayList<>(repos.size());
for (int i = 0; i < repos.size(); i++) {
final String repoInfo = repos.get(i);
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();
}
}
} catch (URISyntaxException e) {
try {
// To stay backward compatible, if the location is not a valid URI, try to handle is as a path. This
// branch is I think only used for Windows paths given directly as "C:/some/path" - not valid URLs.
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("/");
}
}
46 changes: 45 additions & 1 deletion prospero-cli/src/test/java/org/wildfly/prospero/cli/RepositoryDefinitionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,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<Repository> actualList = from(List.of("file:../prospero-cli"));

assertNotNull(actualList);
Expand Down Expand Up @@ -182,4 +182,48 @@ 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 path = System.getProperty("user.dir");
String urlPath = Path.of(path).toUri().getPath();

List<Repository> repos = from(List.of("repo1::file://" + urlPath)); // file:///home/...
assertThat(repos.size()).isEqualTo(1);
assertThat(repos.get(0).getUrl()).isEqualTo("file:" + urlPath); // Three slashes were converted to single slash.

repos = from(List.of("repo1::file:" + urlPath)); // file:/home/...
assertThat(repos.size()).isEqualTo(1);
assertThat(repos.get(0).getUrl()).isEqualTo("file:" + urlPath); // One slash is still one slash.

repos = from(List.of("repo1::file://host/some/path")); // This is interpreted as a remote URL, not a local file system, existence is not checked.
assertThat(repos.size()).isEqualTo(1);
assertThat(repos.get(0).getUrl()).isEqualTo("file://host/some/path");

repos = from(List.of("repo1::file:../prospero-cli")); // This is interpreted as local relative path.
assertThat(repos.size()).isEqualTo(1);
assertThat(repos.get(0).getUrl())
.isEqualTo(Path.of("../prospero-cli").toAbsolutePath().normalize().toUri().toURL().toExternalForm()); // Path was transformed to absolute.

try {
from(List.of("repo1::file://../prospero-cli")); // This is interpreted as local path "/../path". It fails because the path doesn't exist.
fail("This path should fail because it doesn't exist.");
} catch (ArgumentParsingException e) {
// pass
}
}

@Test
public void testNormalizationWindowsPaths() throws Exception {
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");

}

}
6 changes: 3 additions & 3 deletions prospero-cli/src/test/java/org/wildfly/prospero/cli/commands/InstallCommandTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,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")));

Expand All @@ -287,7 +287,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")));

Expand Down Expand Up @@ -420,7 +420,7 @@ public void multipleManifestsAreTranslatedToMultipleChannels() 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")));

Expand Down

0 comments on commit fbac3af

Please sign in to comment.