Skip to content

Commit

Permalink
#24737 Update code to support file assets with spaces (#25783)
Browse files Browse the repository at this point in the history
* #24737 Update code to support file assets with spaces

The code has been updated to support file assets with spaces in the name and to encode/decode URIs accordingly. The gitignore file has been updated to ignore .war/ files. Changes were made in several locations including the RemoteTraversalServiceTest.java and LocationUtils.java files to improve the handling of files with spaces in their names.

* #24737 Update test folder structure and API usage for handling spaces

Refactored folder structure creation in tests to include a folder with a space in its name to verify the proper handling of such cases. Adjusted API calls in tests to use a new encodePath() method which handles spaces in folder and file names, ensuring a proper URL encoding. The change in the AssetsUtils was needed to handle any spaces in the asset names when creating the remote folder path.

Also, updated references to URLDecoder to use the new encodePath() method, simplifying the process and ensuring consistency. This affects methods such as 'resolveAssetAndFolder' in 'AssetPathResolver.java'.

These changes are necessary for ensuring proper test coverage for folders and files with spaces, and for improving code readability and maintainability.

* Reverting changes

* #24737 Improve tests for handling of assets and folders with spaces in names
  • Loading branch information
jgambarios authored Aug 17, 2023
1 parent 1e5fb4d commit 09f9e1e
Show file tree
Hide file tree
Showing 14 changed files with 946 additions and 592 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ dotCMS/src/main/webapp/dotAdmin
dotCMS/src/main/webapp/dotcms-webcomponents
dotCMS/bin/*
dotCMS/classes
WAR/

# Changes after gradle migration #
###############################
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@
import com.dotmarketing.exception.DotDataException;
import com.dotmarketing.exception.DotSecurityException;
import com.dotmarketing.portlets.folders.model.Folder;
import java.net.URISyntaxException;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Test;

Expand All @@ -25,7 +29,8 @@ public static void prepare() throws Exception {
IntegrationTestInitService.getInstance().init();
host = new SiteDataGen().nextPersisted(true);
Folder foo = new FolderDataGen().site(host).name("foo").nextPersisted();
final Folder bar = new FolderDataGen().parent(foo).name("bar").nextPersisted();
new FolderDataGen().parent(foo).name("bar").nextPersisted();
new FolderDataGen().parent(foo).name("test withSpace").nextPersisted();
}

/**
Expand Down Expand Up @@ -61,6 +66,128 @@ public void Test_Parse_Path_With_Asset() throws DotDataException, DotSecurityExc
assertEquals("1234", parse.asset());
}

/**
* Given scenario: host followed by path with an encoded asset name. The asset name contains
* spaces that have been encoded.
* <p>
* Expected: host is resolved, path contains the original asset name with spaces, and asset name
* is the original decoded value.
*
* @throws DotDataException if there is an error accessing data
* @throws DotSecurityException if there is a security violation
*/
@Test
public void Test_Parse_Path_With_Encoded_Asset() throws DotDataException, DotSecurityException {

String url = String.format("http://%s/foo/bar/qwe rty.png", host.getHostname());
url = url.replaceAll(" ", URLEncoder.encode(" ", StandardCharsets.UTF_8));

final ResolvedAssetAndPath parse = AssetPathResolver.newInstance()
.resolve(url, APILocator.systemUser());
assertEquals(host.getHostname(), parse.host());
assertEquals("/foo/bar/qwe rty.png", parse.path());
assertEquals("qwe rty.png", parse.asset());
}

/**
* Given scenario: host followed by path with an asset name with spaces that has not been
* encoded.
* <p>
* Expected: an exception is thrown due to the space character in the asset name, and the cause
* of the exception is URISyntaxException.
*/
@Test
public void Test_Parse_Path_With_Asset_Without_Encoding() {

final String url = String.format("http://%s/foo/bar/qwe rty.png", host.getHostname());

try {
AssetPathResolver.newInstance().resolve(url, APILocator.systemUser());
Assert.fail("Should have thrown an URISyntaxException");
} catch (Exception e) {
Assert.assertTrue(e.getCause() instanceof URISyntaxException);
}
}

