You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by tm...@apache.org on 2021/06/23 10:11:57 UTC

[sling-org-apache-sling-distribution-journal] branch master updated: SLING-10528 - Reject large packages in the distribution publisher (#75)

This is an automated email from the ASF dual-hosted git repository.

tmaret pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-distribution-journal.git


The following commit(s) were added to refs/heads/master by this push:
     new b2a97bc  SLING-10528 - Reject large packages in the distribution publisher (#75)
b2a97bc is described below

commit b2a97bc136f111e7ff1d6e776251eac520536e03
Author: Timothee Maret <tm...@apache.org>
AuthorDate: Wed Jun 23 12:11:49 2021 +0200

    SLING-10528 - Reject large packages in the distribution publisher (#75)
---
 .../impl/publisher/PackageMessageFactory.java      | 18 +++++-
 .../journal/bookkeeper/BookKeeperTest.java         | 51 +++++++++++++++
 .../impl/publisher/PackageMessageFactoryTest.java  | 72 ++++++++++++++++++++++
 3 files changed, 139 insertions(+), 2 deletions(-)

diff --git a/src/main/java/org/apache/sling/distribution/journal/impl/publisher/PackageMessageFactory.java b/src/main/java/org/apache/sling/distribution/journal/impl/publisher/PackageMessageFactory.java
index 26116b5..71b1dde 100644
--- a/src/main/java/org/apache/sling/distribution/journal/impl/publisher/PackageMessageFactory.java
+++ b/src/main/java/org/apache/sling/distribution/journal/impl/publisher/PackageMessageFactory.java
@@ -18,6 +18,7 @@
  */
 package org.apache.sling.distribution.journal.impl.publisher;
 
+import static java.lang.String.format;
 import static java.util.Objects.requireNonNull;
 import static org.apache.sling.distribution.packaging.DistributionPackageInfo.PROPERTY_REQUEST_DEEP_PATHS;
 
@@ -55,6 +56,8 @@ public class PackageMessageFactory {
 
     private static final Logger LOG = LoggerFactory.getLogger(PackageMessageFactory.class);
 
+    static final long MAX_PACKAGE_SIZE = 5242880; // 5MB
+
     @Reference
     private SlingSettingsService slingSettings;
 
@@ -86,7 +89,7 @@ public class PackageMessageFactory {
             case ADD: return createAdd(packageBuilder, resourceResolver, pubAgentName, request);
             case DELETE: return createDelete(packageBuilder, resourceResolver, request, pubAgentName);
             case TEST: return createTest(packageBuilder, resourceResolver, pubAgentName);
-            default: throw new IllegalArgumentException(String.format("Unsupported request with requestType=%s", request.getRequestType()));
+            default: throw new IllegalArgumentException(format("Unsupported request with requestType=%s", request.getRequestType()));
         }
     }
 
@@ -94,7 +97,7 @@ public class PackageMessageFactory {
     private PackageMessage createAdd(DistributionPackageBuilder packageBuilder, ResourceResolver resourceResolver, String pubAgentName, DistributionRequest request) throws DistributionException {
         final DistributionPackage disPkg = requireNonNull(packageBuilder.createPackage(resourceResolver, request));
         final byte[] pkgBinary = pkgBinary(disPkg);
-        long pkgLength = pkgBinary.length;
+        long pkgLength = assertPkgLength(pkgBinary.length);
         final DistributionPackageInfo pkgInfo = disPkg.getInfo();
         final List<String> paths = Arrays.asList(pkgInfo.getPaths());
         final List<String> deepPaths = Arrays.asList(pkgInfo.get(PROPERTY_REQUEST_DEEP_PATHS, String[].class));
@@ -164,4 +167,15 @@ public class PackageMessageFactory {
         }
     }
 
+    private long assertPkgLength(long pkgLength) throws DistributionException {
+        if (pkgLength > MAX_PACKAGE_SIZE) {
+            /*
+             * To ensure that Apache Oak can import binary-less content packages
+             * in an atomic save operation, we limit their supported size.
+             */
+            throw new DistributionException(format("Can't distribute package with size greater than %s Byte, actual %s", MAX_PACKAGE_SIZE, pkgLength));
+        }
+        return pkgLength;
+    }
+
 }
diff --git a/src/test/java/org/apache/sling/distribution/journal/bookkeeper/BookKeeperTest.java b/src/test/java/org/apache/sling/distribution/journal/bookkeeper/BookKeeperTest.java
index f9341d9..5bf9d6e 100644
--- a/src/test/java/org/apache/sling/distribution/journal/bookkeeper/BookKeeperTest.java
+++ b/src/test/java/org/apache/sling/distribution/journal/bookkeeper/BookKeeperTest.java
@@ -18,16 +18,27 @@
  */
 package org.apache.sling.distribution.journal.bookkeeper;
 
+import static java.lang.System.currentTimeMillis;
+import static java.util.Collections.singletonList;
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
+import java.util.UUID;
 import java.util.function.Consumer;
 
 import org.apache.sling.api.resource.LoginException;
 import org.apache.sling.api.resource.PersistenceException;
 import org.apache.sling.api.resource.ResourceResolverFactory;
+import org.apache.sling.commons.metrics.Counter;
+import org.apache.sling.commons.metrics.Histogram;
+import org.apache.sling.commons.metrics.Meter;
+import org.apache.sling.commons.metrics.Timer;
 import org.apache.sling.distribution.ImportPostProcessor;
+import org.apache.sling.distribution.common.DistributionException;
 import org.apache.sling.distribution.journal.messages.LogMessage;
+import org.apache.sling.distribution.journal.messages.PackageMessage;
 import org.apache.sling.distribution.journal.messages.PackageStatusMessage;
 import org.apache.sling.distribution.journal.shared.DistributionMetricsService;
 import org.apache.sling.distribution.packaging.DistributionPackageBuilder;
@@ -44,6 +55,8 @@ public class BookKeeperTest {
 
     private static final int COMMIT_AFTER_NUM_SKIPPED = 10;
 
+    private static final String PUB_AGENT_NAME = "pubAgent";
+
     private ResourceResolverFactory resolverFactory = new MockResourceResolverFactory();
 
     @Mock
@@ -71,6 +84,20 @@ public class BookKeeperTest {
 
     @Before
     public void before() {
+        when(distributionMetricsService.getFailedPackageImports())
+                .thenReturn(mock(Meter.class));
+        when(distributionMetricsService.getImportedPackageDuration())
+                .thenReturn(mock(Timer.class));
+        when(distributionMetricsService.getImportedPackageSize())
+                .thenReturn(mock(Histogram.class));
+        when(distributionMetricsService.getPackageDistributedDuration())
+                .thenReturn(mock(Timer.class));
+        when(distributionMetricsService.getImportPostProcessRequest())
+                .thenReturn(mock(Counter.class));
+        when(distributionMetricsService.getImportPostProcessDuration())
+                .thenReturn(mock(Timer.class));
+        when(distributionMetricsService.getImportPostProcessSuccess())
+                .thenReturn(mock(Counter.class));
         BookKeeperConfig bkConfig = new BookKeeperConfig("subAgentName", "subSlingId", true, 10, PackageHandling.Extract, "package");
         bookKeeper = new BookKeeper(resolverFactory, distributionMetricsService, packageHandler, eventAdmin, sender, logSender, bkConfig,
             importPostProcessor);
@@ -92,4 +119,28 @@ public class BookKeeperTest {
         }
     }
 
+    @Test
+    public void testPackageImport() throws DistributionException {
+        try {
+            bookKeeper.importPackage(buildPackageMessage(), 10, currentTimeMillis());
+        } finally {
+            assertThat(bookKeeper.getRetries(PUB_AGENT_NAME), equalTo(0));
+        }
+    }
+
+    PackageMessage buildPackageMessage() {
+        PackageMessage msg = mock(PackageMessage.class);
+        when(msg.getPkgLength())
+                .thenReturn(100L);
+        when(msg.getPubAgentName())
+                .thenReturn(PUB_AGENT_NAME);
+        when(msg.getReqType())
+                .thenReturn(PackageMessage.ReqType.ADD);
+        when(msg.getPaths())
+                .thenReturn(singletonList("/content"));
+        when(msg.getPkgId())
+                .thenReturn(UUID.randomUUID().toString());
+        return msg;
+    }
+
 }
diff --git a/src/test/java/org/apache/sling/distribution/journal/impl/publisher/PackageMessageFactoryTest.java b/src/test/java/org/apache/sling/distribution/journal/impl/publisher/PackageMessageFactoryTest.java
new file mode 100644
index 0000000..310ed17
--- /dev/null
+++ b/src/test/java/org/apache/sling/distribution/journal/impl/publisher/PackageMessageFactoryTest.java
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sling.distribution.journal.impl.publisher;
+
+import java.io.ByteArrayInputStream;
+
+import org.apache.sling.api.resource.ResourceResolver;
+import org.apache.sling.api.resource.ResourceResolverFactory;
+import org.apache.sling.distribution.DistributionRequest;
+import org.apache.sling.distribution.SimpleDistributionRequest;
+import org.apache.sling.distribution.common.DistributionException;
+import org.apache.sling.distribution.packaging.DistributionPackage;
+import org.apache.sling.distribution.packaging.DistributionPackageBuilder;
+import org.apache.sling.testing.resourceresolver.MockResourceResolverFactory;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import static org.apache.sling.distribution.DistributionRequestType.ADD;
+import static org.apache.sling.distribution.journal.impl.publisher.PackageMessageFactory.MAX_PACKAGE_SIZE;
+import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.any;
+
+@RunWith(MockitoJUnitRunner.class)
+public class PackageMessageFactoryTest {
+
+    private static final String PUB_AGENT_NAME = "pubAgentName";
+
+    @Mock
+    private DistributionPackageBuilder packageBuilder;
+
+    @Mock
+    private DistributionPackage distributionPackage;
+
+    private final ResourceResolverFactory resolverFactory = new MockResourceResolverFactory();
+
+    private final PackageMessageFactory factory = new PackageMessageFactory();
+
+
+    @Before
+    public void before() throws DistributionException {
+        when(packageBuilder.createPackage(any(ResourceResolver.class), any(DistributionRequest.class)))
+                .thenReturn(distributionPackage);
+    }
+
+    @Test(expected = DistributionException.class)
+    public void testAddPkgLengthTooLarge() throws Exception {
+        when(distributionPackage.createInputStream())
+                .thenReturn(new ByteArrayInputStream(new byte[(int)MAX_PACKAGE_SIZE + 1]));
+        DistributionRequest request = new SimpleDistributionRequest(ADD, "/some/path");
+        factory.create(packageBuilder, resolverFactory.getServiceResourceResolver(null), PUB_AGENT_NAME, request);
+    }
+
+}
\ No newline at end of file