This is an automated email from the ASF dual-hosted git repository.
ferdei pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/nifi.git
The following commit(s) were added to refs/heads/main by this push:
new 2a5fe2e809 NIFI-13412 [MiNiFi] Handle absolute paths in Sync/Resource
C2 command
2a5fe2e809 is described below
commit 2a5fe2e809a8021a55f4ef5c54962b84b8d227af
Author: Ferenc Kis <[email protected]>
AuthorDate: Tue Jun 18 11:56:07 2024 +0200
NIFI-13412 [MiNiFi] Handle absolute paths in Sync/Resource C2 command
Signed-off-by: Ferenc Erdei <[email protected]>
This closes #8975.
---
.../syncresource/DefaultSyncResourceStrategy.java | 15 +--
.../DefaultSyncResourceStrategyTest.java | 107 ++++++++++++++++++++-
2 files changed, 112 insertions(+), 10 deletions(-)
diff --git
a/minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-framework-core/src/main/java/org/apache/nifi/minifi/c2/command/syncresource/DefaultSyncResourceStrategy.java
b/minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-framework-core/src/main/java/org/apache/nifi/minifi/c2/command/syncresource/DefaultSyncResourceStrategy.java
index a837436ebd..7c1f497192 100644
---
a/minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-framework-core/src/main/java/org/apache/nifi/minifi/c2/command/syncresource/DefaultSyncResourceStrategy.java
+++
b/minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-framework-core/src/main/java/org/apache/nifi/minifi/c2/command/syncresource/DefaultSyncResourceStrategy.java
@@ -24,6 +24,7 @@ import static java.util.Map.entry;
import static java.util.Optional.empty;
import static java.util.UUID.randomUUID;
import static java.util.function.Predicate.not;
+import static java.util.regex.Pattern.compile;
import static
org.apache.nifi.c2.protocol.api.C2OperationState.OperationState.FULLY_APPLIED;
import static
org.apache.nifi.c2.protocol.api.C2OperationState.OperationState.NOT_APPLIED;
import static
org.apache.nifi.c2.protocol.api.C2OperationState.OperationState.NO_OPERATION;
@@ -39,6 +40,7 @@ import java.util.Optional;
import java.util.Set;
import java.util.function.BiFunction;
import java.util.function.Function;
+import java.util.regex.Pattern;
import org.apache.nifi.c2.client.service.operation.SyncResourceStrategy;
import org.apache.nifi.c2.protocol.api.C2OperationState.OperationState;
import org.apache.nifi.c2.protocol.api.ResourceItem;
@@ -49,6 +51,7 @@ import org.slf4j.LoggerFactory;
public class DefaultSyncResourceStrategy implements SyncResourceStrategy {
private static final Logger LOG =
LoggerFactory.getLogger(DefaultSyncResourceStrategy.class);
+ private static final Pattern ALLOWED_RESOURCE_PATH_PATTERN =
compile("^(?:(?:.[/\\\\])?[^~<>:\\|\\\"\\?\\*\\./\\\\]+(?:(?!(\\.\\.))[^~<>:\\|\\\"\\?\\*])*)?$");
private static final Set<Entry<OperationState, OperationState>>
SUCCESS_RESULT_PAIRS = Set.of(
entry(NO_OPERATION, NO_OPERATION),
@@ -61,8 +64,6 @@ public class DefaultSyncResourceStrategy implements
SyncResourceStrategy {
entry(NOT_APPLIED, NO_OPERATION),
entry(NOT_APPLIED, NOT_APPLIED));
- private static final String CHANGE_TO_PARENT_DIR_PATH_SEGMENT = "..";
-
private final ResourceRepository resourceRepository;
public DefaultSyncResourceStrategy(ResourceRepository resourceRepository) {
@@ -115,11 +116,11 @@ public class DefaultSyncResourceStrategy implements
SyncResourceStrategy {
}
private boolean validate(ResourceItem resourceItem) {
- if (resourceItem.getResourcePath() != null
- && resourceItem.getResourceType() == ASSET
- &&
resourceItem.getResourcePath().contains(CHANGE_TO_PARENT_DIR_PATH_SEGMENT)) {
- LOG.error("Resource path should not contain '..' path segment in
{}", resourceItem);
- return false;
+ if (resourceItem.getResourcePath() != null &&
resourceItem.getResourceType() == ASSET) {
+ if
(!ALLOWED_RESOURCE_PATH_PATTERN.matcher(resourceItem.getResourcePath()).matches())
{
+ LOG.error("Invalid resource path {}",
resourceItem.getResourcePath());
+ return false;
+ }
}
return true;
}
diff --git
a/minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-framework-core/src/test/java/org/apache/nifi/minifi/c2/command/syncresource/DefaultSyncResourceStrategyTest.java
b/minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-framework-core/src/test/java/org/apache/nifi/minifi/c2/command/syncresource/DefaultSyncResourceStrategyTest.java
index c54c828c54..1ba84d747d 100644
---
a/minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-framework-core/src/test/java/org/apache/nifi/minifi/c2/command/syncresource/DefaultSyncResourceStrategyTest.java
+++
b/minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-framework-core/src/test/java/org/apache/nifi/minifi/c2/command/syncresource/DefaultSyncResourceStrategyTest.java
@@ -44,6 +44,7 @@ import java.util.List;
import java.util.Optional;
import java.util.function.BiFunction;
import java.util.function.Function;
+import java.util.stream.Stream;
import org.apache.nifi.c2.protocol.api.C2OperationState.OperationState;
import org.apache.nifi.c2.protocol.api.ResourceItem;
import org.apache.nifi.c2.protocol.api.ResourceType;
@@ -51,6 +52,9 @@ import org.apache.nifi.c2.protocol.api.ResourcesGlobalHash;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
import org.mockito.Mock;
import org.mockito.MockedStatic;
import org.mockito.junit.jupiter.MockitoExtension;
@@ -124,10 +128,37 @@ public class DefaultSyncResourceStrategyTest {
}
}
- @Test
- public void
testAddingNewItemFailureWhenTypeIsAssetAndPathContainsDoubleDots() {
+ @ParameterizedTest
+ @MethodSource("validResourcePaths")
+ public void testAddingNewItemsSuccessWithValidResourcePath(String
validResourcePath) {
List<ResourceItem> c2Items = List.of(
- resourceItem("resource1", "valid_url", "../path", ASSET)
+ resourceItem("resource1", "url1", validResourcePath, ASSET)
+ );
+
when(mockResourceRepository.findAllResourceItems()).thenReturn(List.of());
+
when(mockResourceRepository.saveResourcesGlobalHash(C2_GLOBAL_HASH)).thenReturn(Optional.of(C2_GLOBAL_HASH));
+ c2Items.forEach(resourceItem -> {
+ try {
+ when(mockResourceRepository.addResourceItem(eq(resourceItem),
any())).thenReturn(Optional.of(resourceItem));
+ } catch (Exception e) {
+ }
+ });
+
+ OperationState resultState =
+
testSyncResourceStrategy.synchronizeResourceRepository(C2_GLOBAL_HASH, c2Items,
URL_TO_CONTENT_DOWNLOAD_FUNCTION, PREFIXING_ENRICH_FUNCTION);
+
+ assertEquals(FULLY_APPLIED, resultState);
+ try {
+ verify(mockResourceRepository, never()).deleteResourceItem(any());
+ } catch (Exception e) {
+ }
+ }
+
+
+ @ParameterizedTest
+ @MethodSource("invalidResourcePaths")
+ public void testAddingNewItemFailureWhenTypeIsAssetAndPathIsInvalid(String
invalidResourcePath) {
+ List<ResourceItem> c2Items = List.of(
+ resourceItem("resource1", "valid_url", invalidResourcePath, ASSET)
);
when(mockResourceRepository.findAllResourceItems()).thenReturn(List.of());
@@ -295,6 +326,76 @@ public class DefaultSyncResourceStrategyTest {
}
}
+ private static Stream<Arguments> validResourcePaths() {
+ return Stream.of(
+ null,
+ "",
+ "sub-folder",
+ "sub-folder/",
+ "sub-folder\\",
+ "./sub-folder",
+ "./sub-folder/",
+ ".\\sub-folder",
+ ".\\sub-folder\\",
+ "./sub-folder/sub-sub-folder",
+ "./sub-folder/sub-sub-folder/",
+ ".\\sub-folder\\sub-sub-folder",
+ ".\\sub-folder\\sub-sub-folder\\",
+ "./sub-folder/sub-sub-folder/sub-sub-sub-folder",
+ "./sub-folder/sub-sub-folder/sub-sub-sub-folder/",
+ ".\\sub-folder\\sub-sub-folder\\sub-sub-sub-folder",
+ ".\\sub-folder\\sub-sub-folder\\sub-sub-sub-folder\\"
+ )
+ .map(Arguments::of);
+ }
+
+ private static Stream<Arguments> invalidResourcePaths() {
+ return Stream.of(
+ "~",
+ "~/",
+ "~\\",
+ "../sub-folder",
+ "sub-folder/../..",
+ "/relative-path/../..",
+ "sub-folder/../sub-sub-folder",
+ "/relative-path/../sub-sub-folder",
+ "./sub-folder/../sub-sub-folder",
+ "..\\sub-folder",
+ "sub-folder\\..\\..",
+ "\\relative-path\\..\\..",
+ "sub-folder\\..\\sub-sub-folder",
+ "\\relative-path\\..\\sub-sub-folder",
+ ".\\sub-folder\\..\\sub-sub-folder",
+ "sub-folder/..",
+ "./sub-folder/..",
+ "sub-folder\\..",
+ ".\\sub-folder\\..",
+ "sub-folder/../sub-sub-folder",
+ "./sub-folder/../sub-sub-folder",
+ "sub-folder\\..\\sub-sub-folder",
+ ".\\sub-folder\\..\\sub-sub-folder",
+ "invalid-char-in-path-<",
+ "invalid-char-in-path->",
+ "invalid-char-in-path-:",
+ "invalid-char-in-path-|",
+ "invalid-char-in-path-?",
+ "invalid-char-in-path-*",
+ "invalid-char-in-path-~",
+ "sub-folder/invalid-char-in-path-~",
+ "/absolute-path",
+ "/absolute-path/..",
+ "/absolute-path/invalid-char-in-path-~",
+ "\\absolute-path",
+ "\\absolute-path\\..",
+ "\\absolute-path\\invalid-char-in-path-~",
+ "C:\\",
+ "C:\\path",
+ "C:/",
+ "C:/path"
+ )
+ .map(Arguments::of);
+ }
+
private static ResourcesGlobalHash resourcesGlobalHash(String digest) {
ResourcesGlobalHash resourcesGlobalHash = new ResourcesGlobalHash();
resourcesGlobalHash.setDigest(digest);