/**
* Given scenario: host followed by path with an asset name that has a folder with spaces that
* has been encoded.
* <p>
* Expected: the URL is properly resolved, the host, path, and asset are correctly extracted.
* the folder with spaces is properly decoded to its original form. No exceptions are expected
* to be thrown.
*
* @throws DotDataException If there is an error resolving the asset path
* @throws DotSecurityException If the user does not have the necessary permissions
*/
@Test
public void Test_Parse_Path_With_Encoded_Folder()
throws DotDataException, DotSecurityException {

String url = String.format("http://%s/foo/test withSpace/qwerty.png", host.getHostname());
url = url.replaceAll(" ", URLEncoder.encode(" ", StandardCharsets.UTF_8));

final ResolvedAssetAndPath parse = AssetPathResolver.newInstance()
.resolve(url, APILocator.systemUser());
assertEquals(host.getHostname(), parse.host());
assertEquals("/foo/test withSpace/qwerty.png", parse.path());
assertEquals("qwerty.png", parse.asset());
}

/**
* Given scenario: host followed by path with an asset name that has a folder with spaces that
* has not been encoded.
* <p>
* Expected: the URL is not properly resolved and an URISyntaxException is thrown.
* <p>
* This method tests the case where the URL contains a path with spaces that has not been
* encoded. It ensures that if the path contains spaces without proper encoding, an
* URISyntaxException is thrown.
*
* @throws URISyntaxException if the URL contains an improperly encoded path with spaces
*/
@Test
public void Test_Parse_Path_Without_Encoding() {

String url = String.format("http://%s/foo/test withSpace/qwerty.png", host.getHostname());

try {
AssetPathResolver.newInstance().resolve(url, APILocator.systemUser());
Assert.fail("Should have thrown an URISyntaxException");
} catch (Exception e) {
Assert.assertTrue(e.getCause() instanceof URISyntaxException);
}
}

/**
* Given scenario: host followed by path with an asset name with spaces that also has a folder
* with spaces that has been properly encoded.
* <p>
* Expected: the URL is properly resolved, the host, path, and asset are correctly extracted.
* the folder and asset with spaces is properly decoded to its original form. No exceptions are
* expected to be thrown.
* <p>
* This method tests the case where the URL contains a path with spaces that has been properly
* encoded. It ensures that the path is correctly resolved and the parsed asset and path match
* the input URL.
*
* @throws DotDataException if there is an error in the asset data
* @throws DotSecurityException if there is a security exception
*/
@Test
public void Test_Parse_Path_With_Encoded_Folder_And_Asset()
throws DotDataException, DotSecurityException {

String url = String.format("http://%s/foo/test withSpace/qwe rty.png", host.getHostname());
url = url.replaceAll(" ", URLEncoder.encode(" ", StandardCharsets.UTF_8));

final ResolvedAssetAndPath parse = AssetPathResolver.newInstance()
.resolve(url, APILocator.systemUser());
assertEquals(host.getHostname(), parse.host());
assertEquals("/foo/test withSpace/qwe rty.png", parse.path());
assertEquals("qwe rty.png", parse.asset());
}

