added +

Author: smiklosovic

src/java/org/apache/cassandra/service/snapshot/SnapshotOptions.java

@@ -46,10 +46,10 @@ public class SnapshotOptions
     public static final String SKIP_FLUSH = "skipFlush";
     public static final String TTL = "ttl";
 
-    // Conservative subset of the AWS S3 "Safe characters" set: 0-9  a-z  A-Z  -  _  .
-    // See the validation site in validateTag for the full rationale on excluded characters.
+    // Allowed snapshot-name characters:  0-9  a-z  A-Z  -  _  .  +
+    // See the validation site in validateTag for the full rationale (an S3 safe-set subset, plus '+').
     // Hyphen is placed last in the character class, so it stays literal and never becomes a range operator.
-    private static final Pattern SAFE_SNAPSHOT_NAME = Pattern.compile("[a-zA-Z0-9_.-]+");
+    private static final Pattern SAFE_SNAPSHOT_NAME = Pattern.compile("[a-zA-Z0-9_.+-]+");
 
     public final SnapshotType type;
     public final String tag;
@@ -247,10 +247,12 @@ private void validateTag(String tag)
             // Allowed characters are a conservative subset of the AWS S3 "Safe characters" set
             // (https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html#object-key-guidelines):
             //   0-9  a-z  A-Z  -  _  .
+            // plus '+', which is not an S3 "Safe character" but can legitimately appear in system
+            // snapshot names via version build metadata (e.g. an upgrade snapshot "<millis>-upgrade-5.0.4+build-...").
             // The remaining S3-safe characters (! * ' ( )) are intentionally excluded as they are
             // shell-significant and error-prone in paths, and the path separator '/' is excluded too,
             // which is what blocks traversal attempts such as "../../mysnapshot"
-            if (type == SnapshotType.USER && !SAFE_SNAPSHOT_NAME.matcher(resolvedSnapshotName).matches())
+            if (!SAFE_SNAPSHOT_NAME.matcher(resolvedSnapshotName).matches())
             {
                 throw new IllegalArgumentException("Snapshot name contains illegal characters: " + resolvedSnapshotName + ". " +
                                                    "Allowed characters must match the pattern: " + SAFE_SNAPSHOT_NAME.pattern() +

test/unit/org/apache/cassandra/service/snapshot/SnapshotOptionsTest.java

@@ -103,13 +103,12 @@ public void testSnapshotNameValidation()
             .isInstanceOf(IllegalArgumentException.class)
             .hasMessage("Snapshot name '..' is reserved");
 
-            // snapshot name contains "+" which might be present in a Cassandra version
-            // the usage of "+" is forbidden for USER snapshots
+            // "+" is part of the allowed set because it can appear in a Cassandra version
+            // (build metadata, e.g. "7.0.0+abc123"), which ends up in system snapshot names.
+            // The character check applies uniformly to all snapshot types, so "+" is accepted
+            // for system and user snapshots alike.
             validate("this_is_system_snapshot-7.0.0+abc123", SnapshotType.UPGRADE);
-
-            assertThatThrownBy(() -> validate("this_is_snapshot-7.0.0+abc123", USER))
-            .isInstanceOf(IllegalArgumentException.class)
-            .hasMessageContaining("Snapshot name contains illegal characters: this_is_snapshot-7.0.0+abc123");
+            validate("this_is_snapshot-7.0.0+abc123", USER);
         }
 
         try (WithProperties p = new WithProperties().set(CassandraRelevantProperties.SNAPSHOT_NAME_VALIDATION, false))