From 1b87696d801a14ad0f71b8abd2dabf35d4168c93 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 28 Jan 2026 19:48:38 +1300 Subject: [PATCH 1/2] Add integration tests to reproduce a crash in WpComApiClient --- wp_api/src/api_error.rs | 14 +++++++++ wp_api_integration_tests/src/lib.rs | 12 +++++++ .../tests/test_wp_com_immut.rs | 31 +++++++++++++++++++ 3 files changed, 57 insertions(+) create mode 100644 wp_api_integration_tests/tests/test_wp_com_immut.rs diff --git a/wp_api/src/api_error.rs b/wp_api/src/api_error.rs index b6972417f..34e8a84c2 100644 --- a/wp_api/src/api_error.rs +++ b/wp_api/src/api_error.rs @@ -50,6 +50,20 @@ pub enum WpApiError { }, } +impl WpApiError { + pub fn status_code(&self) -> Option { + match self { + WpApiError::InvalidHttpStatusCode { status_code } + | WpApiError::UnknownError { status_code, .. } + | WpApiError::WpError { status_code, .. } => Some(*status_code), + WpApiError::RequestExecutionFailed { status_code, .. } => *status_code, + WpApiError::MediaFileNotFound { .. } + | WpApiError::ResponseParsingError { .. } + | WpApiError::SiteUrlParsingError { .. } => None, + } + } +} + impl MaybeWpError for WpApiError { fn wp_error_code(&self) -> Option<&WpErrorCode> { match self { diff --git a/wp_api_integration_tests/src/lib.rs b/wp_api_integration_tests/src/lib.rs index 9d5a55757..49fb988f6 100644 --- a/wp_api_integration_tests/src/lib.rs +++ b/wp_api_integration_tests/src/lib.rs @@ -153,6 +153,18 @@ pub fn wp_com_client() -> WpComApiClient { }) } +pub fn wp_com_client_with_invalid_token() -> WpComApiClient { + WpComApiClient::new(WpApiClientDelegate { + auth_provider: WpAuthenticationProvider::static_with_auth(WpAuthentication::Bearer { + token: "invalid_token".to_string(), + }) + .into(), + request_executor: Arc::new(ReqwestRequestExecutor::default()), + middleware_pipeline: Arc::new(WpApiMiddlewarePipeline::default()), + app_notifier: Arc::new(EmptyAppNotifier), + }) +} + pub fn api_client_backed_by_wp_com(site_id: String) -> WpApiClient { WpApiClient::new( Arc::new(WpComDotOrgApiUrlResolver::new( diff --git a/wp_api_integration_tests/tests/test_wp_com_immut.rs b/wp_api_integration_tests/tests/test_wp_com_immut.rs new file mode 100644 index 000000000..dde570dbd --- /dev/null +++ b/wp_api_integration_tests/tests/test_wp_com_immut.rs @@ -0,0 +1,31 @@ +use serial_test::parallel; +use wp_api_integration_tests::wp_com_client_with_invalid_token; + +#[tokio::test] +#[parallel] +async fn use_invalid_token_get_me() { + let client = wp_com_client_with_invalid_token(); + let result = client.me().get().await; + let err = result.unwrap_err(); + let status_code = err.status_code().unwrap(); + assert!( + (400..500).contains(&status_code), + "Expected status code in 4xx range, got: {status_code}" + ); +} + +#[tokio::test] +#[parallel] +async fn use_invalid_token_get_support_conversation_list() { + let client = wp_com_client_with_invalid_token(); + let result = client + .support_tickets() + .get_support_conversation_list() + .await; + let err = result.unwrap_err(); + let status_code = err.status_code().unwrap(); + assert!( + (400..500).contains(&status_code), + "Expected status code in 4xx range, got: {status_code}" + ); +} From fce2199a2c8d74ded6ab3d199c893f7227c2a04b Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 28 Jan 2026 19:51:35 +1300 Subject: [PATCH 2/2] Add can_resolve function to prevent panic --- wp_api/src/request.rs | 6 +++++- wp_api/src/request/endpoint.rs | 6 ++++++ wp_api/src/wp_com/endpoint.rs | 34 ++++++++++++++++++---------------- 3 files changed, 29 insertions(+), 17 deletions(-) diff --git a/wp_api/src/request.rs b/wp_api/src/request.rs index 86f717716..78a4d3f09 100644 --- a/wp_api/src/request.rs +++ b/wp_api/src/request.rs @@ -10,7 +10,7 @@ use crate::{ use base64::Engine; use chrono::{DateTime, Utc}; use endpoint::{ - ApiEndpointUrl, ApiUrlResolver, + ApiEndpointUrl, ApiUrlResolver, AsNamespace, WpNamespace, application_passwords_endpoint::{ ApplicationPasswordsRequestBuilder, ApplicationPasswordsRequestRetrieveCurrentWithEditContextResponse, @@ -943,6 +943,10 @@ pub async fn fetch_authentication_state( api_url_resolver: Arc, authentication_provider: Arc, ) -> Result { + if !api_url_resolver.can_resolve(WpNamespace::WpV2.namespace_value().to_string()) { + return Ok(AuthenticationState::Unauthorized); + } + let request = ApplicationPasswordsRequestBuilder::new(api_url_resolver, authentication_provider) .retrieve_current_with_edit_context() diff --git a/wp_api/src/request/endpoint.rs b/wp_api/src/request/endpoint.rs index 656a34e7b..9c9e338d4 100644 --- a/wp_api/src/request/endpoint.rs +++ b/wp_api/src/request/endpoint.rs @@ -118,6 +118,8 @@ impl AsNamespace for WpNamespace { #[uniffi::export(with_foreign)] pub trait ApiUrlResolver: Send + Sync { + fn can_resolve(&self, namespace: String) -> bool; + fn resolve(&self, namespace: String, endpoint_segments: Vec) -> Arc; } @@ -140,6 +142,10 @@ impl WpOrgSiteApiUrlResolver { #[uniffi::export] impl ApiUrlResolver for WpOrgSiteApiUrlResolver { + fn can_resolve(&self, _namespace: String) -> bool { + true + } + fn resolve(&self, namespace: String, endpoint_segments: Vec) -> Arc { Arc::new( self.api_root_url diff --git a/wp_api/src/wp_com/endpoint.rs b/wp_api/src/wp_com/endpoint.rs index b27623216..420a48a2d 100644 --- a/wp_api/src/wp_com/endpoint.rs +++ b/wp_api/src/wp_com/endpoint.rs @@ -37,16 +37,17 @@ impl WpComDotOrgApiUrlResolver { #[uniffi::export] impl ApiUrlResolver for WpComDotOrgApiUrlResolver { + fn can_resolve(&self, namespace: String) -> bool { + WpNamespace::iter().any(|n| n.namespace_value() == namespace) + } + fn resolve(&self, namespace: String, endpoint_segments: Vec) -> Arc { - { - if !WpNamespace::iter().any(|n| n.namespace_value() == namespace) { - panic!( - "`WpComDotOrgApiUrlResolver` doesn't support the namespace `{}`. The supported namespaces are: {:?}", - namespace, - WpNamespace::iter() - ); - } - } + assert!( + self.can_resolve(namespace.clone()), + "`WpComDotOrgApiUrlResolver` doesn't support the namespace `{}`. The supported namespaces are: {:?}", + namespace, + WpNamespace::iter() + ); // The API root endpoint needs special handling for WordPress.com if namespace == WpNamespace::None.namespace_value() && endpoint_segments.is_empty() { @@ -91,14 +92,15 @@ impl Default for WpComApiClientInternalUrlResolver { } impl ApiUrlResolver for WpComApiClientInternalUrlResolver { + fn can_resolve(&self, namespace: String) -> bool { + !WpNamespace::iter().any(|n| n.namespace_value() == namespace) + } + fn resolve(&self, namespace: String, endpoint_segments: Vec) -> Arc { - { - if WpNamespace::iter().any(|n| n.namespace_value() == namespace) { - panic!( - "`WpComApiClient` doesn't support the namespace `{namespace}`. Try using `WpApiClient` instead.", - ); - } - } + assert!( + self.can_resolve(namespace.clone()), + "`WpComApiClient` doesn't support the namespace `{namespace}`. Try using `WpApiClient` instead.", + ); Arc::new( self.base_url .by_extending_and_splitting_by_forward_slash(