[graphql/rpc] easy: check variables validity in client (MystenLabs#15432

)

## Description 

Checks early that variable names are valid

## Test Plan 

Unit

---
If your changes are not user-facing and not a breaking change, you can
skip the following section. Otherwise, please indicate what changed, and
then add to the Release Notes section as highlighted during the release
process.

### Type of Change (Check all that apply)

- [ ] protocol change
- [ ] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes

crates/sui-graphql-rpc/src/client/mod.rs

@@ -26,6 +26,11 @@ pub enum ClientError {
     },
     #[error("{item_type} at pos {idx} must not be empty")]
     InvalidEmptyItem { item_type: String, idx: usize },
+    #[error(
+        "Invalid variable name: `{var_name}`. Variable names must be non-empty and start with a letter or underscore, and may only contain letters, digits, and underscores."
+    )]
+    InvalidVariableName { var_name: String },
+
     #[error(
         "Conflicting type definitions for variable {var_name}: {var_type_prev} vs {var_type_curr}"
     )]

crates/sui-graphql-rpc/src/client/simple_client.rs

@@ -113,7 +113,11 @@ pub fn resolve_variables(
     let mut var_vals: BTreeMap<String, Value> = BTreeMap::new();
 
     for (idx, GraphqlQueryVariable { name, ty, value }) in vars.iter().enumerate() {
-        // todo: check that name is valid identifier
+        if !is_valid_variable_name(name) {
+            return Err(ClientError::InvalidVariableName {
+                var_name: name.to_owned(),
+            });
+        }
         if name.trim().is_empty() {
             return Err(ClientError::InvalidEmptyItem {
                 item_type: "Variable name".to_owned(),
@@ -148,3 +152,16 @@ pub fn resolve_variables(
 
     Ok((type_defs, var_vals))
 }
+
+pub fn is_valid_variable_name(s: &str) -> bool {
+    let mut cs = s.chars();
+    let Some(fst) = cs.next() else { return false };
+
+    match fst {
+        '_' => if s.len() > 1 {},
+        'a'..='z' | 'A'..='Z' => {}
+        _ => return false,
+    }
+
+    cs.all(|c| matches!(c, '_' | 'a' ..= 'z' | 'A' ..= 'Z' | '0' ..= '9'))
+}

crates/sui-graphql-rpc/tests/e2e_tests.rs

@@ -11,6 +11,7 @@ mod tests {
     use std::sync::Arc;
     use std::time::Duration;
     use sui_graphql_rpc::client::simple_client::GraphqlQueryVariable;
+    use sui_graphql_rpc::client::ClientError;
     use sui_graphql_rpc::config::ConnectionConfig;
     use sui_graphql_rpc::test_infra::cluster::DEFAULT_INTERNAL_DATA_SOURCE_PORT;
     use sui_types::digests::ChainIdentifier;
@@ -246,6 +247,50 @@ mod tests {
             .await;
 
         assert!(res.is_err());
+
+        let bad_variables = vec![
+            GraphqlQueryVariable {
+                name: "framework addr".to_string(),
+                ty: "SuiAddress!".to_string(),
+                value: json!("0x2"),
+            },
+            GraphqlQueryVariable {
+                name: " deepbook_addr".to_string(),
+                ty: "SuiAddress!".to_string(),
+                value: json!("0xdee9"),
+            },
+            GraphqlQueryVariable {
+                name: "4deepbook_addr".to_string(),
+                ty: "SuiAddressP!".to_string(),
+                value: json!("0xdee9"),
+            },
+            GraphqlQueryVariable {
+                name: "".to_string(),
+                ty: "SuiAddress!".to_string(),
+                value: json!("0xdee9"),
+            },
+            GraphqlQueryVariable {
+                name: " ".to_string(),
+                ty: "SuiAddress!".to_string(),
+                value: json!("0xdee9"),
+            },
+        ];
+
+        for var in bad_variables {
+            let res = cluster
+                .graphql_client
+                .execute_to_graphql(query.to_string(), true, vec![var.clone()], vec![])
+                .await;
+
+            assert!(res.is_err());
+            assert!(
+                res.unwrap_err().to_string()
+                    == ClientError::InvalidVariableName {
+                        var_name: var.name.clone()
+                    }
+                    .to_string()
+            );
+        }
     }
 
     #[tokio::test]