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() #737

Merged
merged 2 commits into from
Jul 29, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fd6c1ac
JBEAP-27450 Normalize URL notation via URI.toURL().toExternalForm()
TomasHofman Jul 23, 2024
01128f0
Limit repository URLs to file, http, https schemas
TomasHofman Jul 26, 2024
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
8 changes: 8 additions & 0 deletions prospero-cli/src/main/java/org/wildfly/prospero/cli/CliMessages.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,14 @@ default ArgumentParsingException nonExistingFilePath(Path nonExistingPath) {
return new ArgumentParsingException(format(bundle.getString("prospero.general.validation.file_path.not_exists"), nonExistingPath));
}

default ArgumentParsingException unsupportedRemoteScheme(String url) {
return new ArgumentParsingException(format(bundle.getString("prospero.general.validation.url.unsupported_remote_scheme"), url));
}

default ArgumentParsingException unsupportedScheme(String url) {
return new ArgumentParsingException(format(bundle.getString("prospero.general.validation.url.unsupported_scheme"), url));
}

default IllegalArgumentException updateCandidateStateNotMatched(Path targetDir, Path updateDir) {
return new IllegalArgumentException(format(bundle.getString("prospero.updates.apply.validation.candidate.outdated"), targetDir, updateDir));
}
Expand Down
128 changes: 66 additions & 62 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,22 +17,22 @@

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;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

public class RepositoryDefinition {

private static final Logger logger = Logger.getLogger(RepositoryDefinition.class.getName());
private static final List<String> ALLOWED_SCHEMAS = Arrays.asList("file", "http", "https");

public static List<Repository> from(List<String> repos) throws ArgumentParsingException {
final ArrayList<Repository> repositories = new ArrayList<>(repos.size());
Expand All @@ -41,81 +41,85 @@ public static List<Repository> from(List<String> 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);
}
repoId = parts[0];
repoUri = parseRepositoryLocation(parts[1], true);
} else {
if (StringUtils.isBlank(repoInfo)) {
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 = "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;
}
static String parseRepositoryLocation(String location, boolean checkLocalPathExists) throws ArgumentParsingException {
URI uri;
try {
uri = new URI(location);

private static boolean isRemoteUrl(String repoInfo) {
return repoInfo.startsWith("http://") || repoInfo.startsWith("https://");
}
if ("file".equals(uri.getScheme()) || StringUtils.isBlank(uri.getScheme())) {
if (StringUtils.isNotBlank(uri.getHost())) {
throw CliMessages.MESSAGES.unsupportedRemoteScheme(location);
}

private static boolean isValidUrl(String text) {
try {
new URL(text);
return true;
} catch (MalformedURLException e) {
return false;
}
}
// 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();
}
}

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);
}
}
// Resulting URI must be convertible to URL.
//noinspection ResultOfMethodCallIgnored
uri.toURL();

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());
// Check it is supported scheme.
if (StringUtils.isNotBlank(uri.getScheme()) && !ALLOWED_SCHEMAS.contains(uri.getScheme())) {
throw CliMessages.MESSAGES.unsupportedScheme(location);
}
} else {
} catch (URISyntaxException | MalformedURLException e) {
try {
return Path.of(repoInfo);
} catch (InvalidPathException e) {
throw CliMessages.MESSAGES.invalidFilePath(repoInfo, e);
// 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);
}

try {
return uri.toURL().toExternalForm();
} catch (MalformedURLException | IllegalArgumentException e) {
throw CliMessages.MESSAGES.invalidFilePath(location, e);
}
}

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

}
2 changes: 2 additions & 0 deletions prospero-cli/src/main/resources/UsageMessages.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,8 @@ prospero.general.validation.local_repo.not_directory=Repository path `%s` is a f
prospero.general.validation.repo_format=Repository definition [%s] is invalid. The definition format should be [id::url] or [url].
prospero.general.validation.file_path.not_exists=The provided path [%s] doesn't exist or is not accessible. The local repository has to an existing, readable folder.
prospero.general.validation.file_path.invalid=The given file path [%s] is invalid.
prospero.general.validation.url.unsupported_remote_scheme=Invalid URL, only the "http" and "https" schemas are supported for remote URLs: [%s]
prospero.general.validation.url.unsupported_scheme=Invalid URL, only the "file", "http" and "https" schemas are supported: [%s]

prospero.general.error.missing_file=Required file at `%s` cannot be opened.
prospero.general.error.galleon.parse=Failed to parse provisioning configuration: %s
Expand Down
76 changes: 71 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 All @@ -30,6 +31,7 @@
import java.util.stream.Collectors;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.junit.Assert.*;
import static org.wildfly.prospero.cli.RepositoryDefinition.from;

Expand Down Expand Up @@ -77,17 +79,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 +102,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 +181,71 @@ 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);

assertThatExceptionOfType(ArgumentParsingException.class)
.isThrownBy(() -> {
RepositoryDefinition.parseRepositoryLocation("file://host/some/path", true);
});

assertThat(RepositoryDefinition.parseRepositoryLocation("file:../prospero-cli", true))
.isEqualTo("file:" + cwdPath);

assertThatExceptionOfType(ArgumentParsingException.class)
.isThrownBy(() -> {
RepositoryDefinition.parseRepositoryLocation("file://../prospero-cli", true); // This is interpreted as local absolute path "/../path". });
});

// 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");
assertThatExceptionOfType(ArgumentParsingException.class)
.isThrownBy(() -> {
RepositoryDefinition.parseRepositoryLocation("a:foo/bar", true); // This is interpreted as local absolute path "/../path". });
});
} else {
assertThat(RepositoryDefinition.parseRepositoryLocation("a:foo/bar", false)) // interpreted as local relative path
.isEqualTo("file:" + cwdPath + "a:foo/bar");
assertThatExceptionOfType(ArgumentParsingException.class)
.isThrownBy(() -> {
RepositoryDefinition.parseRepositoryLocation("a:foo/bar", true); // This is interpreted as local absolute path "/../path". });
});
}
}

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

assertThatExceptionOfType(ArgumentParsingException.class)
.isThrownBy(() -> {
RepositoryDefinition.parseRepositoryLocation("file://host/c:/some/path", false);
});

// 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");
}
}

}
6 changes: 3 additions & 3 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 @@ -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
Loading