From 304356aa852bd1bded3906c4a17facc12f136ce5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jalil=20David=20Salam=C3=A9=20Messina?= Date: Sat, 30 Aug 2025 22:55:56 +0200 Subject: [PATCH] refactor(encoding): remove as many as casts as possible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There was a bug in the `impl EncodeGaugeValue for u64` implementation due to the `as i64` cast. The implementation only checked if `self == u64::MAX`, this is wrong since `u64::MAX` is not the only value that cannot be represented by an `i64`. The range of values is actually `self > i64::MAX`. `u64::MAX == 2^64 - 1` whereas `i64::MAX == 2^63 - 1`, this means there are `2^63` `u64` values that are not representable by an `i64`, not just `i64::MAX`. Delegating the checks to `i64::try_from(self)` ensures the logic is right. For this reason I also switched the rest of the implementations to use `N::from` instead of `as N` since the `N::from` function is only implemented for types whose conversion is infallible, this guarantees no such issues will ever happen. The only `as` cast left is `u64 as f64` in `EncodeExemplarValue`, this is not possible to remove since there is no `impl TryFrom for f64`. Signed-off-by: Jalil David Salamé Messina --- CHANGELOG.md | 11 +++++++++++ src/encoding.rs | 22 +++++++++------------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index de4db13..526489c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,17 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.24.1] + +### Fixed + +- `EncodeGaugeValue`, `EncodeCounterValue` and `EncodeExemplarValue` now use + fewer `as` casts in their implementation. This caught an issue where + `EncodeGaugeValue` would not error when encoding some `u64`s that don't fit + in a `i64`. See [PR 281]. + +[PR 281]: https://github.com/prometheus/client_rust/pull/281 + ## [0.24.0] ### Added diff --git a/src/encoding.rs b/src/encoding.rs index 7c25d98..9eb18bd 100644 --- a/src/encoding.rs +++ b/src/encoding.rs @@ -601,13 +601,9 @@ impl EncodeGaugeValue for i64 { impl EncodeGaugeValue for u64 { fn encode(&self, encoder: &mut GaugeValueEncoder) -> Result<(), std::fmt::Error> { // Between forcing end users to do endless as i64 for things that are - // clearly u64 and having one error case for rarely used protobuf when - // a gauge is set to u64::MAX, the latter seems like the right choice. - if *self == u64::MAX { - return Err(std::fmt::Error); - } - - encoder.encode_i64(*self as i64) + // clearly valid i64 and having one error case for rarely used protobuf when + // a gauge is set to >i64::MAX, the latter seems like the right choice. + encoder.encode_i64(i64::try_from(*self).map_err(|_err| std::fmt::Error)?) } } @@ -619,13 +615,13 @@ impl EncodeGaugeValue for f64 { impl EncodeGaugeValue for i32 { fn encode(&self, encoder: &mut GaugeValueEncoder) -> Result<(), std::fmt::Error> { - encoder.encode_i64(*self as i64) + encoder.encode_i64(i64::from(*self)) } } impl EncodeGaugeValue for f32 { fn encode(&self, encoder: &mut GaugeValueEncoder) -> Result<(), std::fmt::Error> { - encoder.encode_f64(*self as f64) + encoder.encode_f64(f64::from(*self)) } } @@ -687,13 +683,13 @@ impl EncodeCounterValue for f64 { impl EncodeCounterValue for u32 { fn encode(&self, encoder: &mut CounterValueEncoder) -> Result<(), std::fmt::Error> { - encoder.encode_u64(*self as u64) + encoder.encode_u64(u64::from(*self)) } } impl EncodeCounterValue for f32 { fn encode(&self, encoder: &mut CounterValueEncoder) -> Result<(), std::fmt::Error> { - encoder.encode_f64(*self as f64) + encoder.encode_f64(f64::from(*self)) } } @@ -751,13 +747,13 @@ impl EncodeExemplarValue for u64 { impl EncodeExemplarValue for f32 { fn encode(&self, mut encoder: ExemplarValueEncoder) -> Result<(), std::fmt::Error> { - encoder.encode(*self as f64) + encoder.encode(f64::from(*self)) } } impl EncodeExemplarValue for u32 { fn encode(&self, mut encoder: ExemplarValueEncoder) -> Result<(), std::fmt::Error> { - encoder.encode(*self as f64) + encoder.encode(f64::from(*self)) } }