diff --git a/.changesets/fix_igni_safely_deal_with_invalid_redis_keys.md b/.changesets/fix_igni_safely_deal_with_invalid_redis_keys.md new file mode 100644 index 0000000000..7c1169450c --- /dev/null +++ b/.changesets/fix_igni_safely_deal_with_invalid_redis_keys.md @@ -0,0 +1,6 @@ +### Redis storage: return an error instead if a non serializable value is sent. ([#3594](https://github.com/apollographql/router/issues/3594)) + +This changeset returns an error if a value couldn't be serialized before being sent to the redis storage backend. +It also logs the error in console and prompts you to open an issue (This message showing up would be a router bug!). + +By [@o0Ignition0o](https://github.com/o0Ignition0o) in https://github.com/apollographql/router/pull/3597 diff --git a/apollo-router/src/cache/redis.rs b/apollo-router/src/cache/redis.rs index 1dae895287..67c070a3b6 100644 --- a/apollo-router/src/cache/redis.rs +++ b/apollo-router/src/cache/redis.rs @@ -101,8 +101,13 @@ where type Error = RedisError; fn try_into(self) -> Result { - let v = serde_json::to_vec(&self.0) - .expect("JSON serialization should not fail for redis values"); + let v = serde_json::to_vec(&self.0).map_err(|e| { + tracing::error!("couldn't serialize value to redis {}. This is a bug in the router, please file an issue: https://github.com/apollographql/router/issues/new", e); + RedisError::new( + RedisErrorKind::Parse, + format!("couldn't serialize value to redis {}", e), + ) + })?; Ok(fred::types::RedisValue::Bytes(v.into())) } @@ -332,3 +337,25 @@ impl RedisCacheStorage { tracing::trace!("insert result {:?}", r); } } + +#[cfg(test)] +mod test { + use std::time::SystemTime; + + #[test] + fn ensure_invalid_payload_serialization_doesnt_fail() { + #[derive(Clone, Debug, serde::Serialize, serde::Deserialize)] + struct Stuff { + time: SystemTime, + } + + let invalid_json_payload = super::RedisValue(Stuff { + // this systemtime is invalid, serialization will fail + time: std::time::UNIX_EPOCH - std::time::Duration::new(1, 0), + }); + + let as_value: Result = invalid_json_payload.try_into(); + + assert!(as_value.is_err()); + } +}