Skip to content

Commit 2a47331

Browse files
committed
Encode PastEpochDeletionPolicy as a portable, bincode-safe u64
Upstream (openmls#2051) deserializes this policy through a #[serde(untagged)] enum, which requires a self-describing format. libxmtp persists MlsGroupJoinConfig through bincode (non-self-describing), where untagged deserialization fails at runtime — so on the merged tree every group's config load would error. Upstream does not hit this because its own storage backend uses serde_json. Restore the fork's plain-u64 encoding: serialize KeepAll as u64::MAX (not the platform-dependent usize::MAX, which is u32::MAX on wasm32) and deserialize a plain u64, checking the sentinel before narrowing to usize. This is both bincode-safe and portable across the 32/64-bit boundary. The pre-8bdba6f tagged form is dropped: the fork only ever wrote the plain-integer form.
1 parent e11f76e commit 2a47331

3 files changed

Lines changed: 54 additions & 33 deletions

File tree

Cargo.lock

Lines changed: 10 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

openmls/Cargo.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,10 @@ wasm-bindgen = "0.2.100"
170170
wasm-bindgen-test = "0.3.50"
171171
clap = { version = "4", features = ["derive"] }
172172
base64 = "0.22.1"
173+
# Non-self-describing codec used to guard `PastEpochDeletionPolicy` against a
174+
# regression to `#[serde(untagged)]`, which fails under such formats (the shape
175+
# libxmtp persists with). Matches libxmtp's bincode 1.x.
176+
bincode = "1.3"
173177
flate2 = "1.0"
174178
indicatif = "0.18.3"
175179

openmls/src/group/mls_group/config.rs

Lines changed: 40 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -76,38 +76,36 @@ impl Serialize for PastEpochDeletionPolicy {
7676
where
7777
S: serde::Serializer,
7878
{
79-
let usize = match self {
80-
Self::MaxEpochs(epochs) => *epochs,
81-
Self::KeepAll => usize::MAX,
79+
// Wire encoding is always `u64` (never platform-dependent `usize`) so
80+
// bytes written on a 64-bit host stay readable on `wasm32` and vice
81+
// versa. `KeepAll` is encoded as `u64::MAX` rather than `usize::MAX as
82+
// u64`, because `usize::MAX` differs between platforms (`u32::MAX` on
83+
// `wasm32`, `u64::MAX` on 64-bit).
84+
let value: u64 = match self {
85+
Self::MaxEpochs(epochs) => *epochs as u64,
86+
Self::KeepAll => u64::MAX,
8287
};
83-
serializer.serialize_u64(usize as u64)
88+
serializer.serialize_u64(value)
8489
}
8590
}
8691

8792
impl<'de> Deserialize<'de> for PastEpochDeletionPolicy {
93+
// Intentionally a plain `u64`, not upstream's `#[serde(untagged)]`
94+
// `Int | Tagged` form. `untagged` buffers into a self-describing value and
95+
// so requires `Deserializer::deserialize_any`; libxmtp persists this via
96+
// bincode, which is non-self-describing and errors on `deserialize_any`, so
97+
// the untagged form fails at runtime for every group. The fork only ever
98+
// wrote the plain-integer form, so the pre-8bdba6f tagged variant is moot.
8899
fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
89-
// Previously supported format (changed to plain integer in 8bdba6f)
90-
#[derive(Deserialize)]
91-
enum Tagged {
92-
MaxEpochs(usize),
93-
KeepAll,
100+
// Check the `KeepAll` sentinel before narrowing, so a `KeepAll` written
101+
// on a 64-bit host (`u64::MAX`) still loads on `wasm32`.
102+
let value = u64::deserialize(deserializer)?;
103+
if value == u64::MAX {
104+
Ok(Self::KeepAll)
105+
} else {
106+
let epochs = usize::try_from(value).map_err(serde::de::Error::custom)?;
107+
Ok(Self::MaxEpochs(epochs))
94108
}
95-
96-
#[derive(Deserialize)]
97-
#[serde(untagged)]
98-
enum Format {
99-
Int(u64),
100-
Tagged(Tagged),
101-
}
102-
103-
Ok(match Format::deserialize(deserializer)? {
104-
Format::Int(u64::MAX) => Self::KeepAll,
105-
Format::Int(n) => {
106-
Self::MaxEpochs(usize::try_from(n).map_err(serde::de::Error::custom)?)
107-
}
108-
Format::Tagged(Tagged::MaxEpochs(n)) => Self::MaxEpochs(n),
109-
Format::Tagged(Tagged::KeepAll) => Self::KeepAll,
110-
})
111109
}
112110
}
113111

@@ -724,14 +722,23 @@ mod tests {
724722
assert_eq!(deserialized, PastEpochDeletionPolicy::KeepAll);
725723
}
726724

727-
#[test]
728-
fn past_epoch_deletion_policy_deserializes_legacy_tagged_format() {
729-
// Externally tagged enum format used before 8bdba6f.
730-
let deserialized: PastEpochDeletionPolicy =
731-
serde_json::from_str(r#"{"MaxEpochs":42}"#).unwrap();
732-
assert_eq!(deserialized, PastEpochDeletionPolicy::MaxEpochs(42));
725+
// The pre-8bdba6f externally-tagged format is intentionally unsupported: it
726+
// is unrepresentable under the non-self-describing codec libxmtp persists
727+
// with (see the `Deserialize` impl), and the fork never wrote it.
733728

734-
let deserialized: PastEpochDeletionPolicy = serde_json::from_str(r#""KeepAll""#).unwrap();
735-
assert_eq!(deserialized, PastEpochDeletionPolicy::KeepAll);
729+
#[test]
730+
fn past_epoch_deletion_policy_roundtrips_through_bincode() {
731+
// bincode is non-self-describing (like libxmtp's store) and errors on
732+
// `deserialize_any`, so an `#[serde(untagged)]` deserializer would fail
733+
// here. This is the regression guard for the #2051 trap.
734+
for policy in [
735+
PastEpochDeletionPolicy::MaxEpochs(0),
736+
PastEpochDeletionPolicy::MaxEpochs(42),
737+
PastEpochDeletionPolicy::KeepAll,
738+
] {
739+
let bytes = bincode::serialize(&policy).unwrap();
740+
let deserialized: PastEpochDeletionPolicy = bincode::deserialize(&bytes).unwrap();
741+
assert_eq!(deserialized, policy);
742+
}
736743
}
737744
}

0 commit comments

Comments
 (0)