Skip to content

Commit

Permalink
fix(CLI): Issue 26879 cli pushing files whose name contain whitespace…
Browse files Browse the repository at this point in the history
…s fail (dotCMS#27696)

* dotCMS#26879 Change methods to use URI's raw path and improve URL encoding

The `AssetPathResolver` has been updated to use the URI's raw path rather than the path. Additionally, the `resolveAssetAndFolder` method now takes a `decodedRawPath` parameter rather than decoding the raw path within the method. Similarly, the `encodePath` method in `LocationUtils` has been significantly revised which now encodes the URL path segment by segment and maintains both single and double slashes at the beginning of URLs.

* dotCMS#27423 Related init command missing changes
dotCMS#26879 Better handling for white spaces and uncommon characters in names

* dotCMS#26879 Adding IT
  • Loading branch information
jgambarios authored and spbolton committed Feb 28, 2024
1 parent b9b37f5 commit f8f9376
Show file tree
Hide file tree
Showing 10 changed files with 111 additions and 88 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public ResolvedAssetAndPath resolve(final String url, final User user, final boo
}

final Host host = siteByName.get();
final String path = BLANK.equals(uri.getPath()) ? FORWARD_SLASH : uri.getPath();
final String path = BLANK.equals(uri.getRawPath()) ? FORWARD_SLASH : uri.getRawPath();
if (null == path) {
throw new IllegalArgumentException(String.format("Unable to determine path: [%s].", url));
}
Expand All @@ -87,7 +87,8 @@ public ResolvedAssetAndPath resolve(final String url, final User user, final boo
final Optional<Folder> folder = resolveExistingFolder(decodedPath, host, user);
if(folder.isEmpty()){
//if we've got this far we need to verify if the path is an asset. The folder will be expected to be the parent folder
Optional<FolderAndAsset> folderAndAsset = resolveAssetAndFolder(uri.getPath(), host, user, createMissingFolders);
Optional<FolderAndAsset> folderAndAsset = resolveAssetAndFolder(decodedPath, host,
user, createMissingFolders);
if(folderAndAsset.isEmpty()){
throw new NotFoundInDbException(String.format("Unable to determine a valid folder or asset from uri: [%s].", url));
}
Expand All @@ -102,7 +103,8 @@ public ResolvedAssetAndPath resolve(final String url, final User user, final boo
}

//if we succeed to determine a valid folder from the path then we resolve the last bit as an asset name
final String resource = uri.getRawQuery() != null ? uri.getPath() + "?" + uri.getRawQuery() : uri.getPath();
final String resource = uri.getRawQuery() != null ?
uri.getRawPath() + "?" + uri.getRawQuery() : uri.getRawPath();
final Optional<String> asset = asset(folder.get(), resource);

final ResolvedAssetAndPath.Builder builder = ResolvedAssetAndPath.builder();
Expand Down Expand Up @@ -160,18 +162,17 @@ Optional<Folder> resolveExistingFolder(final String rawPath, final Host host, fi
/**
* here we test a specific case we try to resolve anything that matches a pattern like forward slash followed by a string
*
* @param rawPath
* @param decodedRawPath the decoded raw path
* @param host
* @param user
* @return
* @throws DotDataException
* @throws DotSecurityException
*/
Optional<FolderAndAsset> resolveAssetAndFolder(final String rawPath, final Host host, final User user, final boolean createMissingFolder)
Optional<FolderAndAsset> resolveAssetAndFolder(final String decodedRawPath, final Host host,
final User user, final boolean createMissingFolder)
throws DotDataException, DotSecurityException {

final var decodedRawPath = URLDecoder.decode(rawPath, StandardCharsets.UTF_8);

final String startsWithForwardSlash = "^\\/[a-zA-Z0-9\\.\\-]+$";
// if our path starts with / followed by a string then we're looking at file asset in the root folder
if (decodedRawPath.matches(startsWithForwardSlash)) {
Expand Down
12 changes: 6 additions & 6 deletions tools/dotcms-cli/action/.github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,15 @@ jobs:
echo "$GITHUB_ENV"
- name: Get changes
id: changed-files
run: |
echo "changed_files=$(git diff --name-only ${{ github.event.before }} ${{ github.event.after }} | xargs)" >> $GITHUB_OUTPUT
git diff --name-only ${{ github.event.before }} ${{ github.event.after }} > changed_files.txt
- name: List changed files
run: |
for file in ${{ steps.changed-files.outputs.changed_files }}; do
echo "$file was changed"
done
while IFS= read -r line
do
echo "\"$line\" was changed"
done < changed_files.txt
- name: Github Event Context properties
run: |
Expand Down Expand Up @@ -90,7 +90,7 @@ jobs:
run: |
chmod +x ./.github/workflows/scripts/run-push.sh
source ./.github/workflows/scripts/run-push.sh
install_cli "${{env.DOT_CLI_JAR_DOWNLOAD_URL}}" "${{env.DOT_FORCE_DOWNLOAD}}" "${{env.DOT_API_URL}}"
install_cli "${{env.DOT_CLI_JAR_DOWNLOAD_URL}}" "${{env.DOT_FORCE_DOWNLOAD}}"
run_cli_push "${{github.workspace}}${{env.DOT_REPO_BASE_PATH}}" "${{ secrets.DOT_TOKEN }}" "${{ env.DOT_CLI_OPTS }}"
echo "exit-code=$exit_code" >> "$GITHUB_OUTPUT"
print_log
Expand Down
17 changes: 5 additions & 12 deletions tools/dotcms-cli/action/.github/workflows/scripts/run-push.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
DOT_SERVICE_YML="dot-service.yml"
RUN_JAVA_VERSION=1.3.8

SERVICES_FILE_CONTENT='
- name: "default"
SERVICES_FILE_CONTENT="
- name: \"default\"
active: true
'
url: \"$DOT_API_URL\"
"

_make_home(){
if [ ! -d "$DOT_CLI_HOME" ]; then
Expand Down Expand Up @@ -52,7 +53,6 @@ _get_run_java_script(){
}

_setup_CLI(){
API_URL=$1
#Lets create the services file dot-service.yml
#the services yml is used to store the server configurations or profiles if you Will
DOT_SERVICES_HOME=$HOME/.dotcms/
Expand All @@ -69,13 +69,7 @@ _setup_CLI(){
# Now generate the file
echo "$SERVICES_FILE_CONTENT" >> "$SERVICE_FILE";

#Tell the CLI to use the demo server through the profile "default"
#The suffix value used to create the environment value must match the name on dot-service.yml file in this case we are using default
#dotcms.client.servers.default=https://demo.dotcms.com/api

export DOTCMS_CLIENT_SERVERS_DEFAULT=$API_URL
export QUARKUS_LOG_FILE_PATH=$DOT_CLI_HOME"dotcms-cli.log"

}

print_log(){
Expand Down Expand Up @@ -105,12 +99,11 @@ _run_cli_push(){
install_cli(){
cli_release_download_url=$1
force_download=$2
dotApiURL=$3

_make_home
_get_CLI "$cli_release_download_url" "$force_download"
_get_run_java_script "$force_download"
_setup_CLI "$dotApiURL"
_setup_CLI
}

run_cli_push(){
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,36 @@ public static Path localPathFromAssetData(final String workspace, final String s
}

/**
* Encodes the given path by replacing spaces with URL-encoded spaces.
* Encodes a URL path by URL encoding each path segment.
*
* @param path the path to be encoded
* @return the encoded path
* @param path the URL path to encode
* @return the encoded URL path
*/
public static String encodePath(final String path) {
return path.replaceAll(" ",
URLEncoder.encode(" ", StandardCharsets.UTF_8));

var url = path;
boolean startsWithDoubleSlash = url.startsWith("//");

// If it starts with double slash, remove them
if (startsWithDoubleSlash) {
url = url.substring(2);
}

// Split the URL into individual parts/segments based on "/" separator
String[] parts = url.split("/");

StringBuilder encodedUrlBuilder = new StringBuilder();
for (String part : parts) {
String encodedPart = URLEncoder.encode(part, StandardCharsets.UTF_8);
encodedUrlBuilder.append('/').append(encodedPart);
}

// If the original URL started with a double slash, we add a slash back at the beginning
if (startsWithDoubleSlash) {
encodedUrlBuilder.insert(0, "/");
}

return encodedUrlBuilder.toString();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@
import io.quarkus.test.junit.TestProfile;
import java.io.IOException;
import java.net.URL;
import java.net.URLDecoder;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
Expand Down Expand Up @@ -146,12 +144,9 @@ void validateSite(final Path tempFolder, final String testSiteName)
}
try (final Stream<Path> walk = Files.walk(tempFolder)) {
walk.filter(Files::isRegularFile)
.forEach(path -> {
String decodedPath = URLDecoder.decode(
tempFolder.relativize(path).toString(),
StandardCharsets.UTF_8);
collectedFiles.add(decodedPath);
});
.forEach(path -> collectedFiles.add(
tempFolder.relativize(path).toString()
));
}
//Let's remove the workspace folder from the list
List<String> existingFolders = collectedFolders.stream()
Expand Down Expand Up @@ -180,7 +175,7 @@ void validateSite(final Path tempFolder, final String testSiteName)
Map.entry(basePath + "/folder2",
Arrays.asList("subFolder2-1", "subFolder2-2", "subFolder2-3")),
Map.entry(basePath + "/folder2/subFolder2-1",
Arrays.asList("subFolder2-1-1", "subFolder2-1-2",
Arrays.asList("subFolder2-1-1-子資料夾", "subFolder2-1-2",
"subFolder2-1-3")),
Map.entry(basePath + "/folder2/subFolder2-2",
Collections.emptyList()),
Expand All @@ -197,8 +192,8 @@ void validateSite(final Path tempFolder, final String testSiteName)
basePath, Collections.emptyList(),
basePath + "/folder1/subFolder1-1/subFolder1-1-1",
Arrays.asList("image1.png", "image4.jpg"),
basePath + "/folder2/subFolder2-1/subFolder2-1-1",
Arrays.asList("image2.png"),
basePath + "/folder2/subFolder2-1/subFolder2-1-1-子資料夾",
Arrays.asList("image2.png", "這就是我的想像6.png", "image (7)+.png"),
basePath + "/folder3", Arrays.asList("image 3.png"),
basePath + "/folder4 withSpace", Arrays.asList("image5.jpg")
);
Expand Down Expand Up @@ -329,11 +324,8 @@ void Test_Folders_Check() throws IOException {
}
try (final Stream<Path> walk = Files.walk(tempFolder)) {
walk.filter(Files::isRegularFile)
.forEach(path -> {
String decodedPath = URLDecoder.decode(
tempFolder.relativize(path).toString(), StandardCharsets.UTF_8);
collectedFiles.add(decodedPath);
});
.forEach(
path -> collectedFiles.add(tempFolder.relativize(path).toString()));
}
//Let's remove the workspace folder from the list
List<String> existingFolders = collectedFolders.stream().map(folder -> folder.replaceAll(
Expand All @@ -357,7 +349,8 @@ void Test_Folders_Check() throws IOException {
Map.entry(basePath + "/folder2",
Arrays.asList("subFolder2-1", "subFolder2-2", "subFolder2-3")),
Map.entry(basePath + "/folder2/subFolder2-1",
Arrays.asList("subFolder2-1-1", "subFolder2-1-2", "subFolder2-1-3")),
Arrays.asList("subFolder2-1-1-子資料夾", "subFolder2-1-2",
"subFolder2-1-3")),
Map.entry(basePath + "/folder2/subFolder2-2",
Collections.emptyList()),
Map.entry(basePath + "/folder2/subFolder2-3",
Expand All @@ -372,7 +365,8 @@ void Test_Folders_Check() throws IOException {
Map<String, List<String>> expectedFiles = Map.of(
basePath, Collections.emptyList(),
basePath + "/folder1/subFolder1-1/subFolder1-1-1", Arrays.asList("image1.png", "image4.jpg"),
basePath + "/folder2/subFolder2-1/subFolder2-1-1", Arrays.asList("image2.png"),
basePath + "/folder2/subFolder2-1/subFolder2-1-1-子資料夾",
Arrays.asList("image2.png", "這就是我的想像6.png", "image (7)+.png"),
basePath + "/folder3", Arrays.asList("image 3.png"),
basePath + "/folder4 withSpace", Arrays.asList("image5.jpg")
);
Expand Down Expand Up @@ -494,11 +488,9 @@ void Test_Asset_Check() throws IOException {
}
try (final Stream<Path> walk = Files.walk(tempFolder)) {
walk.filter(Files::isRegularFile)
.forEach(path -> {
String decodedPath = URLDecoder.decode(
tempFolder.relativize(path).toString(), StandardCharsets.UTF_8);
collectedFiles.add(decodedPath);
});
.forEach(
path -> collectedFiles.add(tempFolder.relativize(path).toString())
);
}
//Let's remove the workspace folder from the list
List<String> existingFolders = collectedFolders.stream()
Expand Down Expand Up @@ -594,7 +586,7 @@ void Test_Asset_Check2() throws IOException {
final var testSiteName = filesTestHelper.prepareData();

final var folderPath = String.format(
"//%s/folder2/subFolder2-1/subFolder2-1-1/image2.png", testSiteName);
"//%s/folder2/subFolder2-1/subFolder2-1-1-子資料夾/image2.png", testSiteName);

// Pulling the content
OutputOptionMixin outputOptions = new MockOutputOptionMixin();
Expand Down Expand Up @@ -638,11 +630,9 @@ void Test_Asset_Check2() throws IOException {
}
try (final Stream<Path> walk = Files.walk(tempFolder)) {
walk.filter(Files::isRegularFile)
.forEach(path -> {
String decodedPath = URLDecoder.decode(
tempFolder.relativize(path).toString(), StandardCharsets.UTF_8);
collectedFiles.add(decodedPath);
});
.forEach(
path -> collectedFiles.add(tempFolder.relativize(path).toString())
);
}
//Let's remove the workspace folder from the list
List<String> existingFolders = collectedFolders.stream()
Expand All @@ -656,13 +646,14 @@ void Test_Asset_Check2() throws IOException {
Map<String, List<String>> expectedFolders = Map.of(
basePath, Arrays.asList("folder2"),
basePath + "/folder2", Arrays.asList("subFolder2-1"),
basePath + "/folder2/subFolder2-1", Arrays.asList("subFolder2-1-1")
basePath + "/folder2/subFolder2-1", Arrays.asList("subFolder2-1-1-子資料夾")
);

// Expected folder structure based on the treeNode object
Map<String, List<String>> expectedFiles = Map.of(
basePath, Collections.emptyList(),
basePath + "/folder2/subFolder2-1/subFolder2-1-1", Arrays.asList("image2.png")
basePath + "/folder2/subFolder2-1/subFolder2-1-1-子資料夾",
Arrays.asList("image2.png")
);

// Validate the actual folders against the expected folders
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,8 +263,8 @@ void Test_Push_New_Site() throws IOException {
var treeNodePushInfo = treeNode.collectPushInfo();

// Should be nothing to push as we are pushing the same folder we pull
Assertions.assertEquals(5, treeNodePushInfo.assetsToPushCount());
Assertions.assertEquals(5, treeNodePushInfo.assetsNewCount());
Assertions.assertEquals(7, treeNodePushInfo.assetsToPushCount());
Assertions.assertEquals(7, treeNodePushInfo.assetsNewCount());
Assertions.assertEquals(0, treeNodePushInfo.assetsModifiedCount());
Assertions.assertEquals(0, treeNodePushInfo.assetsToDeleteCount());
Assertions.assertEquals(9, treeNodePushInfo.foldersToPushCount());
Expand Down Expand Up @@ -309,8 +309,10 @@ void Test_Push_New_Site() throws IOException {
//Validating the tree
// subFolder1-1-1 (has 2 asset)
Assertions.assertEquals(2, newSiteTreeNode.children().get(0).children().get(0).children().get(0).assets().size());
// subFolder2-1-1 (has 1 asset)
Assertions.assertEquals(1, newSiteTreeNode.children().get(1).children().get(0).children().get(0).assets().size());
// subFolder2-1-1-子資料夾 (has 3 asset)
Assertions.assertEquals(3,
newSiteTreeNode.children().get(1).
children().get(0).children().get(0).assets().size());
// Folder 3 (has 1 asset)
Assertions.assertEquals(1, newSiteTreeNode.children().get(2).assets().size());

Expand Down Expand Up @@ -381,7 +383,8 @@ void Test_Push_Modified_Data() throws IOException {
//If we want a folder to be removed from the remote instance it needs to be removed from all our folder branches for good
Path workingFolderToRemove = Paths.get(absolutePath.toString(),"working","en-us",testSiteName,"folder3");
// Removing an asset
Path assetToRemove = Paths.get(absolutePath.toString(),"live","en-us",testSiteName,"folder2","subFolder2-1","subFolder2-1-1","image2.png");
Path assetToRemove = Paths.get(absolutePath.toString(), "live", "en-us", testSiteName,
"folder2", "subFolder2-1", "subFolder2-1-1-子資料夾", "image2.png");

FileUtils.deleteDirectory(liveFolderToRemove.toFile());
FileUtils.deleteDirectory(workingFolderToRemove.toFile());
Expand Down Expand Up @@ -477,8 +480,9 @@ void Test_Push_Modified_Data() throws IOException {
//Validating the tree
// subFolder1-1-1 (has 2 asset)
Assertions.assertEquals(2, updatedTreeNode.children().get(0).children().get(0).children().get(0).assets().size());
// subFolder2-1-1 (has 0 asset)
Assertions.assertEquals(0, updatedTreeNode.children().get(1).children().get(0).children().get(0).assets().size());
// subFolder2-1-1-子資料夾 (has 2 assets) -> 1 was removed
Assertions.assertEquals(2, updatedTreeNode.children().
get(1).children().get(0).children().get(0).assets().size());
// Folder 4 withSpace (has 1 asset)
Assertions.assertEquals(1, updatedTreeNode.children().get(2).assets().size());
// Folder 5 (has 1 asset)
Expand Down
Loading

0 comments on commit f8f9376

Please sign in to comment.