Skip to content
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

Jump to bottom

JBEAP-27450 Normalize URL notation via URI.toURL().toExternalForm() #743

Merged
merged 1 commit into from
Sep 13, 2024
Merged

JBEAP-27450 Normalize URL notation via URI.toURL().toExternalForm() #743

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
122 changes: 57 additions & 65 deletions prospero-cli/src/main/java/org/wildfly/prospero/cli/RepositoryDefinition.java
View file
Open in desktop
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,83 @@

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();
}
}

// 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("/");
}
}
65 changes: 60 additions & 5 deletions prospero-cli/src/test/java/org/wildfly/prospero/cli/RepositoryDefinitionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -77,17 +78,14 @@ 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::")));

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
Expand All @@ -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<Repository> actualList = from(List.of("file:../prospero-cli"));

assertNotNull(actualList);
Expand Down Expand Up @@ -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");
}
}

}
4 changes: 2 additions & 2 deletions prospero-cli/src/test/java/org/wildfly/prospero/cli/commands/InstallCommandTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")));

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

Expand Down
Loading