/**
* Given scenario: host followed by path and resource followed by extension
* Expected: host is resolved and so it is path and resource
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import io.vavr.control.Try;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URLDecoder;
import java.nio.charset.StandardCharsets;
import java.util.Optional;

/**
Expand Down Expand Up @@ -79,9 +81,10 @@ public ResolvedAssetAndPath resolve(final String url, final User user, final boo
if (null == path) {
throw new IllegalArgumentException(String.format("Unable to determine path: [%s].", url));
}
final var decodedPath = URLDecoder.decode(path, StandardCharsets.UTF_8);

//This line determines if the path is a folder
final Optional<Folder> folder = resolveExistingFolder(path, host, user);
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);
Expand All @@ -93,7 +96,7 @@ public ResolvedAssetAndPath resolve(final String url, final User user, final boo
.resolvedHost(host)
.host(host.getHostname())
.resolvedFolder(folderAsset.folder())
.path(path)
.path(decodedPath)
.asset(folderAsset.asset())
.build();
}
Expand All @@ -106,7 +109,7 @@ public ResolvedAssetAndPath resolve(final String url, final User user, final boo
builder.resolvedHost(host)
.host(host.getHostname())
.resolvedFolder(folder.get())
.path(path);
.path(decodedPath);

asset.ifPresent(builder::asset);
return builder.build();
Expand Down Expand Up @@ -166,22 +169,26 @@ Optional<Folder> resolveExistingFolder(final String rawPath, final Host host, fi
*/
Optional<FolderAndAsset> resolveAssetAndFolder(final String rawPath, 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(rawPath.matches(startsWithForwardSlash)){
final String[] split = rawPath.split(FORWARD_SLASH, 2);
if (decodedRawPath.matches(startsWithForwardSlash)) {
final String[] split = decodedRawPath.split(FORWARD_SLASH, 2);
return Optional.of(
FolderAndAsset.builder().folder(folderAPI.findSystemFolder()).asset(split[1]).build()
);
}
//if we're not looking at the root folder then we need to extract the parent folder and the asset name
final int index = rawPath.lastIndexOf(FORWARD_SLASH);
final int index = decodedRawPath.lastIndexOf(FORWARD_SLASH);
if(index == -1){
return Optional.empty();
}

final String parentFolderPath = Try.of(()-> rawPath.substring(0, index)).getOrNull();
final String assetName = Try.of(()->rawPath.substring(index + 1)).getOrNull();
final String parentFolderPath = Try.of(() -> decodedRawPath.substring(0, index))
.getOrNull();
final String assetName = Try.of(() -> decodedRawPath.substring(index + 1)).getOrNull();
final String folderPath = parentFolderPath + FORWARD_SLASH;
final Folder parentFolder = folderAPI.findFolderByPath(folderPath, host, user, false);

Expand Down Expand Up @@ -210,7 +217,10 @@ Optional<FolderAndAsset> resolveAssetAndFolder(final String rawPath, final Host
* @return
*/
Optional<String> asset(final Folder resolvedFolder, final String rawResource) {
String resource = rawResource.toLowerCase();

final var decodedRawResource = URLDecoder.decode(rawResource, StandardCharsets.UTF_8);

String resource = decodedRawResource.toLowerCase();
final int index = resource.lastIndexOf(FORWARD_SLASH);
if (index == 0) {
return Optional.empty();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.dotcms.common;

import static com.dotcms.common.LocationUtils.encodePath;

import com.dotcms.model.config.Workspace;
import com.google.common.base.Strings;
import java.io.File;
Expand Down Expand Up @@ -82,18 +84,18 @@ public static String buildRemoteAssetURL(final String site, final String folderP
}

// Build the folder path based on the input parameters
final String remoteFolderPath;
String remoteFolderPath;
if (!emptyFolderPath) {
remoteFolderPath = String.format("//%s/%s", site, cleanedFolderPath);
} else {
remoteFolderPath = String.format("//%s/", site);
}

if (assetName != null && !assetName.isEmpty()) {
return String.format("%s/%s", remoteFolderPath, assetName);
remoteFolderPath = String.format("%s/%s", remoteFolderPath, assetName);
}

return remoteFolderPath;
return encodePath(remoteFolderPath);
}

/**
Expand All @@ -114,7 +116,7 @@ public static RemotePathStructure parseRemotePath(String remotePathToParse) {

final URI uri;
try {
uri = new URI(remotePathToParse);
uri = new URI(encodePath(remotePathToParse));
} catch (URISyntaxException e) {
throw new IllegalArgumentException(e.getMessage(), e);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package com.dotcms.common;

import com.dotcms.model.config.Workspace;

import java.net.URI;
import java.net.URISyntaxException;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.nio.file.Paths;

Expand All @@ -23,7 +24,7 @@ public static boolean isFolderURL(final String url) {

final URI uri;
try {
uri = new URI(url);
uri = new URI(encodePath(url));
} catch (URISyntaxException e) {
throw new IllegalArgumentException(e.getMessage(), e);
}
Expand Down Expand Up @@ -69,4 +70,15 @@ public static Path localPathFromAssetData(final String workspace, final String s
siteName, folderPath, assetName);
}

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

}
Loading

0 comments on commit 09f9e1e

Please sign in to comment.