diff --git a/runners/api_caller/src/lib.rs b/runners/api_caller/src/lib.rs index aa7cf6e..09881cf 100644 --- a/runners/api_caller/src/lib.rs +++ b/runners/api_caller/src/lib.rs @@ -1,11 +1,3 @@ -#![allow( - clippy::as_conversions, - clippy::cast_possible_truncation, - reason = "pagination limit/offset casts between i32/usize/u64 are unaudited; \ - tracked as a dedicated numeric-safety follow-up to issue #1, not \ - rushed into this lint-hygiene pass" -)] - //! A [`DataConnectionRunner`] adapter that resolves an operation's request //! (method, endpoint, params, auth) and executes it over HTTP, handling //! pagination across multiple requests when configured. @@ -50,6 +42,38 @@ where .collect() } +/// Resolves the `options.limit` pagination cap from JSON to an `i32`, +/// falling back to [`constants::DEFAULT_LIMIT`] when absent, non-numeric, or +/// out of `i32`'s range (rather than silently wrapping to an unrelated +/// value). +fn resolve_total_limit(options: &serde_json::Value) -> i32 { + options + .get("limit") + .and_then(|value| match value { + #[allow( + clippy::cast_possible_truncation, + reason = "float-to-int `as` casts saturate rather than wrap (defined \ + behavior since Rust 1.45): an out-of-range or NaN limit clamps \ + to i32::MAX/i32::MIN/0, and a fractional limit truncates toward \ + zero — both are the intended behavior for a pagination limit" + )] + serde_json::Value::Number(n) if n.is_f64() => n.as_f64().map(|n| n as i32), + serde_json::Value::Number(n) if n.is_i64() => { + n.as_i64().and_then(|n| i32::try_from(n).ok()) + } + serde_json::Value::Number(n) if n.is_u64() => { + n.as_u64().and_then(|n| i32::try_from(n).ok()) + } + serde_json::Value::Null + | serde_json::Value::Bool(_) + | serde_json::Value::Number(_) + | serde_json::Value::String(_) + | serde_json::Value::Array(_) + | serde_json::Value::Object(_) => None, + }) + .unwrap_or(constants::DEFAULT_LIMIT) +} + /// Extracts the paginated results from a raw response, by resolving the /// configured pagination strategy's `resultsPath` (stripped of its /// `$response.body#` runtime-expression prefix) as a JSON pointer into @@ -779,21 +803,7 @@ impl APICaller { .get(operation_name) .ok_or_else(|| error::APICaller::OperationNotFound(operation_name.into()))?; - let total_limit = options.get("limit"); - - let total_limit: i32 = total_limit - .and_then(|value| match value { - serde_json::Value::Number(n) if n.is_f64() => n.as_f64().map(|n| n as i32), - serde_json::Value::Number(n) if n.is_i64() => n.as_i64().map(|n| n as i32), - serde_json::Value::Number(n) if n.is_u64() => n.as_u64().map(|n| n as i32), - serde_json::Value::Null - | serde_json::Value::Bool(_) - | serde_json::Value::Number(_) - | serde_json::Value::String(_) - | serde_json::Value::Array(_) - | serde_json::Value::Object(_) => None, - }) - .unwrap_or(constants::DEFAULT_LIMIT); + let total_limit: i32 = resolve_total_limit(options); let mut total: i32 = 0; let mut current_page: i32 = 0; @@ -950,21 +960,7 @@ impl AsyncAPICaller { .get(operation_name) .ok_or_else(|| error::APICaller::OperationNotFound(operation_name.into()))?; - let total_limit = options.get("limit"); - - let total_limit: i32 = total_limit - .and_then(|value| match value { - serde_json::Value::Number(n) if n.is_f64() => n.as_f64().map(|n| n as i32), - serde_json::Value::Number(n) if n.is_i64() => n.as_i64().map(|n| n as i32), - serde_json::Value::Number(n) if n.is_u64() => n.as_u64().map(|n| n as i32), - serde_json::Value::Null - | serde_json::Value::Bool(_) - | serde_json::Value::Number(_) - | serde_json::Value::String(_) - | serde_json::Value::Array(_) - | serde_json::Value::Object(_) => None, - }) - .unwrap_or(constants::DEFAULT_LIMIT); + let total_limit: i32 = resolve_total_limit(options); let mut total: i32 = 0; let mut current_page: i32 = 0; @@ -1292,4 +1288,53 @@ mod tests { "expected an unrecognized auth type to error instead of silently skipping auth, got {result:?}" ); } + + #[test] + fn resolve_total_limit_passes_through_in_range_numbers() { + assert_eq!(resolve_total_limit(&serde_json::json!({ "limit": 42 })), 42); + assert_eq!( + resolve_total_limit(&serde_json::json!({ "limit": 42.9 })), + 42 + ); + assert_eq!( + resolve_total_limit(&serde_json::json!({ "limit": 1_000_000_000_u64 })), + 1_000_000_000 + ); + } + + #[test] + fn resolve_total_limit_falls_back_to_default_when_absent_or_non_numeric() { + assert_eq!( + resolve_total_limit(&serde_json::json!({})), + constants::DEFAULT_LIMIT + ); + assert_eq!( + resolve_total_limit(&serde_json::json!({ "limit": "not a number" })), + constants::DEFAULT_LIMIT + ); + } + + #[test] + fn resolve_total_limit_falls_back_to_default_instead_of_wrapping_an_out_of_range_i64() { + // i64::from(i32::MAX) + 1 wraps to i32::MIN under `as i32`, which + // would corrupt the pagination limit into a large negative number + // instead of safely falling back to the default. + let oversized = i64::from(i32::MAX) + 1; + assert_eq!( + resolve_total_limit(&serde_json::json!({ "limit": oversized })), + constants::DEFAULT_LIMIT + ); + } + + #[test] + fn resolve_total_limit_falls_back_to_default_instead_of_wrapping_an_out_of_range_u64() { + // 2^32 + 1 wraps to 1 under `as i32`, which would be silently + // misread as a valid (tiny) limit instead of falling back to the + // configured default. + let oversized = u64::from(u32::MAX) + 2; + assert_eq!( + resolve_total_limit(&serde_json::json!({ "limit": oversized })), + constants::DEFAULT_LIMIT + ); + } } diff --git a/usecases/execution_engine/src/lib.rs b/usecases/execution_engine/src/lib.rs index a5ec677..2cc5a4b 100644 --- a/usecases/execution_engine/src/lib.rs +++ b/usecases/execution_engine/src/lib.rs @@ -182,10 +182,6 @@ impl Engine { options: Value, context: &EngineInputContext, ) -> error::Result { - // SimpleCode -> CodeRunner - // ApiWrapper -> FilteredRunner - // ScriptedAction -> ScriptRunner - if identifier == "$input" { if let Some(input_handler) = &self.input_handler { return input_handler.run(params, context); @@ -209,127 +205,220 @@ impl Engine { let manifest = service.manifest.v2(); let result = match &manifest.value { - Some(service_manifest_latest::Value::Swagger(swagger)) => { - if let Some(connector) = &self.connector { - let api = &service.commonApi; - let creds = credentials.as_ref(); - - let bundle = DataConnectorBundle { - manifest: swagger, - api, - creds, - }; - connector.run( - service_name, - operation_name, - &bundle, - params, - options, - context, - ) - } else { - Err(error::ExecutionEngine::NotFound( - "Data connector runner".into(), - )) - } - } - Some(service_manifest_latest::Value::Action(action)) => { - let operation = action - .operations - .iter() - .find(|item| item.id == *operation_name); - if let Some(operation) = operation { - let operation = operation.function(); - - let path = format!("{}/{}", action.source, operation.js()); - - let source = service - .resources - .iter() - .find(|item| item.relativePath == path) - .ok_or(error::ExecutionEngine::NotFound(format!( - "Source file for {service_name}.{operation_name}" - )))?; - - if let Some(code_runner) = self.code_runners.get(&operation.lang) { - self.log(identifier, "ACTION", "STARTED")?; - let result = code_runner.run( - service_name, - operation_name, - &source.content, - params, - context, - )?; - self.log(identifier, "ACTION", "COMPLETED")?; - - Ok(result) - } else { - Err(error::ExecutionEngine::NotFound(format!( - "Code Runner for language {} not found", - operation.lang - ))) - } - } else { - Err(error::ExecutionEngine::NotFound(format!( - "Action operation {operation_name}" - ))) - } - } - Some(service_manifest_latest::Value::ApiWrapped(api_wrapped)) => { - if let Some(filtered_runner) = &self.filtered_runner { - self.log(identifier, "API_WRAPPED", "STARTED")?; - let result = filtered_runner.run( - service_name, - operation_name, - api_wrapped, - params, - context, - )?; - self.log(identifier, "API_WRAPPED", "COMPLETED")?; - - Ok(result) - } else { - Err(error::ExecutionEngine::NotFound( - "API Wrapper runner not found".into(), - )) - } - } - Some(service_manifest_latest::Value::SimpleCode(simple_code)) => { - match simple_code.code.language.enum_value() { - Ok(Language::PYTHON) => self.dispatch_code_runner( - identifier, - service_name, - operation_name, - "python", - simple_code.code.codeString(), - params, - context, - ), - Ok(Language::JAVASCRIPT) => self.dispatch_code_runner( - identifier, - service_name, - operation_name, - "js", - simple_code.code.codeString(), - params, - context, - ), - // LUA is deliberately not dispatched to here - see #73: - // `Workflow`-kind manifests (via `WorkflowRunner`) are - // the replacement for Lua `SimpleCode` operations, not - // a second parallel Lua execution path through this - // arm. The `LUA` enum variant itself stays defined - // (harmless, and a smaller footprint than removing a - // wire enum value), it's just unreachable here now. - _ => Err(error::ExecutionEngine::NotFound("Unknown language".into())), - } - } + Some(service_manifest_latest::Value::Swagger(swagger)) => self.dispatch_swagger( + service_name, + operation_name, + service, + swagger, + credentials, + params, + options, + context, + ), + Some(service_manifest_latest::Value::Action(action)) => self.dispatch_action( + identifier, + service_name, + operation_name, + service, + action, + params, + context, + ), + Some(service_manifest_latest::Value::ApiWrapped(api_wrapped)) => self + .dispatch_api_wrapped( + identifier, + service_name, + operation_name, + api_wrapped, + params, + context, + ), + Some(service_manifest_latest::Value::SimpleCode(simple_code)) => self + .dispatch_simple_code( + identifier, + service_name, + operation_name, + simple_code, + params, + context, + ), _ => Err(error::ExecutionEngine::Unimplemented("API Runner".into())), }?; Ok(wrap_result(result, context.raw_response)) } + /// Dispatches a `Swagger`-kind manifest to the registered + /// [`DataConnectionRunner`] - the `Swagger` arm of [`Engine::run`]'s + /// dispatch. + #[allow( + clippy::too_many_arguments, + reason = "each argument is a distinct dispatch input passed through from Engine::run; \ + see dispatch_code_runner's doc comment above for the same reasoning (#16)" + )] + fn dispatch_swagger( + &self, + service_name: &str, + operation_name: &str, + service: &core_entities::service::versioned_service_tree::V1, + swagger: &core_entities::service::SwaggerService, + credentials: Option, + params: Value, + options: Value, + context: &EngineInputContext, + ) -> error::Result { + if let Some(connector) = &self.connector { + let api = &service.commonApi; + let creds = credentials.as_ref(); + + let bundle = DataConnectorBundle { + manifest: swagger, + api, + creds, + }; + connector.run( + service_name, + operation_name, + &bundle, + params, + options, + context, + ) + } else { + Err(error::ExecutionEngine::NotFound( + "Data connector runner".into(), + )) + } + } + + /// Dispatches an `Action`-kind manifest to the registered [`CodeRunner`] + /// for the resolved operation's language - the `Action` arm of + /// [`Engine::run`]'s dispatch. + #[allow( + clippy::too_many_arguments, + reason = "each argument is a distinct dispatch input passed through from Engine::run; \ + see dispatch_code_runner's doc comment above for the same reasoning (#16)" + )] + fn dispatch_action( + &self, + identifier: &str, + service_name: &str, + operation_name: &str, + service: &core_entities::service::versioned_service_tree::V1, + action: &core_entities::service::ActionService, + params: Value, + context: &EngineInputContext, + ) -> error::Result { + let operation = action + .operations + .iter() + .find(|item| item.id == *operation_name); + if let Some(operation) = operation { + let operation = operation.function(); + + let path = format!("{}/{}", action.source, operation.js()); + + let source = service + .resources + .iter() + .find(|item| item.relativePath == path) + .ok_or(error::ExecutionEngine::NotFound(format!( + "Source file for {service_name}.{operation_name}" + )))?; + + if let Some(code_runner) = self.code_runners.get(&operation.lang) { + self.log(identifier, "ACTION", "STARTED")?; + let result = code_runner.run( + service_name, + operation_name, + &source.content, + params, + context, + )?; + self.log(identifier, "ACTION", "COMPLETED")?; + + Ok(result) + } else { + Err(error::ExecutionEngine::NotFound(format!( + "Code Runner for language {} not found", + operation.lang + ))) + } + } else { + Err(error::ExecutionEngine::NotFound(format!( + "Action operation {operation_name}" + ))) + } + } + + /// Dispatches an `ApiWrapped`-kind manifest to the registered + /// [`FilteredRunner`] - the `ApiWrapped` arm of [`Engine::run`]'s + /// dispatch. + fn dispatch_api_wrapped( + &self, + identifier: &str, + service_name: &str, + operation_name: &str, + api_wrapped: &core_entities::service::APIWrappedService, + params: Value, + context: &EngineInputContext, + ) -> error::Result { + if let Some(filtered_runner) = &self.filtered_runner { + self.log(identifier, "API_WRAPPED", "STARTED")?; + let result = + filtered_runner.run(service_name, operation_name, api_wrapped, params, context)?; + self.log(identifier, "API_WRAPPED", "COMPLETED")?; + + Ok(result) + } else { + Err(error::ExecutionEngine::NotFound( + "API Wrapper runner not found".into(), + )) + } + } + + /// Dispatches a `SimpleCode`-kind manifest to the [`CodeRunner`] + /// registered for its `language` - the `SimpleCode` arm of + /// [`Engine::run`]'s dispatch. + fn dispatch_simple_code( + &self, + identifier: &str, + service_name: &str, + operation_name: &str, + simple_code: &core_entities::service::SimpleCodeService, + params: Value, + context: &EngineInputContext, + ) -> error::Result { + match simple_code.code.language.enum_value() { + Ok(Language::PYTHON) => self.dispatch_code_runner( + identifier, + service_name, + operation_name, + "python", + simple_code.code.codeString(), + params, + context, + ), + Ok(Language::JAVASCRIPT) => self.dispatch_code_runner( + identifier, + service_name, + operation_name, + "js", + simple_code.code.codeString(), + params, + context, + ), + // LUA is deliberately not dispatched to here - see #73: + // `Workflow`-kind manifests (via `WorkflowRunner`) are + // the replacement for Lua `SimpleCode` operations, not + // a second parallel Lua execution path through this + // arm. The `LUA` enum variant itself stays defined + // (harmless, and a smaller footprint than removing a + // wire enum value), it's just unreachable here now. + _ => Err(error::ExecutionEngine::NotFound("Unknown language".into())), + } + } + /// Resolves `identifier` against a `Workflow`-kind manifest, returning /// the fully **owned** pieces (`service_name`, `operation_name`, the /// manifest's cloned `WorkflowService`, and the registered diff --git a/usecases/service_loader/src/loaders/openapi/mod.rs b/usecases/service_loader/src/loaders/openapi/mod.rs index e761646..88b6baa 100644 --- a/usecases/service_loader/src/loaders/openapi/mod.rs +++ b/usecases/service_loader/src/loaders/openapi/mod.rs @@ -478,54 +478,21 @@ fn handle_schema( _ => {} } } else { - let result = optional_field::>(source, "oneOf")?; - if let Some(result) = result { - let schema: error::Result> = result - .iter() - .map(|value| { - let mut common_schema = service::Schema::new(); - handle_schema(value, &mut common_schema, root, fetcher, cache, schemas)?; - Ok(common_schema) - }) - .collect(); - let schema = schema?; - + if let Some(schema) = resolve_schema_list(source, "oneOf", root, fetcher, cache, schemas)? { sink.set_oneOf(service::ComposedSchema { schema, ..Default::default() }); } - let result = optional_field::>(source, "anyOf")?; - if let Some(result) = result { - let schema: error::Result> = result - .iter() - .map(|value| { - let mut common_schema = service::Schema::new(); - handle_schema(value, &mut common_schema, root, fetcher, cache, schemas)?; - Ok(common_schema) - }) - .collect(); - let schema = schema?; - + if let Some(schema) = resolve_schema_list(source, "anyOf", root, fetcher, cache, schemas)? { sink.set_anyOf(service::ComposedSchema { schema, ..Default::default() }); } - let result = optional_field::>(source, "allOf")?; - if let Some(result) = result { - let schema: error::Result> = result - .iter() - .map(|value| { - let mut common_schema = service::Schema::new(); - handle_schema(value, &mut common_schema, root, fetcher, cache, schemas)?; - Ok(common_schema) - }) - .collect(); - let schema = schema?; - + if let Some(schema) = resolve_schema_list(source, "allOf", root, fetcher, cache, schemas)? { sink.set_allOf(service::ComposedSchema { schema, ..Default::default() @@ -536,6 +503,33 @@ fn handle_schema( Ok(()) } +/// Resolves `source`'s `field` (`"oneOf"`/`"anyOf"`/`"allOf"`) as a list of +/// schemas, recursively converting each branch via [`handle_schema`]. +/// Returns `None` if `field` is absent - the shared body of +/// [`handle_schema`]'s three composition-field arms. +fn resolve_schema_list( + source: &serde_json::Value, + field: &str, + root: &serde_json::Value, + fetcher: &dyn Fetcher, + cache: &mut HashMap, + schemas: &mut HashMap, +) -> error::Result>> { + let Some(values) = optional_field::>(source, field)? else { + return Ok(None); + }; + + values + .iter() + .map(|value| { + let mut common_schema = service::Schema::new(); + handle_schema(value, &mut common_schema, root, fetcher, cache, schemas)?; + Ok(common_schema) + }) + .collect::>>() + .map(Some) +} + #[cfg(test)] mod test { use core::cell::RefCell;