Commit 5a4db67b authored by eta's avatar eta
Browse files

Add Futureproof<T> wrapper type, use for GuardDisabled enum 

The Futureproof<T> type lets you serialize and deserialize types whose
representations might change (most useful for enums that might grow
additional variants). It uses #[serde(untagged)] to accomplish this.

This gets used in order to make the `disabled` field of `Guard` more
robust against future guard disablement reasons being added.

A test was also added to verify correct behaviour of the new type.
parent 7a931b4d

crates/tor-guardmgr/src/guard.rs

+4 −6
Original line number Diff line number Diff line
@@ -12,7 +12,7 @@ use tracing::{trace, warn}; 


use crate::util::randomize_time;

use crate::{GuardId, GuardParams, GuardRestriction, GuardUsage};

use tor_persist::JsonValue;

use tor_persist::{Futureproof, JsonValue};



/// Tri-state to represent whether a guard is believed to be reachable or not.

#[derive(Debug, Clone, Copy, Eq, PartialEq)]
@@ -99,10 +99,8 @@ pub(crate) struct Guard { 


    /// If present, this guard is permanently disabled, and this

    /// object tells us why.

    // TODO: Wrap this in some kind of a future-proofing wrapper so that

    // we can safely add more variants to GuardDisabled.

    #[serde(default)]

    disabled: Option<GuardDisabled>,

    disabled: Option<Futureproof<GuardDisabled>>,



    /// When, approximately, did we first successfully use this guard?

    ///
@@ -510,7 +508,7 @@ impl Guard { 
                    threshold_ratio: DISABLE_THRESHOLD,

                };

                warn!(guard=?self.id, "Disabling guard: {:.1}% of circuits died under mysterious circumstances, exceeding threshold of {:.1}%", ratio*100.0, (DISABLE_THRESHOLD*100.0));

                self.disabled = Some(reason);

                self.disabled = Some(reason.into());

            } else if ratio > WARN_THRESHOLD && !self.suspicious_behavior_warned {

                warn!(guard=?self.id, "Questionable guard: {:.1}% of circuits died under mysterious circumstances.", ratio*100.0);

                self.suspicious_behavior_warned = true;
@@ -979,7 +977,7 @@ mod test { 
        assert!(g.disabled.is_some());



        #[allow(unreachable_patterns)]

        match g.disabled.unwrap() {

        match g.disabled.unwrap().into_option().unwrap() {

            GuardDisabled::TooManyIndeterminateFailures {

                history: _,

                failure_ratio,

crates/tor-persist/src/lib.rs

+31 −1
Original line number Diff line number Diff line
@@ -43,7 +43,7 @@ mod handle; 
#[cfg(feature = "testing")]

mod testing;



use serde::{de::DeserializeOwned, Serialize};

use serde::{de::DeserializeOwned, Deserialize, Serialize};

use std::sync::Arc;



/// Wrapper type for Results returned from this crate.
@@ -152,3 +152,33 @@ impl From<serde_json::Error> for Error { 
        Error::JsonError(Arc::new(e))

    }

}



/// A wrapper type for types whose representation may change in future versions of Arti.

///

/// This uses `#[serde(untagged)]` to attempt deserializing as a type `T` first, and falls back

/// to a generic JSON value representation if that fails.

#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)]

#[serde(untagged)]

#[allow(clippy::exhaustive_enums)]

pub enum Futureproof<T> {

    /// A successfully-deserialized `T`.

    Understandable(T),

    /// A generic JSON value, representing a failure to deserialize a `T`.

    Unknown(JsonValue),

}



impl<T> Futureproof<T> {

    /// Convert the `Futureproof` into an `Option<T>`, throwing away an `Unknown` value.

    pub fn into_option(self) -> Option<T> {

        match self {

            Futureproof::Understandable(x) => Some(x),

            Futureproof::Unknown(_) => None,

        }

    }

}



impl<T> From<T> for Futureproof<T> {

    fn from(inner: T) -> Self {

        Self::Understandable(inner)

    }

}

crates/tor-persist/src/testing.rs

+42 −0
Original line number Diff line number Diff line
@@ -121,6 +121,15 @@ mod test { 
        s1: String,

        s2: String,

    }

    #[derive(Eq, PartialEq, Clone, Debug, Serialize, Deserialize)]

    enum OldEnum {

        Variant1,

    }

    #[derive(Eq, PartialEq, Clone, Debug, Serialize, Deserialize)]

    enum NewEnum {

        Variant1,

        Variant2,

    }



    #[test]

    fn basic_tests() {
@@ -202,4 +211,37 @@ mod test { 
        assert_eq!(h2.load().unwrap(), Some(s1));

        assert_eq!(h3.load().unwrap(), Some(s2));

    }



    #[test]

    fn futureproof() {

        use crate::Futureproof;



        let v1 = Ex1 { v1: 8, v2: 99 };



        let v1_ser = serde_json::to_string(&v1).unwrap();



        let v1_as_ex1: Futureproof<Ex1> = serde_json::from_str(&v1_ser).unwrap();

        let v1_as_ex2: Futureproof<Ex2> = serde_json::from_str(&v1_ser).unwrap();

        assert!(v1_as_ex1.clone().into_option().is_some());

        assert!(v1_as_ex2.into_option().is_none());



        assert_eq!(serde_json::to_string(&v1_as_ex1).unwrap(), v1_ser);

    }



    #[test]

    fn futureproof_enums() {

        use crate::Futureproof;



        let new1 = NewEnum::Variant1;

        let new2 = NewEnum::Variant2;



        let new1_ser = serde_json::to_string(&new1).unwrap();

        let new2_ser = serde_json::to_string(&new2).unwrap();



        let old1: Futureproof<OldEnum> = serde_json::from_str(&new1_ser).unwrap();

        let old2: Futureproof<OldEnum> = serde_json::from_str(&new2_ser).unwrap();



        assert!(old1.into_option().is_some());

        assert!(old2.into_option().is_none());

    }

}