Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ use crate::error::{path_field, path_index, PathElement, ValidationErrors};
use crate::template::*;
use crate::types::{ModelExtension, ValidationContext};

/// Maximum length of a `let` binding's `<UserIdentifier>` (§3.6.1).
///
/// Flat, so not `EffectiveLimits::max_identifier_len`: that is the §7.1 cap, 64
/// without `FEATURE_BUNDLE_1`, and a 512-character name must be accepted with
/// EXPR alone.
const MAX_LET_IDENTIFIER_LEN: usize = 512;

/// Build a symbol table containing Param/RawParam entries from job parameter definitions.
/// RawParam.* is always STRING for PATH types and LIST_STRING for LIST_PATH types,
/// matching Python behavior where RawParam holds the raw unprocessed value.
Expand Down Expand Up @@ -1385,6 +1392,13 @@ fn validate_let_bindings(
format!("name '{name}' contains invalid characters."),
);
}
// Name omitted from the message; at 513 characters it would dwarf it.
if name.len() > MAX_LET_IDENTIFIER_LEN {
errors.add(
Comment thread
leongdl marked this conversation as resolved.
&b_path,
format!("name exceeds {MAX_LET_IDENTIFIER_LEN} characters."),
);
}
if !names.insert(name.to_string()) {
errors.add(&b_path, format!("duplicate name '{name}'."));
}
Expand Down
59 changes: 59 additions & 0 deletions crates/openjd-model/tests/integration/test_let_bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1122,3 +1122,62 @@ fn script_let_allows_apply_path_mapping() {
}"#,
);
}

// === Identifier length (§3.6.1: <UserIdentifier> max 512 characters) ===
//
// The cap is flat, so the `_expr_only` cases pin it without FEATURE_BUNDLE_1.
// A regression to the §7.1 cap would pass without them.

fn decode_ok_expr_only(s: &str) {
Comment thread
leongdl marked this conversation as resolved.
Outdated
Comment thread
leongdl marked this conversation as resolved.
Outdated
let v = yaml_val(s);
decode_job_template(v, Some(&["EXPR"]), &CallerLimits::default()).expect("Expected success");
}

#[test]
fn test_let_name_512_chars_succeeds() {
let name = "a".repeat(512);
decode_ok(&job_with_step_let(&format!(r#""{name} = 1""#)));
}

#[test]
fn test_let_name_512_chars_succeeds_expr_only() {
let name = "a".repeat(512);
decode_ok_expr_only(&job_with_step_let(&format!(r#""{name} = 1""#)));
}

#[test]
fn test_let_name_513_chars_fails() {
let name = "a".repeat(513);
check_err(
&job_with_step_let(&format!(r#""{name} = 1""#)),
&["steps[0] -> let[0]:\n\tname exceeds 512 characters."],
);
}

#[test]
fn test_let_name_513_chars_fails_expr_only() {
let name = "a".repeat(513);
let v = yaml_val(&job_with_step_let(&format!(r#""{name} = 1""#)));
let err = decode_job_template(v, Some(&["EXPR"]), &CallerLimits::default())
.expect_err("Expected error");
assert!(
err.to_string()
.contains("steps[0] -> let[0]:\n\tname exceeds 512 characters."),
"Got:\n{err}"
);
}

#[test]
fn test_let_name_513_chars_fails_script() {
let name = "a".repeat(513);
check_err(
&job_with_script_let(&format!(r#""{name} = 1""#)),
&["steps[0] -> script -> let[0]:\n\tname exceeds 512 characters."],
);
}

#[test]
fn test_let_name_512_chars_succeeds_script() {
let name = "a".repeat(512);
decode_ok(&job_with_script_let(&format!(r#""{name} = 1""#)));
}
Loading