diff --git a/CHANGELOG.md b/CHANGELOG.md index 170d464..799d166 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,19 @@ Five doctests in `typedialog-core` failed to compile against the current API: - `config/cli_loader.rs`: `ignore` annotation with fictional `MyBackendConfig` type; replaced with a concrete inline struct and changed to `no_run` +**Fixed - Unbounded retry recursion in required-field validation** + +`InquireBackend::execute_field_sync` retried invalid required-field input by calling itself +recursively with no depth limit. A user repeatedly entering empty values for a required field +would grow the call stack unboundedly until stack overflow. + +- Wrapped the `match field.field_type` dispatch in a `loop {}` +- Replaced `return self.execute_field_sync(field, previous_results)` with `continue` +- Changed all non-retry exit points from `Ok(...)` to `return Ok(...)` +- `RepeatingGroup` arm retains `return self.execute_repeating_group(...)` — it carries its + own internal loop and returns a terminal `Result` +- No behavior change for the happy path; retry now uses O(1) stack instead of O(n) + **Added - Architecture documentation** - New `docs/architecture.md`: technical positioning (TypeDialog vs Inquire/Ratatui/Axum), diff --git a/crates/typedialog-core/src/backends/cli.rs b/crates/typedialog-core/src/backends/cli.rs index cce5ca3..1a74b41 100644 --- a/crates/typedialog-core/src/backends/cli.rs +++ b/crates/typedialog-core/src/backends/cli.rs @@ -54,153 +54,156 @@ impl InquireBackend { let is_required = field.required.unwrap_or(false); let required_marker = if is_required { " *" } else { " (optional)" }; - match field.field_type { - FieldType::Text => { - let prompt_with_marker = format!("{}{}", field.prompt, required_marker); - let result = prompts::text( - &prompt_with_marker, - field.default.as_deref(), - field.placeholder.as_deref(), - )?; + loop { + match field.field_type { + FieldType::Text => { + let prompt_with_marker = format!("{}{}", field.prompt, required_marker); + let result = prompts::text( + &prompt_with_marker, + field.default.as_deref(), + field.placeholder.as_deref(), + )?; - if is_required && result.is_empty() { - eprintln!("⚠ This field is required. Please enter a value."); - return self.execute_field_sync(field, previous_results); + if is_required && result.is_empty() { + eprintln!("⚠ This field is required. Please enter a value."); + continue; + } + return Ok(serde_json::json!(result)); } - Ok(serde_json::json!(result)) - } - FieldType::Confirm => { - let prompt_with_marker = format!("{}{}", field.prompt, required_marker); - let default_bool = - field - .default - .as_deref() - .and_then(|s| match s.to_lowercase().as_str() { - "true" | "yes" => Some(true), - "false" | "no" => Some(false), - _ => None, - }); - let result = prompts::confirm(&prompt_with_marker, default_bool, None)?; - Ok(serde_json::json!(result)) - } - - FieldType::Password => { - let prompt_with_marker = format!("{}{}", field.prompt, required_marker); - let with_toggle = field.placeholder.as_deref() == Some("toggle"); - let result = prompts::password(&prompt_with_marker, with_toggle)?; - - if is_required && result.is_empty() { - eprintln!("⚠ This field is required. Please enter a value."); - return self.execute_field_sync(field, previous_results); + FieldType::Confirm => { + let prompt_with_marker = format!("{}{}", field.prompt, required_marker); + let default_bool = + field + .default + .as_deref() + .and_then(|s| match s.to_lowercase().as_str() { + "true" | "yes" => Some(true), + "false" | "no" => Some(false), + _ => None, + }); + let result = prompts::confirm(&prompt_with_marker, default_bool, None)?; + return Ok(serde_json::json!(result)); } - Ok(serde_json::json!(result)) - } - FieldType::Select => { - if field.options.is_empty() { - return Err(crate::ErrorWrapper::form_parse_failed( - "Select field requires 'options'", - )); + FieldType::Password => { + let prompt_with_marker = format!("{}{}", field.prompt, required_marker); + let with_toggle = field.placeholder.as_deref() == Some("toggle"); + let result = prompts::password(&prompt_with_marker, with_toggle)?; + + if is_required && result.is_empty() { + eprintln!("⚠ This field is required. Please enter a value."); + continue; + } + return Ok(serde_json::json!(result)); } - let prompt_with_marker = format!("{}{}", field.prompt, required_marker); - let filtered = filter_options_from(field, previous_results); - if filtered.is_empty() { - return Err(crate::ErrorWrapper::form_parse_failed(format!( - "No options available for field '{}'. Check options_from reference.", - field.name - ))); + + FieldType::Select => { + if field.options.is_empty() { + return Err(crate::ErrorWrapper::form_parse_failed( + "Select field requires 'options'", + )); + } + let prompt_with_marker = format!("{}{}", field.prompt, required_marker); + let filtered = filter_options_from(field, previous_results); + if filtered.is_empty() { + return Err(crate::ErrorWrapper::form_parse_failed(format!( + "No options available for field '{}'. Check options_from reference.", + field.name + ))); + } + let options = filtered + .iter() + .map(|opt| opt.as_string()) + .collect::>(); + let result = prompts::select( + &prompt_with_marker, + options, + field.page_size, + field.vim_mode.unwrap_or(false), + )?; + return Ok(serde_json::json!(result)); } - let options = filtered - .iter() - .map(|opt| opt.as_string()) - .collect::>(); - let result = prompts::select( - &prompt_with_marker, - options, - field.page_size, - field.vim_mode.unwrap_or(false), - )?; - Ok(serde_json::json!(result)) - } - FieldType::MultiSelect => { - if field.options.is_empty() { - return Err(crate::ErrorWrapper::form_parse_failed( - "MultiSelect field requires 'options'", - )); + FieldType::MultiSelect => { + if field.options.is_empty() { + return Err(crate::ErrorWrapper::form_parse_failed( + "MultiSelect field requires 'options'", + )); + } + let prompt_with_marker = format!("{}{}", field.prompt, required_marker); + let options = field + .options + .iter() + .map(|opt| opt.as_string()) + .collect::>(); + let results = prompts::multi_select( + &prompt_with_marker, + options, + field.page_size, + field.vim_mode.unwrap_or(false), + )?; + + if is_required && results.is_empty() { + eprintln!("⚠ This field is required. Please select at least one option."); + continue; + } + return Ok(serde_json::json!(results)); } - let prompt_with_marker = format!("{}{}", field.prompt, required_marker); - let options = field - .options - .iter() - .map(|opt| opt.as_string()) - .collect::>(); - let results = prompts::multi_select( - &prompt_with_marker, - options, - field.page_size, - field.vim_mode.unwrap_or(false), - )?; - if is_required && results.is_empty() { - eprintln!("⚠ This field is required. Please select at least one option."); - return self.execute_field_sync(field, previous_results); + FieldType::Editor => { + let prompt_with_marker = format!("{}{}", field.prompt, required_marker); + let result = prompts::editor( + &prompt_with_marker, + field.file_extension.as_deref(), + field.prefix_text.as_deref(), + )?; + + if is_required && result.is_empty() { + eprintln!("⚠ This field is required. Please enter a value."); + continue; + } + return Ok(serde_json::json!(result)); } - Ok(serde_json::json!(results)) - } - FieldType::Editor => { - let prompt_with_marker = format!("{}{}", field.prompt, required_marker); - let result = prompts::editor( - &prompt_with_marker, - field.file_extension.as_deref(), - field.prefix_text.as_deref(), - )?; - - if is_required && result.is_empty() { - eprintln!("⚠ This field is required. Please enter a value."); - return self.execute_field_sync(field, previous_results); + FieldType::Date => { + let prompt_with_marker = format!("{}{}", field.prompt, required_marker); + let week_start = field.week_start.as_deref().unwrap_or("Mon"); + let result = prompts::date( + &prompt_with_marker, + field.default.as_deref(), + field.min_date.as_deref(), + field.max_date.as_deref(), + week_start, + )?; + return Ok(serde_json::json!(result)); } - Ok(serde_json::json!(result)) - } - FieldType::Date => { - let prompt_with_marker = format!("{}{}", field.prompt, required_marker); - let week_start = field.week_start.as_deref().unwrap_or("Mon"); - let result = prompts::date( - &prompt_with_marker, - field.default.as_deref(), - field.min_date.as_deref(), - field.max_date.as_deref(), - week_start, - )?; - Ok(serde_json::json!(result)) - } + FieldType::Custom => { + let prompt_with_marker = format!("{}{}", field.prompt, required_marker); + let type_name = field.custom_type.as_ref().ok_or_else(|| { + crate::ErrorWrapper::form_parse_failed( + "Custom field requires 'custom_type'", + ) + })?; + let result = + prompts::custom(&prompt_with_marker, type_name, field.default.as_deref())?; - FieldType::Custom => { - let prompt_with_marker = format!("{}{}", field.prompt, required_marker); - let type_name = field.custom_type.as_ref().ok_or_else(|| { - crate::ErrorWrapper::form_parse_failed("Custom field requires 'custom_type'") - })?; - let result = - prompts::custom(&prompt_with_marker, type_name, field.default.as_deref())?; - - if is_required && result.is_empty() { - eprintln!("⚠ This field is required. Please enter a value."); - return self.execute_field_sync(field, previous_results); + if is_required && result.is_empty() { + eprintln!("⚠ This field is required. Please enter a value."); + continue; + } + return Ok(serde_json::json!(result)); } - Ok(serde_json::json!(result)) - } - FieldType::RepeatingGroup => { - let fragment_path = field.fragment.as_ref().ok_or_else(|| { - crate::ErrorWrapper::form_parse_failed( - "RepeatingGroup requires 'fragment' field", - ) - })?; - - self.execute_repeating_group(field, fragment_path) + FieldType::RepeatingGroup => { + let fragment_path = field.fragment.as_ref().ok_or_else(|| { + crate::ErrorWrapper::form_parse_failed( + "RepeatingGroup requires 'fragment' field", + ) + })?; + return self.execute_repeating_group(field, fragment_path); + } } } } diff --git a/docs/architecture.md b/docs/architecture.md index 5fdbc19..8106524 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -202,18 +202,6 @@ for new code is `execute_with_backend_two_phase_with_defaults`. ## Known Technical Debt -### Unbounded Retry Recursion - -Required-field validation retries via direct recursion in `InquireBackend::execute_field_sync`: - -```rust -if is_required && result.is_empty() { - return self.execute_field_sync(field, previous_results); // no depth limit -} -``` - -A user who repeatedly submits empty input on a required field grows the call stack indefinitely. - ### `RenderContext` Clone Cost The executor clones `results` into `context` on every field iteration: