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/22 20:19:13 UTC

[sling-org-apache-sling-distribution-journal] 01/01: SLING-10528 - Reject large packages

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

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

commit ca4e646416decbae20d33883f277bad46f75f6a8
Author: tmaret <tm...@adobe.com>
AuthorDate: Tue Jun 22 22:18:20 2021 +0200

    SLING-10528 - Reject large packages
---
 .../journal/bookkeeper/BookKeeper.java             | 13 +++++
 .../journal/bookkeeper/BookKeeperTest.java         | 61 ++++++++++++++++++++++
 2 files changed, 74 insertions(+)

diff --git a/src/main/java/org/apache/sling/distribution/journal/bookkeeper/BookKeeper.java b/src/main/java/org/apache/sling/distribution/journal/bookkeeper/BookKeeper.java
index 0d9a696..45f1bdf 100644
--- a/src/main/java/org/apache/sling/distribution/journal/bookkeeper/BookKeeper.java
+++ b/src/main/java/org/apache/sling/distribution/journal/bookkeeper/BookKeeper.java
@@ -85,6 +85,7 @@ public class BookKeeper implements Closeable {
     private static final String SUBSERVICE_IMPORTER = "importer";
     private static final String SUBSERVICE_BOOKKEEPER = "bookkeeper";
     private static final int RETRY_SEND_DELAY = 1000;
+    static final long MAX_PACKAGE_SIZE = 5 * 1024 * 1024; // 5MB
 
     private final Logger log = LoggerFactory.getLogger(this.getClass());
     private final ResourceResolverFactory resolverFactory;
@@ -151,6 +152,7 @@ public class BookKeeper implements Closeable {
         log.debug("Importing distribution package {} at offset={}", pkgMsg, offset);
         try (Timer.Context context = distributionMetricsService.getImportedPackageDuration().time();
                 ResourceResolver importerResolver = getServiceResolver(SUBSERVICE_IMPORTER)) {
+            assessPackage(pkgMsg);
             packageHandler.apply(importerResolver, pkgMsg);
             if (config.isEditable()) {
                 storeStatus(importerResolver, new PackageStatus(PackageStatusMessage.Status.IMPORTED, offset, pkgMsg.getPubAgentName()));
@@ -358,6 +360,17 @@ public class BookKeeper implements Closeable {
         return resolverFactory.getServiceResourceResolver(singletonMap(SUBSERVICE, subService));
     }
 
+    private void assessPackage(PackageMessage pkgMsg) throws DistributionException {
+        long pkgLength = pkgMsg.getPkgLength();
+        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 import package with size greater than %s Byte, actual %s", MAX_PACKAGE_SIZE, pkgLength));
+        }
+    }
+
     static void retryDelay() {
         try {
             Thread.sleep(RETRY_SEND_DELAY);
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..e1a94d7 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,28 @@
  */
 package org.apache.sling.distribution.journal.bookkeeper;
 
+import static java.lang.System.currentTimeMillis;
+import static java.util.Collections.singletonList;
+import static org.apache.sling.distribution.journal.bookkeeper.BookKeeper.MAX_PACKAGE_SIZE;
 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 +56,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 +85,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 +120,37 @@ public class BookKeeperTest {
         }
     }
 
+    @Test
+    public void testPackageImport() throws DistributionException {
+        try {
+            bookKeeper.importPackage(buildPackageMessage(100), 10, currentTimeMillis());
+        } finally {
+            assertThat(bookKeeper.getRetries(PUB_AGENT_NAME), equalTo(0));
+        }
+    }
+
+    @Test(expected = DistributionException.class)
+    public void testPackageTooLarge() throws DistributionException {
+        try {
+            bookKeeper.importPackage(buildPackageMessage(MAX_PACKAGE_SIZE + 1), 10, currentTimeMillis());
+        } finally {
+            assertThat(bookKeeper.getRetries(PUB_AGENT_NAME), equalTo(1));
+        }
+    }
+
+    PackageMessage buildPackageMessage(long pkgLength) {
+        PackageMessage msg = mock(PackageMessage.class);
+        when(msg.getPkgLength())
+                .thenReturn(pkgLength);
+        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;
+    }
+
 }