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:12 UTC

[sling-org-apache-sling-distribution-journal] branch SLING-10528 created (now ca4e646)

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

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


      at ca4e646  SLING-10528 - Reject large packages

This branch includes the following new commits:

     new ca4e646  SLING-10528 - Reject large packages

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


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

Posted by tm...@apache.org.
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;
+    }
+
 }