-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Description
Is there an existing issue for this?
- I have searched the existing issues
Environment
- Milvus version: master-20260114-eb63a363-amd64
- Deployment mode: standalone
- SDK version: pymilvus 2.7.0rc113
- OS: macOS / Linux
Current Behavior
create_snapshot and drop_snapshot APIs accept whitespace-only strings (e.g., " ", " ") as snapshot_name without validation. This creates snapshots with invalid names that are difficult to manage.
# This should fail but succeeds
client.create_snapshot(collection_name="test_col", snapshot_name=" ") # Single space - OK??
client.create_snapshot(collection_name="test_col", snapshot_name=" ") # Two spaces - OK??Expected Behavior
The server should reject whitespace-only snapshot_name with a clear error message:
ParamError: snapshot_name must be a non-empty string (after trimming whitespace)
Steps To Reproduce
from pymilvus import MilvusClient
client = MilvusClient('http://localhost:19530')
# Create collection
client.create_collection("test_col", dimension=128)
# Create snapshot with whitespace-only name - should fail but succeeds
client.create_snapshot(collection_name="test_col", snapshot_name=" ")
print("Created snapshot with name=' ' (single space)")
# List snapshots - the whitespace snapshot appears
snapshots = client.list_snapshots(collection_name="test_col")
print(f"Snapshots: {snapshots}") # Contains ' '
# Try to create another one with same whitespace name
try:
client.create_snapshot(collection_name="test_col", snapshot_name=" ")
except Exception as e:
print(f"Second creation fails: {e}")
# Error message is confusing: "snapshot name already exists"Root Cause Analysis
Server Side
The server-side validation in internal/proxy/task_snapshot.go doesn't check for whitespace-only names:
CreateSnapshot (lines 97-105):
func (cst *createSnapshotTask) PreExecute(ctx context.Context) error {
// Only validates collection exists, no snapshot_name validation
collectionID, err := globalMetaCache.GetCollectionID(ctx, cst.req.GetDbName(), cst.req.GetCollectionName())
if err != nil {
return err
}
cst.collectionID = collectionID
return nil // No validation for empty/whitespace snapshot_name
}DropSnapshot (lines 183-186):
func (dst *dropSnapshotTask) PreExecute(ctx context.Context) error {
// No additional validation needed for drop snapshot
return nil // Comment explicitly says no validation!
}SDK Side (pymilvus)
The SDK's validate_str() function in pymilvus/client/check.py only checks for empty strings, not whitespace:
def validate_str(var: Any) -> bool:
"""check if a variable is legal non-empty str"""
return var and isinstance(var, str) # " " is truthy, so it passesSuggested Fix
Server Side (Preferred)
Add validation in createSnapshotTask.PreExecute and dropSnapshotTask.PreExecute:
import "strings"
func (cst *createSnapshotTask) PreExecute(ctx context.Context) error {
// Add snapshot_name validation
if strings.TrimSpace(cst.req.GetName()) == "" {
return merr.WrapErrParameterInvalidMsg("snapshot_name must be a non-empty string")
}
collectionID, err := globalMetaCache.GetCollectionID(ctx, cst.req.GetDbName(), cst.req.GetCollectionName())
if err != nil {
return err
}
cst.collectionID = collectionID
return nil
}
func (dst *dropSnapshotTask) PreExecute(ctx context.Context) error {
if strings.TrimSpace(dst.req.GetName()) == "" {
return merr.WrapErrParameterInvalidMsg("snapshot_name must be a non-empty string")
}
return nil
}SDK Side (Additional)
Update validate_str() to strip whitespace:
def validate_str(var: Any) -> bool:
"""check if a variable is legal non-empty str"""
return var and isinstance(var, str) and var.strip()Impact
- Creates "invisible" snapshots that are hard to identify and manage
- Confusing error messages when trying to create duplicate whitespace snapshots
- Potential for orphaned snapshots that can't be easily cleaned up
- Poor user experience and API hygiene
Milvus Log
N/A
Anything else?
This was discovered during Snapshot feature testing. Related test cases:
test_snapshot_create_whitespace_nametest_snapshot_drop_whitespace_name
Note: Empty string "" and None are correctly rejected by pymilvus SDK (PR #3083), but whitespace-only strings pass through both SDK and server validation.
Related to: #44358 (Snapshot feature)