fix: support JMESPath backtick string literals and improve module upload error (#511)

- Add normalize_backtick_literals() to convert unquoted backtick literals
  to proper JSON format (e.g., `foo` -> `"foo"`)
- The jmespath crate requires valid JSON inside backticks, but the JMESPath
  spec allows elided quotes for string literals
- Queries like [?module_name==`jmespath`] now work correctly

- Improve error message when module upload returns HTTP 405
- Redis Enterprise 8.x doesn't support module upload via REST API
- Now shows helpful message directing users to Admin UI or rladmin CLI

Fixes: JMESPath filter expressions with backtick literals failing
Fixes: Unclear error when module upload not supported

Author: joshrotenberg

Cargo.lock

@@ -86,6 +86,10 @@ impl ModuleHandler {
     }
 
     /// Upload new module (tries v2 first, falls back to v1)
+    ///
+    /// Note: Some Redis Enterprise versions (particularly RE 8.x) do not support
+    /// module upload via the REST API. In those cases, use the Admin UI or
+    /// node-level CLI tools (rladmin) to upload modules.
     pub async fn upload(&self, module_data: Vec<u8>, file_name: &str) -> Result<Value> {
         // Try v2 first (returns action_uid for async tracking)
         match self
@@ -96,9 +100,26 @@ impl ModuleHandler {
             Ok(response) => Ok(response),
             Err(crate::error::RestError::NotFound) => {
                 // v2 endpoint doesn't exist, try v1
-                self.client
+                match self
+                    .client
                     .post_multipart("/v1/modules", module_data, "module", file_name)
                     .await
+                {
+                    Ok(response) => Ok(response),
+                    Err(crate::error::RestError::ApiError { code: 405, .. }) => {
+                        Err(crate::error::RestError::ValidationError(
+                            "Module upload via REST API is not supported in this Redis Enterprise version. \
+                             Use the Admin UI or rladmin CLI to upload modules.".to_string()
+                        ))
+                    }
+                    Err(e) => Err(e),
+                }
+            }
+            Err(crate::error::RestError::ApiError { code: 405, .. }) => {
+                Err(crate::error::RestError::ValidationError(
+                    "Module upload via REST API is not supported in this Redis Enterprise version. \
+                     Use the Admin UI or rladmin CLI to upload modules.".to_string()
+                ))
             }
             Err(e) => Err(e),
         }

crates/redis-enterprise/src/modules.rs

@@ -60,6 +60,7 @@ keyring = { version = "3.6", optional = true, features = ["apple-native", "windo
 # Tower and resilience patterns
 tower = { version = "0.5", features = ["util", "timeout", "buffer", "ready-cache"] }
 tower-resilience = { version = "0.1", features = ["circuitbreaker", "retry", "ratelimiter"] }
+regex = "1.12.2"
 
 [target.'cfg(unix)'.dependencies]
 pager = "0.16"

crates/redisctl/Cargo.toml

@@ -3,6 +3,7 @@
 use anyhow::{Context, Result};
 use comfy_table::Table;
 use jmespath::Runtime;
+use regex::Regex;
 use serde::Serialize;
 use serde_json::Value;
 use std::sync::OnceLock;
@@ -20,11 +21,54 @@ pub fn get_jmespath_runtime() -> &'static Runtime {
     })
 }
 
-/// Compile a JMESPath expression using the extended runtime
+/// Normalize backtick literals in JMESPath expressions.
+///
+/// The JMESPath specification allows "elided quotes" in backtick literals,
+/// meaning `` `foo` `` is equivalent to `` `"foo"` ``. However, the Rust
+/// jmespath crate requires valid JSON inside backticks.
+///
+/// This function converts unquoted string literals like `` `foo` `` to
+/// properly quoted JSON strings like `` `"foo"` ``.
+///
+/// Examples:
+/// - `` `foo` `` -> `` `"foo"` ``
+/// - `` `true` `` -> `` `true` `` (unchanged, valid JSON boolean)
+/// - `` `123` `` -> `` `123` `` (unchanged, valid JSON number)
+/// - `` `"already quoted"` `` -> `` `"already quoted"` `` (unchanged)
+fn normalize_backtick_literals(query: &str) -> String {
+    static BACKTICK_RE: OnceLock<Regex> = OnceLock::new();
+    let re = BACKTICK_RE.get_or_init(|| {
+        // Match backtick-delimited content, handling escaped backticks
+        Regex::new(r"`([^`\\]*(?:\\.[^`\\]*)*)`").unwrap()
+    });
+
+    re.replace_all(query, |caps: &regex::Captures| {
+        let content = &caps[1];
+        let trimmed = content.trim();
+
+        // Check if it's already valid JSON
+        if serde_json::from_str::<Value>(trimmed).is_ok() {
+            // Already valid JSON (number, boolean, null, quoted string, array, object)
+            format!("`{}`", content)
+        } else {
+            // Not valid JSON - treat as unquoted string literal and add quotes
+            // Escape any double quotes in the content
+            let escaped = trimmed.replace('\\', "\\\\").replace('"', "\\\"");
+            format!("`\"{}\"`", escaped)
+        }
+    })
+    .into_owned()
+}
+
+/// Compile a JMESPath expression using the extended runtime.
+///
+/// This function normalizes backtick literals to handle the JMESPath
+/// specification's "elided quotes" feature before compilation.
 pub fn compile_jmespath(
     query: &str,
 ) -> Result<jmespath::Expression<'static>, jmespath::JmespathError> {
-    get_jmespath_runtime().compile(query)
+    let normalized = normalize_backtick_literals(query);
+    get_jmespath_runtime().compile(&normalized)
 }
 
 #[derive(Debug, Clone, Copy, clap::ValueEnum, Default)]
