Skip to content

Commit

Permalink
[JBEAP-26295] [installation-manager] The file URL passed as --reposit…
Browse files Browse the repository at this point in the history 
…ories argument is not validated correctly
  • Loading branch information
@ChristinaDsl
ChristinaDsl committed Apr 19, 2024
1 parent 9a013a4 commit 5cc91b3
Show file tree
Hide file tree
Showing 7 changed files with 111 additions and 48 deletions.
18 changes: 8 additions & 10 deletions integration-tests/src/test/java/org/wildfly/prospero/it/cli/InstallTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,12 @@
import java.io.File;
import java.net.URL;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;
import java.util.concurrent.TimeUnit;

import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.wildfly.prospero.cli.ReturnCodes;
import org.wildfly.prospero.cli.commands.CliConstants;
import org.wildfly.prospero.it.ExecutionUtils;
Expand All @@ -36,15 +35,12 @@

public class InstallTest extends CliTestBase {

@Rule
public TemporaryFolder tempDir = new TemporaryFolder();

private File targetDir;

@Before
public void setUp() throws Exception {
super.setUp();
targetDir = tempDir.newFolder();
targetDir = temp.newFolder();
}

@Test
Expand All @@ -62,8 +58,8 @@ public void testInstallWithProvisionConfig() throws Exception {

@Test
public void testInstallWithLocalRepositories() throws Exception {
final Path manifestPath = tempDir.newFile().toPath();
final Path provisionConfig = tempDir.newFile().toPath();
final Path manifestPath = temp.newFile().toPath();
final Path provisionConfig = temp.newFile().toPath();
MetadataTestUtils.copyManifest("manifests/wfcore-base.yaml", manifestPath);
MetadataTestUtils.prepareChannel(provisionConfig, List.of(manifestPath.toUri().toURL()));

Expand All @@ -73,10 +69,12 @@ public void testInstallWithLocalRepositories() throws Exception {

final URL temporaryRepo = mockTemporaryRepo(true);

Path relativePath = Paths.get(System.getProperty("user.dir")).relativize(Path.of(temporaryRepo.toURI()));

ExecutionUtils.prosperoExecution(CliConstants.Commands.UPDATE, CliConstants.Commands.PERFORM,
CliConstants.REPOSITORIES, temporaryRepo.toString(),
CliConstants.REPOSITORIES, relativePath.toString(),
CliConstants.Y,
CliConstants.NO_LOCAL_MAVEN_CACHE,
CliConstants.VERBOSE,
CliConstants.DIR, targetDir.getAbsolutePath())
.execute()
.assertReturnCode(ReturnCodes.SUCCESS);
Expand Down
4 changes: 4 additions & 0 deletions prospero-cli/src/main/java/org/wildfly/prospero/cli/CliMessages.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,10 @@ default ArgumentParsingException invalidRepositoryDefinition(String repoKey) {
return new ArgumentParsingException(format(bundle.getString("prospero.general.validation.repo_format"), repoKey));
}

default ArgumentParsingException invalidFilePath(String repoKey) {
return new ArgumentParsingException(format(bundle.getString("prospero.general.validation.file_path.not_exists"), repoKey));
}

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

package org.wildfly.prospero.cli;

import org.jboss.logging.Logger;
import org.wildfly.channel.Repository;

import java.io.File;
import java.net.MalformedURLException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;

public class RepositoryDefinition {

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

public static List<Repository> from(List<String> repos) throws ArgumentParsingException {
ArrayList<Repository> repositories = new ArrayList<>(repos.size());
for (int i = 0; i < repos.size(); i++) {
final String text = repos.get(i);
String text = repos.get(i);
if(text.contains("::")) {
final String[] parts = text.split("::");
if (parts.length != 2 || parts[0].isEmpty() || parts[1].isEmpty() || !isValidUrl(parts[1])) {
throw CliMessages.MESSAGES.invalidRepositoryDefinition(text);
}

repositories.add(new Repository(parts[0], parts[1]));
} else {
if (!isValidUrl(text)) {
if (!text.startsWith("http") && !text.isEmpty()) {
try {
text = getAbsoluteFileURI(text).toString();
} catch (URISyntaxException e) {
logger.warn("An error occurred while processing URI");
}
}
if (!isValidUrl(text)){
throw CliMessages.MESSAGES.invalidRepositoryDefinition(text);
}

repositories.add(new Repository("temp-repo-" + i, text));
}
}
Expand All @@ -52,15 +63,37 @@ public static List<Repository> from(List<String> repos) throws ArgumentParsingEx

private static boolean isValidUrl(String text) {
try {
URL url = new URL(text);
if (text.startsWith("file")){
String path = Paths.get(url.getPath()).normalize().toString();
File f = new File(path);
return f.exists();
}
new URL(text);
return true;
} catch (MalformedURLException e) {
return false;
}
}

public static URI getAbsoluteFileURI(String text) throws ArgumentParsingException, URISyntaxException {
String path;
path = getPath(text);
File file = new File(path);
if (file.exists()) {
return file.toURI();
} else {
throw CliMessages.MESSAGES.invalidFilePath(path);
}
}

public static String getPath(String text) throws URISyntaxException {
if (text.startsWith("file")) {
URI inputUri = new URI(text);
if (inputUri.getScheme().equals("file") &&
!inputUri.getSchemeSpecificPart().startsWith("/")) {
inputUri = new URI(("file:/" +
Paths.get("").toAbsolutePath() +
File.separator +
Paths.get(inputUri.getSchemeSpecificPart())).replace("//","/").replaceAll("\\\\","/"));
}
return Path.of(inputUri).normalize().toFile().getAbsolutePath();
} else {
return Path.of(text).toAbsolutePath().normalize().toString();
}
}
}
4 changes: 3 additions & 1 deletion prospero-cli/src/main/java/org/wildfly/prospero/cli/commands/AbstractInstallCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
import org.wildfly.prospero.wfchannel.MavenSessionManager;
import picocli.CommandLine;

import java.net.MalformedURLException;
import java.net.URISyntaxException;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -100,7 +102,7 @@ protected MavenOptions getMavenOptions() throws ArgumentParsingException {
return mavenOptions.build();
}

protected ProvisioningDefinition.Builder buildDefinition() throws MetadataException, NoChannelException, ArgumentParsingException {
protected ProvisioningDefinition.Builder buildDefinition() throws MetadataException, NoChannelException, ArgumentParsingException, MalformedURLException, URISyntaxException {
return ProvisioningDefinition.builder()
.setFpl(featurePackOrDefinition.fpl.orElse(null))
.setProfile(featurePackOrDefinition.profile.orElse(null))
Expand Down
1 change: 1 addition & 0 deletions prospero-cli/src/main/resources/UsageMessages.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ prospero.general.error.resolve.streams.header=Required artifact streams are not
prospero.general.validation.conflicting_options=Only one of %s and %s can be set.
prospero.general.validation.local_repo.not_directory=Repository path `%s` is a file not a directory.
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 given file path [%s] is invalid.
prospero.general.error.missing_file=Required file at `%s` cannot be opened.
prospero.general.error.galleon.parse=Failed to parse provisioning configuration: %s
prospero.general.error.feature_pack.not_found=The feature pack `%s` is not available in the subscribed channels.
Expand Down
59 changes: 40 additions & 19 deletions 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 @@ -19,35 +19,26 @@

import org.apache.commons.lang3.StringUtils;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.wildfly.channel.Repository;

import java.io.File;
import java.nio.file.Path;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

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

public class RepositoryDefinitionTest {

@Rule
public TemporaryFolder tempDir = new TemporaryFolder();

public String fileURL;

public String filePath;
private String tempRepoURL;

@Before
public void setUp() throws Exception {
File target = tempDir.newFolder();
filePath = target.getAbsolutePath();
fileURL = target.toURI().toURL().toString();
tempRepoURL = Path.of(".").normalize().toAbsolutePath().toUri().toString().replace("///","/");
}

@Test
Expand Down Expand Up @@ -97,17 +88,47 @@ public void throwsErrorIfFormatIsIncorrect() throws Exception {
}

@Test
public void throwsErrorIfFormatIsIncorrectForFileURLOrPathDoesNotExist() throws Exception {
assertThrows(ArgumentParsingException.class, ()->from(List.of("::"+fileURL)));
public void throwsErrorIfFormatIsIncorrectForFileURLorPathDoesNotExist() throws Exception {
assertThrows(ArgumentParsingException.class, ()->from(List.of("::"+tempRepoURL)));

assertThrows(ArgumentParsingException.class, ()->from(List.of("repo-1::")));

assertThrows(ArgumentParsingException.class, ()->from(List.of("repo-1:::"+fileURL)));
assertThrows(ArgumentParsingException.class, ()->from(List.of("repo-1:::"+tempRepoURL)));

assertThrows(ArgumentParsingException.class, ()->from(List.of("foo::bar::"+tempRepoURL)));
}

@Test
public void testCorrectRelativeOrAbsolutePathForFileURL() throws Exception {
Repository repository = new Repository("temp-repo-0", tempRepoURL);
List<Repository> actualList = from(List.of("file:../prospero-cli"));

assertNotNull(actualList);
assertEquals(1, actualList.size());
assertTrue(actualList.contains(repository));

actualList = from(List.of(("file:/"+(System.getProperty("user.dir").replaceAll("\\\\","/"))).replace("//","/")));

assertNotNull(actualList);
assertEquals(1, actualList.size());
assertTrue(actualList.contains(repository));

actualList = from(List.of(("file:///"+(System.getProperty("user.dir").replaceAll("\\\\","/"))).replace("////","///")));

assertNotNull(actualList);
assertEquals(1, actualList.size());
assertTrue(actualList.contains(repository));

actualList = from(List.of(System.getProperty("user.dir")));

assertThrows(ArgumentParsingException.class, ()->from(List.of("foo::bar::"+fileURL)));
assertNotNull(actualList);
assertEquals(1, actualList.size());
assertTrue(actualList.contains(repository));

assertThrows(ArgumentParsingException.class, ()->from(List.of("file:/path/to/repo")));
actualList = from(List.of(".."+File.separator+"prospero-cli"));

assertThrows(ArgumentParsingException.class, ()->from(List.of(filePath)));
assertNotNull(actualList);
assertEquals(1, actualList.size());
assertTrue(actualList.contains(repository));
}
}
20 changes: 12 additions & 8 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 @@ -241,41 +241,45 @@ public void provisionConfigAndChannelSet() throws IOException {
@Test
public void provisionConfigAndRemoteRepoSet() throws Exception {
Path channelsFile = temporaryFolder.newFile().toPath();
String test = temporaryFolder.newFolder().toURI().toURL().toString();

File test = temporaryFolder.newFolder();
String testURL = test.toURI().toURL().toString();

MetadataTestUtils.prepareChannel(channelsFile, List.of(new URL("file:some-manifest.yaml")));

int exitCode = commandLine.execute(CliConstants.Commands.INSTALL, CliConstants.DIR, test,
int exitCode = commandLine.execute(CliConstants.Commands.INSTALL, CliConstants.DIR, test.getName(),
CliConstants.CHANNELS, channelsFile.toString(),
CliConstants.REPOSITORIES, test,
CliConstants.REPOSITORIES, testURL,
CliConstants.FPL, "g:a");

assertEquals(ReturnCodes.SUCCESS, exitCode);
Mockito.verify(provisionAction).provision(configCaptor.capture(), channelCaptor.capture(), any());
assertThat(channelCaptor.getValue().get(0).getRepositories()
.stream().map(Repository::getUrl).collect(Collectors.toList()))
.containsOnly(test);
.containsOnly(testURL);
}

@SuppressWarnings("unchecked")
@Test
public void passShadowRepositories() throws Exception {
Path channelsFile = temporaryFolder.newFile().toPath();
String test = temporaryFolder.newFolder().toURI().toURL().toString();

File test = temporaryFolder.newFolder();
String testURL = test.toURI().toURL().toString();

MetadataTestUtils.prepareChannel(channelsFile, List.of(new URL("file:some-manifest.yaml")));

int exitCode = commandLine.execute(CliConstants.Commands.INSTALL, CliConstants.DIR, test,
int exitCode = commandLine.execute(CliConstants.Commands.INSTALL, CliConstants.DIR, test.getName(),
CliConstants.CHANNELS, channelsFile.toString(),
CliConstants.SHADE_REPOSITORIES, test,
CliConstants.SHADE_REPOSITORIES, testURL,
CliConstants.FPL, "g:a");

assertEquals(ReturnCodes.SUCCESS, exitCode);
final ArgumentCaptor<List<Repository>> listArgumentCaptor = ArgumentCaptor.forClass(List.class);
Mockito.verify(provisionAction).provision(configCaptor.capture(), channelCaptor.capture(), listArgumentCaptor.capture());
assertThat(listArgumentCaptor.getValue())
.map(Repository::getUrl)
.contains(test);
.contains(testURL);
}

@Test
Expand Down

0 comments on commit 5cc91b3

Please sign in to comment.