Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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,15 @@ fn validate_let_bindings(
format!("name '{name}' contains invalid characters."),
);
}
// Characters, not bytes: §3.6.1 caps characters, and the charset check
// above does not `continue`, so a multi-byte name would otherwise draw a
// spurious length error alongside the real one.
if name.chars().count() > MAX_LET_IDENTIFIER_LEN {
errors.add(
Comment thread
leongdl marked this conversation as resolved.
&b_path,
format!("name '{name}' exceeds {MAX_LET_IDENTIFIER_LEN} characters."),
Comment thread
leongdl marked this conversation as resolved.
);
}
if !names.insert(name.to_string()) {
errors.add(&b_path, format!("duplicate name '{name}'."));
}
Expand Down
93 changes: 93 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,96 @@ fn script_let_allows_apply_path_mapping() {
}"#,
);
}

// === Identifier length (§3.6.1: <UserIdentifier> max 512 characters) ===
//
// Effective extensions are the intersection of the template's `extensions` list
// with the caller allowlist, so the FB1 cases need it declared in the body: the
// other helpers here emit `["EXPR"]` only.

fn job_with_step_let_fb1(let_bindings: &str) -> String {
format!(
r#"{{
"specificationVersion": "jobtemplate-2023-09",
"name": "Test",
"extensions": ["EXPR", "FEATURE_BUNDLE_1"],
"steps": [{{
"name": "S",
"let": [{let_bindings}],
"script": {{"actions": {{"onRun": {{"command": "foo"}}}}}}
}}]
}}"#
)
}

// FB1 off: the cap is 512, not the 64 that `max_identifier_len` would give.
#[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_513_chars_fails() {
let name = "a".repeat(513);
check_err(
&job_with_step_let(&format!(r#""{name} = 1""#)),
&[&format!(
"steps[0] -> let[0]:\n\tname '{name}' exceeds 512 characters."
)],
);
}

// FB1 on: still 512, so the extension does not raise this cap either.
#[test]
fn test_let_name_512_chars_succeeds_with_fb1() {
let name = "a".repeat(512);
decode_ok(&job_with_step_let_fb1(&format!(r#""{name} = 1""#)));
}

#[test]
fn test_let_name_513_chars_fails_with_fb1() {
let name = "a".repeat(513);
check_err(
&job_with_step_let_fb1(&format!(r#""{name} = 1""#)),
&[&format!(
"steps[0] -> let[0]:\n\tname '{name}' 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""#)));
}

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

// Characters, not bytes: 200 multi-byte characters are 600 bytes but well under
// the cap, so only the charset error should fire.
#[test]
fn test_let_name_multibyte_under_cap_reports_only_charset() {
Comment thread
leongdl marked this conversation as resolved.
let name = "の".repeat(200);
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");
let msg = err.to_string();
assert!(
msg.contains("contains invalid characters."),
"expected the charset error, got:\n{msg}"
);
assert!(
!msg.contains("exceeds 512 characters."),
"600 bytes is only 200 characters; the length error must not fire:\n{msg}"
);
}
Loading