From 0a58e80bbc312a5a628c3cf8a65cde5f2c6384d1 Mon Sep 17 00:00:00 2001 From: marc-signal Date: Thu, 26 Mar 2026 20:08:19 -0400 Subject: [PATCH] Add support for a retry later duration in rate limit challenge responses --- RELEASE_NOTES.md | 2 ++ .../net/RateLimitChallengeException.kt | 25 +++++++++++++++-- .../net/RegistrationServiceTest.java | 9 +++++- node/ts/Errors.ts | 1 + node/ts/test/RegistrationTest.ts | 12 ++++++++ rust/bridge/ffi/src/error.rs | 28 +++++++++++++------ .../shared/testing/src/net/registration.rs | 6 ++++ rust/bridge/shared/types/src/ffi/convert.rs | 2 ++ rust/bridge/shared/types/src/jni/mod.rs | 18 ++++++++++-- rust/bridge/shared/types/src/node/error.rs | 17 +++++++++-- rust/net/chat/src/api.rs | 20 +++++++++++-- rust/net/chat/src/ws.rs | 25 +++++++++++++++-- rust/net/chat/src/ws/messages.rs | 12 ++++++-- swift/Sources/LibSignalClient/Error.swift | 14 ++++++++-- swift/Sources/SignalFfi/signal_ffi.h | 7 ++++- .../RegistrationServiceTests.swift | 5 +++- 16 files changed, 175 insertions(+), 28 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 5595a75ac..b63931123 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,5 +1,7 @@ v0.90.0 +- Add support for a retry later duration in rate limit responses + - keytrans: Simplify the top-level API - Use CDSI enclave 3a1ac5e5 in staging. diff --git a/java/client/src/main/java/org/signal/libsignal/net/RateLimitChallengeException.kt b/java/client/src/main/java/org/signal/libsignal/net/RateLimitChallengeException.kt index bdc84dd18..4d0e1719f 100644 --- a/java/client/src/main/java/org/signal/libsignal/net/RateLimitChallengeException.kt +++ b/java/client/src/main/java/org/signal/libsignal/net/RateLimitChallengeException.kt @@ -6,6 +6,7 @@ package org.signal.libsignal.net import org.signal.libsignal.internal.CalledFromNative +import java.time.Duration import java.util.EnumSet /** @@ -17,10 +18,30 @@ import java.util.EnumSet public class RateLimitChallengeException : ChatServiceException { public val token: String public val options: Set + public val retryLater: Duration? - @CalledFromNative - public constructor(message: String, token: String, options: Array) : super(message) { + public constructor( + message: String, + token: String, + options: Array, + retryLater: Duration?, + ) : super(message) { this.token = token this.options = EnumSet.copyOf(options.asList()) + this.retryLater = retryLater + } + + @CalledFromNative + internal constructor( + message: String, + token: String, + options: Array, + retryLater: Long, + ) : this( + message, + token, + options, + if (retryLater < 0) null else Duration.ofSeconds(retryLater), + ) { } } diff --git a/java/client/src/test/java/org/signal/libsignal/net/RegistrationServiceTest.java b/java/client/src/test/java/org/signal/libsignal/net/RegistrationServiceTest.java index 9630e2222..1d3e3e20b 100644 --- a/java/client/src/test/java/org/signal/libsignal/net/RegistrationServiceTest.java +++ b/java/client/src/test/java/org/signal/libsignal/net/RegistrationServiceTest.java @@ -266,11 +266,18 @@ public class RegistrationServiceTest { } private static void assertIsPushChallengeError(ThrowingConsumer throwError) { - RateLimitChallengeException e = + final RateLimitChallengeException e = assertRegistrationSessionErrorIs( "PushChallenge", RateLimitChallengeException.class, throwError); assertEquals(e.getToken(), "token"); assertEquals(e.getOptions(), EnumSet.of(ChallengeOption.PUSH_CHALLENGE)); + assertEquals(e.getRetryLater(), null); + final RateLimitChallengeException e2 = + assertRegistrationSessionErrorIs( + "PushChallengeRetryAfter42Seconds", RateLimitChallengeException.class, throwError); + assertEquals(e2.getToken(), "token42"); + assertEquals(e2.getOptions(), EnumSet.of(ChallengeOption.PUSH_CHALLENGE)); + assertEquals(e2.getRetryLater(), Duration.ofSeconds(42)); } @Test diff --git a/node/ts/Errors.ts b/node/ts/Errors.ts index eb6086ce5..e4fe3bf4e 100644 --- a/node/ts/Errors.ts +++ b/node/ts/Errors.ts @@ -307,6 +307,7 @@ export type RateLimitChallengeError = LibSignalErrorBase & { code: ErrorCode.RateLimitChallengeError; readonly token: string; readonly options: Set<'pushChallenge' | 'captcha'>; + readonly retryAfterSecs: number | null; }; export type ChatServiceInactive = LibSignalErrorBase & { diff --git a/node/ts/test/RegistrationTest.ts b/node/ts/test/RegistrationTest.ts index f3a300d83..86f696ec4 100644 --- a/node/ts/test/RegistrationTest.ts +++ b/node/ts/test/RegistrationTest.ts @@ -133,6 +133,16 @@ describe('Registration types', () => { code: ErrorCode.RateLimitChallengeError, token: 'token', options: new Set(['pushChallenge']), + retryAfterSecs: null, + }, + ]; + const rateLimitRetryChallengeCase: [string, object] = [ + 'PushChallengeRetryAfter42Seconds', + { + code: ErrorCode.RateLimitChallengeError, + token: 'token42', + options: new Set(['pushChallenge']), + retryAfterSecs: 42, }, ]; const cases: Array<{ @@ -150,6 +160,7 @@ describe('Registration types', () => { timeoutCase, serverSideErrorCase, rateLimitChallengeCase, + rateLimitRetryChallengeCase, ], }, { @@ -162,6 +173,7 @@ describe('Registration types', () => { timeoutCase, serverSideErrorCase, rateLimitChallengeCase, + rateLimitRetryChallengeCase, ], }, { diff --git a/rust/bridge/ffi/src/error.rs b/rust/bridge/ffi/src/error.rs index 407ab8646..d8ca362fe 100644 --- a/rust/bridge/ffi/src/error.rs +++ b/rust/bridge/ffi/src/error.rs @@ -12,6 +12,7 @@ use libsignal_bridge::ffi::{ use libsignal_bridge::{IllegalArgumentError, ffi_arg_type, ffi_result_type}; use libsignal_bridge_macros::bridge_fn; use libsignal_core::ProtocolAddress; +use libsignal_net::infra::errors::RetryLater; use libsignal_net_chat::api::ChallengeOption; use libsignal_net_chat::api::messages::MismatchedDeviceError; use uuid::Uuid; @@ -146,17 +147,28 @@ fn Error_GetTriesRemaining(err: &SignalFfiError) -> Result Result<(String, Box<[ChallengeOption]>), IllegalArgumentError> { - let libsignal_net_chat::api::RateLimitChallenge { token, options } = - err.provide_rate_limit_challenge().map_err(|_| { - IllegalArgumentError::new(format!( - "cannot get rate limit challenge error from error ({err})" - )) - })?; - Ok((token.clone(), options[..].into())) +) -> Result<((String, Box<[ChallengeOption]>), i64), IllegalArgumentError> { + let libsignal_net_chat::api::RateLimitChallenge { + token, + options, + retry_later, + } = err.provide_rate_limit_challenge().map_err(|_| { + IllegalArgumentError::new(format!( + "cannot get rate limit challenge error from error ({err})" + )) + })?; + let retry_later = retry_later + .map( + |RetryLater { + retry_after_seconds, + }| i64::from(retry_after_seconds), + ) + .unwrap_or(-1); + Ok(((token.clone(), options[..].into()), retry_later)) } #[bridge_fn(jni = false, node = false)] diff --git a/rust/bridge/shared/testing/src/net/registration.rs b/rust/bridge/shared/testing/src/net/registration.rs index c7a754836..01d31a194 100644 --- a/rust/bridge/shared/testing/src/net/registration.rs +++ b/rust/bridge/shared/testing/src/net/registration.rs @@ -189,6 +189,12 @@ impl TryFrom<&'a str, Error = strum::ParseError>> TryFrom "PushChallenge" => RequestError::Challenge(RateLimitChallenge { token: "token".to_owned(), options: vec![ChallengeOption::PushChallenge], + retry_later: None, + }), + "PushChallengeRetryAfter42Seconds" => RequestError::Challenge(RateLimitChallenge { + token: "token42".to_owned(), + options: vec![ChallengeOption::PushChallenge], + retry_later: Some(RETRY_AFTER_42_SECONDS), }), "ServerSideError" => RequestError::ServerSideError, _ => TestE::try_from(&value) diff --git a/rust/bridge/shared/types/src/ffi/convert.rs b/rust/bridge/shared/types/src/ffi/convert.rs index 2d3f8caf1..4ee9296ee 100644 --- a/rust/bridge/shared/types/src/ffi/convert.rs +++ b/rust/bridge/shared/types/src/ffi/convert.rs @@ -1381,6 +1381,7 @@ trivial!(u8); trivial!(u16); trivial!(u32); trivial!(u64); +trivial!(i64); trivial!(usize); trivial!(bool); @@ -1493,6 +1494,7 @@ macro_rules! ffi_result_type { (u32) => (u32); (Option) => (u32); (u64) => (u64); + (i64) => (i64); (Option) => (u64); (bool) => (bool); (&str) => (*const std::ffi::c_char); diff --git a/rust/bridge/shared/types/src/jni/mod.rs b/rust/bridge/shared/types/src/jni/mod.rs index c9b4de386..544ced41a 100644 --- a/rust/bridge/shared/types/src/jni/mod.rs +++ b/rust/bridge/shared/types/src/jni/mod.rs @@ -1031,18 +1031,32 @@ impl JniError for RateLimitChallenge { &self, env: &mut JNIEnv<'a>, ) -> Result, BridgeLayerError> { - let Self { token, options } = self; + let Self { + token, + options, + retry_later, + } = self; let (message, token) = try_scoped(|| Ok((env.new_string(self.to_string())?, env.new_string(token)?))) .check_exceptions(env, "RateLimitChallenge")?; let options = options.as_slice().convert_into(env)?; + let retry_later = retry_later + .as_ref() + .map( + |RetryLater { + retry_after_seconds, + }| i64::from(*retry_after_seconds), + ) + .unwrap_or(-1); new_instance( env, ClassName("org.signal.libsignal.net.RateLimitChallengeException"), jni_args!(( message => java.lang.String, token => java.lang.String, - options => [org.signal.libsignal.net.ChallengeOption]) -> void), + options => [org.signal.libsignal.net.ChallengeOption], + retry_later => long, + ) -> void), ) .map(Into::into) } diff --git a/rust/bridge/shared/types/src/node/error.rs b/rust/bridge/shared/types/src/node/error.rs index baeaa6e78..abd5258a1 100644 --- a/rust/bridge/shared/types/src/node/error.rs +++ b/rust/bridge/shared/types/src/node/error.rs @@ -6,7 +6,7 @@ use std::borrow::Cow; use std::fmt; -use libsignal_net::infra::errors::TransportConnectError; +use libsignal_net::infra::errors::{RetryLater, TransportConnectError}; use libsignal_net::infra::ws::WebSocketConnectError; use libsignal_net_chat::api::keys::GetPreKeysFailure; use neon::thread::LocalKey; @@ -521,15 +521,28 @@ impl SignalNodeError for libsignal_net_chat::api::RateLimitChallenge { operation_name: &str, ) -> Handle<'a, JsError> { let message = self.to_string(); - let Self { token, options } = self; + let Self { + token, + options, + retry_later, + } = self; let properties = move |cx: &mut C| { let token = cx.string(token); let options = options.into_boxed_slice().convert_into(cx)?.upcast(); let set_constructor: Handle<'_, JsFunction> = cx.global("Set")?; let options = set_constructor.construct(cx, [options])?; let props = cx.empty_object(); + let retry_later = retry_later + .map( + |RetryLater { + retry_after_seconds, + }| cx.number(retry_after_seconds), + ) + .map(|x| x.as_value(cx)) + .unwrap_or_else(|| cx.null().as_value(cx)); props.set(cx, "token", token)?; props.set(cx, "options", options)?; + props.set(cx, "retryAfterSecs", retry_later)?; Ok(props.upcast()) }; new_js_error( diff --git a/rust/net/chat/src/api.rs b/rust/net/chat/src/api.rs index 0103ec83a..ec75deb85 100644 --- a/rust/net/chat/src/api.rs +++ b/rust/net/chat/src/api.rs @@ -7,8 +7,9 @@ //! chat-server". use std::convert::Infallible; +use std::fmt::Formatter; -use libsignal_net::infra::errors::LogSafeDisplay; +use libsignal_net::infra::errors::{LogSafeDisplay, RetryLater}; use ref_cast::RefCast as _; pub mod backups; @@ -142,12 +143,25 @@ impl From for RequestError { } } -#[derive(Debug, thiserror::Error, displaydoc::Display)] +#[derive(Debug, thiserror::Error)] #[cfg_attr(test, derive(Clone))] -/// retry after completing a rate limit challenge {options:?} pub struct RateLimitChallenge { pub token: String, pub options: Vec, + pub retry_later: Option, +} +impl std::fmt::Display for RateLimitChallenge { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!( + f, + "retry after completing a rate limit challenge {:?}", + self.options + )?; + if let Some(retry_later) = &self.retry_later { + write!(f, " (or {retry_later})")?; + } + Ok(()) + } } impl LogSafeDisplay for RateLimitChallenge {} diff --git a/rust/net/chat/src/ws.rs b/rust/net/chat/src/ws.rs index bb757347d..a008d1ef2 100644 --- a/rust/net/chat/src/ws.rs +++ b/rust/net/chat/src/ws.rs @@ -293,7 +293,11 @@ impl ResponseError { if let Ok(ChallengeBody { token, options }) = parse_json_from_body(&response) { - return RequestError::Challenge(RateLimitChallenge { token, options }); + return RequestError::Challenge(RateLimitChallenge { + token, + options, + retry_later: extract_retry_later(headers), + }); } } if status.as_u16() == 422 { @@ -452,6 +456,18 @@ mod testutil { } } + pub(crate) fn with_headers( + headers: &[(http::HeaderName, &'static str)], + mut response: chat::Response, + ) -> chat::Response { + response.headers.extend( + headers + .iter() + .map(|(k, v)| (k.clone(), http::HeaderValue::from_static(v))), + ); + response + } + pub(crate) struct RequestValidator { pub expected: chat::Request, pub response: chat::Response, @@ -556,7 +572,12 @@ mod test { #[test_case(json(428, "{}") => matches Err(RequestError::Unexpected { log_safe: m }) if m.contains("428"))] #[test_case(json( 428, r#"{"token": "zzz", "options": ["captcha"]}"# - ) => matches Err(RequestError::Challenge(RateLimitChallenge { token, options })) if token == "zzz" && options == vec![ChallengeOption::Captcha])] + ) => matches Err(RequestError::Challenge(RateLimitChallenge { token, options, retry_later: None })) if token == "zzz" && options == vec![ChallengeOption::Captcha])] + #[test_case(with_headers(&[(http::header::RETRY_AFTER, "42")], json( + 428, r#"{"token": "zzz", "options": ["captcha"]}"# + )) => matches Err(RequestError::Challenge(RateLimitChallenge { token, options, retry_later: Some( + RetryLater { retry_after_seconds: 42 } + ) })) if token == "zzz" && options == vec![ChallengeOption::Captcha])] #[test_case(empty(422) => matches Err(RequestError::Unexpected { log_safe: m }) if m.contains("server validation"))] #[test_case(empty(419) => matches Err(RequestError::Unexpected { log_safe: m }) if m.contains("419"))] fn try_parse_empty( diff --git a/rust/net/chat/src/ws/messages.rs b/rust/net/chat/src/ws/messages.rs index 11cd8dcff..a2c924f64 100644 --- a/rust/net/chat/src/ws/messages.rs +++ b/rust/net/chat/src/ws/messages.rs @@ -550,6 +550,7 @@ mod test { use futures_util::FutureExt; use libsignal_core::{Aci, Pni}; + use libsignal_net::infra::errors::RetryLater; use libsignal_protocol::{CiphertextMessage, PlaintextContent, Timestamp}; use serde_json::json; use test_case::test_case; @@ -562,7 +563,7 @@ mod test { }; use crate::api::{ChallengeOption, RateLimitChallenge, UserBasedAuthorization}; use crate::ws::ACCESS_KEY_HEADER_NAME; - use crate::ws::testutil::{JsonRequestValidator, RequestValidator, empty, json}; + use crate::ws::testutil::{JsonRequestValidator, RequestValidator, empty, json, with_headers}; const ACI_UUID: &str = "9d0652a3-dcc3-4d11-975f-74d61598733f"; const PNI_UUID: &str = "796abedb-ca4e-4f18-8803-1fde5b921f9f"; @@ -858,7 +859,12 @@ mod test { #[test_case(json(428, "{}") => matches Err(RequestError::Unexpected { log_safe: m }) if m.contains("428"))] #[test_case(json( 428, r#"{"token": "zzz", "options": ["captcha"]}"# - ) => matches Err(RequestError::Challenge(RateLimitChallenge { token, options })) if token == "zzz" && options == vec![ChallengeOption::Captcha])] + ) => matches Err(RequestError::Challenge(RateLimitChallenge { token, options, retry_later: None })) if token == "zzz" && options == vec![ChallengeOption::Captcha])] + #[test_case(with_headers(&[(http::header::RETRY_AFTER, "42")], json( + 428, r#"{"token": "zzz", "options": ["captcha"]}"# + )) => matches Err(RequestError::Challenge(RateLimitChallenge { token, options, retry_later: Some( + RetryLater { retry_after_seconds: 42 } + ) })) if token == "zzz" && options == vec![ChallengeOption::Captcha])] fn test_unsealed_send(response: Response) -> Result<(), RequestError> { let validator = JsonRequestValidator { expected: Request { @@ -953,7 +959,7 @@ mod test { )] #[test_case(json( 428, r#"{"token": "zzz", "options": ["captcha"]}"# - ) => matches Err(RequestError::Challenge(RateLimitChallenge { token, options })) if token == "zzz" && options == vec![ChallengeOption::Captcha])] + ) => matches Err(RequestError::Challenge(RateLimitChallenge { token, options, retry_later: None })) if token == "zzz" && options == vec![ChallengeOption::Captcha])] fn test_sync_send(response: Response) -> Result<(), RequestError> { let validator = JsonRequestValidator { expected: Request { diff --git a/swift/Sources/LibSignalClient/Error.swift b/swift/Sources/LibSignalClient/Error.swift index cccd6c339..cc3c8e4a9 100644 --- a/swift/Sources/LibSignalClient/Error.swift +++ b/swift/Sources/LibSignalClient/Error.swift @@ -58,7 +58,12 @@ public enum SignalError: Error { case networkProtocolError(String) case cdsiInvalidToken(String) case rateLimitedError(retryAfter: TimeInterval, message: String) - case rateLimitChallengeError(token: String, options: Set, message: String) + case rateLimitChallengeError( + token: String, + options: Set, + retryAfter: TimeInterval?, + message: String + ) case svrDataMissing(String) case svrRestoreFailed(triesRemaining: UInt32, message: String) case svrRotationMachineTooManySteps(String) @@ -230,9 +235,11 @@ internal func checkError(_ error: SignalFfiErrorRef?) throws { } throw SignalError.rateLimitedError(retryAfter: TimeInterval(retryAfterSeconds), message: errStr) case SignalErrorCodeRateLimitChallenge: - let pair = try invokeFnReturningValueByPointer(.init()) { + let outer_pair = try invokeFnReturningValueByPointer(.init()) { signal_error_get_rate_limit_challenge($0, error) } + let pair = outer_pair.first + let retryAfterRaw = outer_pair.second defer { signal_free_string(pair.first) signal_free_buffer(pair.second.base, pair.second.length) @@ -242,7 +249,8 @@ internal func checkError(_ error: SignalFfiErrorRef?) throws { throw SignalError.rateLimitChallengeError( token: token, options: Set(try options.lazy.map { try ChallengeOption(fromNative: $0) }), - message: errStr + retryAfter: (retryAfterRaw < 0) ? nil : TimeInterval(retryAfterRaw), + message: errStr, ) case SignalErrorCodeSvrDataMissing: throw SignalError.svrDataMissing(errStr) diff --git a/swift/Sources/SignalFfi/signal_ffi.h b/swift/Sources/SignalFfi/signal_ffi.h index ee26608d5..e4d7ab6cc 100644 --- a/swift/Sources/SignalFfi/signal_ffi.h +++ b/swift/Sources/SignalFfi/signal_ffi.h @@ -976,6 +976,11 @@ typedef struct { SignalOwnedBuffer second; } SignalPairOfc_charOwnedBufferOfc_uchar; +typedef struct { + SignalPairOfc_charOwnedBufferOfc_uchar first; + int64_t second; +} SignalPairOfPairOfc_charOwnedBufferOfc_uchari64; + typedef struct { const char *first; bool second; @@ -1938,7 +1943,7 @@ SignalFfiError *signal_error_get_mismatched_device_errors(SignalOwnedBufferOfFfi SignalFfiError *signal_error_get_our_fingerprint_version(uint32_t *out, SignalUnwindSafeArgSignalFfiError err); -SignalFfiError *signal_error_get_rate_limit_challenge(SignalPairOfc_charOwnedBufferOfc_uchar *out, SignalUnwindSafeArgSignalFfiError err); +SignalFfiError *signal_error_get_rate_limit_challenge(SignalPairOfPairOfc_charOwnedBufferOfc_uchari64 *out, SignalUnwindSafeArgSignalFfiError err); SignalFfiError *signal_error_get_registration_error_not_deliverable(SignalPairOfc_charbool *out, SignalUnwindSafeArgSignalFfiError err); diff --git a/swift/Tests/LibSignalClientTests/RegistrationServiceTests.swift b/swift/Tests/LibSignalClientTests/RegistrationServiceTests.swift index 474b47e89..10d8847f7 100644 --- a/swift/Tests/LibSignalClientTests/RegistrationServiceTests.swift +++ b/swift/Tests/LibSignalClientTests/RegistrationServiceTests.swift @@ -72,7 +72,8 @@ class RegistrationServiceConversionTests { let timeoutCase = ("Timeout", { (e: Error) in if case SignalError.requestTimeoutError("the request timed out") = e { true } else { false }}) let requestNotValidCase = ("RequestWasNotValid", { (e: Error) in if case RegistrationError.unknown("the request did not pass server validation") = e { true } else { false }}) let serverErrorCase = ("ServerSideError", { (e: Error) in if case RegistrationError.unknown("server-side error, retryable with backoff") = e { true } else { false }}) - let pushChallengeCase = ("PushChallenge", { (e: Error) in if case SignalError.rateLimitChallengeError(token: "token", options: [.pushChallenge], message: "retry after completing a rate limit challenge [PushChallenge]") = e { true } else { false }}) + let pushChallengeCase = ("PushChallenge", { (e: Error) in if case SignalError.rateLimitChallengeError(token: "token", options: [.pushChallenge], retryAfter: nil, message: "retry after completing a rate limit challenge [PushChallenge]") = e { true } else { false }}) + let pushChallengeRetryCase = ("PushChallengeRetryAfter42Seconds", { (e: Error) in if case SignalError.rateLimitChallengeError(token: "token42", options: [.pushChallenge], retryAfter: 42, message: "retry after completing a rate limit challenge [PushChallenge] (or retry after 42s)") = e { true } else { false }}) let cases = [ ErrorTest("CreateSession", signal_testing_registration_service_create_session_error_convert, [ @@ -83,6 +84,7 @@ class RegistrationServiceConversionTests { requestNotValidCase, serverErrorCase, pushChallengeCase, + pushChallengeRetryCase, ]), ErrorTest("ResumeSession", signal_testing_registration_service_resume_session_error_convert, [ ("InvalidSessionId", { if case RegistrationError.invalidSessionId("invalid session ID value") = $0 { true } else { false }}), @@ -92,6 +94,7 @@ class RegistrationServiceConversionTests { requestNotValidCase, serverErrorCase, pushChallengeCase, + pushChallengeRetryCase, ]), ErrorTest( "UpdateSession",