@@ -54,10 +98,11 @@ pub fn print_output<T: Serialize>(
 
     // Apply JMESPath query if provided (using extended runtime with 300+ functions)
     if let Some(query_str) = query {
+        let normalized = normalize_backtick_literals(query_str);
         let runtime = get_jmespath_runtime();
         let expr = runtime
-            .compile(query_str)
-            .context("Invalid JMESPath expression")?;
+            .compile(&normalized)
+            .with_context(|| format!("Invalid JMESPath expression: {}", query_str))?;
         // Convert Value to string then parse as Variable
         let json_str = serde_json::to_string(&json_value)?;
         let data = jmespath::Variable::from_json(&json_str)
@@ -142,3 +187,125 @@ fn format_value(value: &Value) -> String {
         Value::Object(obj) => format!("{{{} fields}}", obj.len()),
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_normalize_backtick_unquoted_string() {
+        // Standard JMESPath backtick literal without quotes
+        assert_eq!(
+            normalize_backtick_literals(r#"[?name==`foo`]"#),
+            r#"[?name==`"foo"`]"#
+        );
+    }
+
+    #[test]
+    fn test_normalize_backtick_already_quoted() {
+        // Already properly quoted - should not double-quote
+        assert_eq!(
+            normalize_backtick_literals(r#"[?name==`"foo"`]"#),
+            r#"[?name==`"foo"`]"#
+        );
+    }
+
+    #[test]
+    fn test_normalize_backtick_number() {
+        // Numbers are valid JSON - should not be quoted
+        assert_eq!(
+            normalize_backtick_literals(r#"[?count==`123`]"#),
+            r#"[?count==`123`]"#
+        );
+    }
+
+    #[test]
+    fn test_normalize_backtick_boolean() {
+        // Booleans are valid JSON - should not be quoted
+        assert_eq!(
+            normalize_backtick_literals(r#"[?enabled==`true`]"#),
+            r#"[?enabled==`true`]"#
+        );
+        assert_eq!(
+            normalize_backtick_literals(r#"[?enabled==`false`]"#),
+            r#"[?enabled==`false`]"#
+        );
+    }
+
+    #[test]
+    fn test_normalize_backtick_null() {
+        // null is valid JSON - should not be quoted
+        assert_eq!(
+            normalize_backtick_literals(r#"[?value==`null`]"#),
+            r#"[?value==`null`]"#
+        );
+    }
+
+    #[test]
+    fn test_normalize_backtick_array() {
+        // Arrays are valid JSON - should not be modified
+        assert_eq!(
+            normalize_backtick_literals(r#"`[1, 2, 3]`"#),
+            r#"`[1, 2, 3]`"#
+        );
+    }
+
+    #[test]
+    fn test_normalize_backtick_object() {
+        // Objects are valid JSON - should not be modified
+        assert_eq!(
+            normalize_backtick_literals(r#"`{"key": "value"}`"#),
+            r#"`{"key": "value"}`"#
+        );
+    }
+
+    #[test]
+    fn test_normalize_multiple_backticks() {
+        // Multiple backtick literals in one expression
+        assert_eq!(
+            normalize_backtick_literals(r#"[?name==`foo` && type==`bar`]"#),
+            r#"[?name==`"foo"` && type==`"bar"`]"#
+        );
+    }
+
+    #[test]
+    fn test_jmespath_backtick_literal_compiles() {
+        // The original failing case should now work
+        let query = r#"[?module_name==`jmespath`]"#;
+        let result = compile_jmespath(query);
+        assert!(
+            result.is_ok(),
+            "Backtick literals should be supported: {:?}",
+            result
+        );
+    }
+
+    #[test]
+    fn test_jmespath_complex_filter() {
+        // Complex filter expression from the bug report
+        let query = r#"[?module_name==`jmespath`].uid | [0]"#;
+        let result = compile_jmespath(query);
+        assert!(
+            result.is_ok(),
+            "Complex filter with backtick should work: {:?}",
+            result
+        );
+    }
+
+    #[test]
+    fn test_jmespath_double_quote_literal() {
+        // Double quotes work as field references, not literals
+        let query = r#"[?module_name=="jmespath"]"#;
+        let result = compile_jmespath(query);
+        // This compiles but semantically compares field to field, not field to literal
+        assert!(result.is_ok());
+    }
+
+    #[test]
+    fn test_jmespath_single_quote_literal() {
+        // Single quotes are raw string literals in JMESPath
+        let query = "[?module_name=='jmespath']";
+        let result = compile_jmespath(query);
+        assert!(result.is_ok());
+    }
+}