addressed copilot agent feedback

Author: bachuv

azure-blob-payloads/src/main/java/com/microsoft/durabletask/azureblobpayloads/BlobPayloadStore.java

@@ -138,6 +138,12 @@ public boolean isKnownPayloadToken(String value) {
         return value.startsWith(TOKEN_PREFIX);
     }
 
+    /**
+     * Ensures the blob container exists, creating it if necessary.
+     * The check-then-act on {@code containerEnsured} is intentionally non-atomic:
+     * concurrent callers may race through to {@code create()}, but the 409 Conflict
+     * handler makes this benign.
+     */
     private void ensureContainerExists() {
         if (this.containerEnsured) {
             return;
@@ -194,13 +200,25 @@ private static byte[] gzipCompress(byte[] data) {
         }
     }
 
+    /**
+     * Maximum decompressed size (20 MiB) to guard against decompression bombs.
+     * This is 2x the default max externalized payload size of 10 MiB.
+     */
+    private static final int MAX_DECOMPRESSED_BYTES = 20 * 1024 * 1024;
+
     private static byte[] gzipDecompress(byte[] compressed) {
         try {
             ByteArrayOutputStream baos = new ByteArrayOutputStream();
             try (GZIPInputStream gzipIn = new GZIPInputStream(new ByteArrayInputStream(compressed))) {
                 byte[] buffer = new byte[8192];
                 int len;
+                int totalRead = 0;
                 while ((len = gzipIn.read(buffer)) != -1) {
+                    totalRead += len;
+                    if (totalRead > MAX_DECOMPRESSED_BYTES) {
+                        throw new IOException(
+                            "Decompressed payload exceeds safety limit of " + MAX_DECOMPRESSED_BYTES + " bytes");
+                    }
                     baos.write(buffer, 0, len);
                 }
             }

azure-blob-payloads/src/main/java/com/microsoft/durabletask/azureblobpayloads/BlobPayloadStoreOptions.java

@@ -138,6 +138,7 @@ public static final class Builder {
         /**
          * Sets the Azure Storage connection string. Mutually exclusive with
          * {@link #setBlobServiceEndpoint(String)} and {@link #setBlobServiceClient(BlobServiceClient)}.
+         * Setting this clears any previously set endpoint or pre-configured client.
          *
          * @param connectionString the connection string
          * @return this builder
@@ -147,12 +148,16 @@ public Builder setConnectionString(String connectionString) {
                 throw new IllegalArgumentException("connectionString must not be null or empty");
             }
             this.connectionString = connectionString;
+            this.blobServiceEndpoint = null;
+            this.credential = null;
+            this.blobServiceClient = null;
             return this;
         }
 
         /**
          * Sets the Azure Blob Storage service endpoint. Use with {@link #setCredential(TokenCredential)}.
          * Mutually exclusive with {@link #setConnectionString(String)} and {@link #setBlobServiceClient(BlobServiceClient)}.
+         * Setting this clears any previously set connection string or pre-configured client.
          *
          * @param blobServiceEndpoint the blob service endpoint URL
          * @return this builder
@@ -162,6 +167,8 @@ public Builder setBlobServiceEndpoint(String blobServiceEndpoint) {
                 throw new IllegalArgumentException("blobServiceEndpoint must not be null or empty");
             }
             this.blobServiceEndpoint = blobServiceEndpoint;
+            this.connectionString = null;
+            this.blobServiceClient = null;
             return this;
         }
 
@@ -183,6 +190,7 @@ public Builder setCredential(TokenCredential credential) {
         /**
          * Sets a pre-configured BlobServiceClient. Mutually exclusive with
          * {@link #setConnectionString(String)} and {@link #setBlobServiceEndpoint(String)}.
+         * Setting this clears any previously set connection string or endpoint.
          *
          * @param blobServiceClient the pre-configured client
          * @return this builder
@@ -192,6 +200,9 @@ public Builder setBlobServiceClient(BlobServiceClient blobServiceClient) {
                 throw new IllegalArgumentException("blobServiceClient must not be null");
             }
             this.blobServiceClient = blobServiceClient;
+            this.connectionString = null;
+            this.blobServiceEndpoint = null;
+            this.credential = null;
             return this;
         }
 

client/src/main/java/com/microsoft/durabletask/LargePayloadOptions.java

@@ -24,12 +24,12 @@ public final class LargePayloadOptions {
     /**
      * Default externalization threshold in bytes (900,000 bytes, matching .NET SDK).
      */
-    static final int DEFAULT_THRESHOLD_BYTES = 900_000;
+    public static final int DEFAULT_THRESHOLD_BYTES = 900_000;
 
     /**
      * Default maximum externalized payload size in bytes (10 MiB, matching .NET SDK).
      */
-    static final int DEFAULT_MAX_EXTERNALIZED_PAYLOAD_BYTES = 10 * 1024 * 1024;
+    public static final int DEFAULT_MAX_EXTERNALIZED_PAYLOAD_BYTES = 10 * 1024 * 1024;
 
     private final int thresholdBytes;
     private final int maxExternalizedPayloadBytes;
@@ -40,8 +40,8 @@ private LargePayloadOptions(Builder builder) {
     }
 
     /**
-     * Gets the size threshold in bytes above which payloads will be externalized.
-     * Payloads at or below this size are sent inline. The comparison uses UTF-8 byte length.
+     * Gets the size threshold in bytes at or above which payloads will be externalized.
+     * Payloads below this size are sent inline. The comparison uses UTF-8 byte length.
      *
      * @return the externalization threshold in bytes
      */

client/src/main/java/com/microsoft/durabletask/PayloadHelper.java

@@ -2,8 +2,6 @@
 // Licensed under the MIT License.
 package com.microsoft.durabletask;
 
-import java.nio.charset.StandardCharsets;
-
 /**
  * Internal utility for externalizing and resolving payloads using a {@link PayloadStore}.
  * <p>
@@ -40,24 +38,24 @@ final class PayloadHelper {
      *
      * @param value the payload string to potentially externalize
      * @return the original value if below threshold, or an opaque token if externalized
-     * @throws IllegalArgumentException if the payload exceeds the maximum externalized payload size
+     * @throws PayloadTooLargeException if the payload exceeds the maximum externalized payload size
      */
     String maybeExternalize(String value) {
         // (1) null/empty guard
         if (value == null || value.isEmpty()) {
             return value;
         }
 
-        // Fast path: if char count is below threshold, byte count is too
-        // (each Java char encodes to 1-3 UTF-8 bytes, so length() <= UTF-8 byte length)
-        if (value.length() <= this.options.getThresholdBytes()) {
+        // Fast path: each Java char contributes at least 1 UTF-8 byte,
+        // so length() is always <= UTF-8 byte length.
+        if (value.length() < this.options.getThresholdBytes()) {
             return value;
         }
 
-        int byteSize = value.getBytes(StandardCharsets.UTF_8).length;
+        int byteSize = utf8ByteLength(value);
 
-        // (2) below-threshold guard
-        if (byteSize <= this.options.getThresholdBytes()) {
+        // (2) below-threshold guard (strict less-than, matching .NET)
+        if (byteSize < this.options.getThresholdBytes()) {
             return value;
         }
 
@@ -96,4 +94,26 @@ String maybeResolve(String value) {
         }
         return resolved;
     }
+
+    /**
+     * Counts the number of UTF-8 bytes needed to encode the given string,
+     * without allocating a byte array (unlike {@code String.getBytes(UTF_8).length}).
+     */
+    private static int utf8ByteLength(String s) {
+        int count = 0;
+        for (int i = 0; i < s.length(); i++) {
+            char c = s.charAt(i);
+            if (c <= 0x7F) {
+                count++;
+            } else if (c <= 0x7FF) {
+                count += 2;
+            } else if (Character.isHighSurrogate(c)) {
+                count += 4;
+                i++; // skip low surrogate
+            } else {
+                count += 3;
+            }
+        }
+        return count;
+    }
 }

client/src/main/java/com/microsoft/durabletask/PayloadStore.java

@@ -19,6 +19,11 @@
  * Blob Storage, configure
  * <a href="https://learn.microsoft.com/azure/storage/blobs/lifecycle-management-overview">
  * lifecycle management policies</a> to automatically expire old payloads.
+ * <p>
+ * <b>Performance note:</b> All methods on this interface are synchronous. Implementations
+ * that perform network I/O (e.g., Azure Blob Storage) should ensure that latency is
+ * acceptable on the calling thread. The framework calls these methods on the worker's
+ * processing thread.
  *
  * @see LargePayloadOptions
  */

client/src/test/java/com/microsoft/durabletask/PayloadHelperTest.java

@@ -106,7 +106,7 @@ void maybeExternalize_exceedsMaxCap_throwsException() {
 
         // Create a payload larger than 50 bytes
         String hugePayload = "x".repeat(100);
-        assertThrows(IllegalArgumentException.class, () -> helper.maybeExternalize(hugePayload));
+        assertThrows(PayloadTooLargeException.class, () -> helper.maybeExternalize(hugePayload));
         assertEquals(0, store.getUploadCount());
     }