Make DecodeAuthorization return a Result rather than throw

Author: adam-p

SecretTestValues.h.enc

@@ -793,6 +793,17 @@ Result<PsiCash::NewExpiringPurchaseResponse> PsiCash::NewExpiringPurchase(
         }
         // Not checking authorization, as it doesn't apply to all expiring purchases
 
+        optional<Authorization> authOptional = nullopt;
+        if (!authorization_encoded.empty()) {
+            auto decodeAuthResult = DecodeAuthorization(authorization_encoded);
+            if (!decodeAuthResult) {
+                // Authorization can be optional, but inability to decode suggests
+                // something is very wrong.
+                return WrapError(decodeAuthResult.error(), "failed to decode Purchase Authorization");
+            }
+            authOptional = *decodeAuthResult;
+        }
+
         Purchase purchase = {
                 transaction_id,
                 transaction_class,
@@ -801,7 +812,7 @@ Result<PsiCash::NewExpiringPurchaseResponse> PsiCash::NewExpiringPurchase(
                         server_expiry),
                 server_expiry.IsZero() ? nullopt : make_optional(
                         server_expiry),
-                authorization_encoded.empty() ? nullopt : make_optional(DecodeAuthorization(authorization_encoded))
+                authOptional
         };
 
         user_data_->UpdatePurchaseLocalTimeExpiry(purchase);
@@ -950,12 +961,18 @@ void from_json(const json& j, Authorization& v) {
     v.encoded = j.value("Encoded", ""s);
 }
 
-Authorization DecodeAuthorization(const string& encoded) {
-    auto decoded = base64::B64Decode(encoded);
-    auto json =json::parse(decoded);
-    auto auth = json.at("Authorization").get<Authorization>();
-    auth.encoded = encoded;
-    return auth;
+Result<Authorization> DecodeAuthorization(const string& encoded) {
+    try {
+        auto decoded = base64::B64Decode(encoded);
+        auto json = json::parse(decoded);
+        auto auth = json.at("Authorization").get<Authorization>();
+        auth.encoded = encoded;
+        return auth;
+    }
+    catch (json::exception& e) {
+        return MakeCriticalError(
+                utils::Stringer("json parse failed: ", e.what(), "; id:", e.id).c_str());
+    }
 }
 
 } // namespace psicash

psicash.cpp

@@ -122,7 +122,7 @@ struct Authorization {
 using Authorizations = std::vector<Authorization>;
 
 // May be used for for decoding non-PsiCash authorizations.
-Authorization DecodeAuthorization(const std::string& encoded);
+error::Result<Authorization> DecodeAuthorization(const std::string& encoded);
 
 using TransactionID = std::string;
 extern const char* const kTransactionIDZero; // The "zero value" for a TransactionID

psicash.hpp

@@ -284,10 +284,11 @@ TEST_F(TestPsiCash, GetPurchases) {
     auto v = pc.GetPurchases();
     ASSERT_EQ(v.size(), 0);
 
-    auto auth = DecodeAuthorization("eyJBdXRob3JpemF0aW9uIjp7IklEIjoiMFYzRXhUdmlBdFNxTGZOd2FpQXlHNHpaRUJJOGpIYnp5bFdNeU5FZ1JEZz0iLCJBY2Nlc3NUeXBlIjoic3BlZWQtYm9vc3QtdGVzdCIsIkV4cGlyZXMiOiIyMDE5LTAxLTE0VDE3OjIyOjIzLjE2ODc2NDEyOVoifSwiU2lnbmluZ0tleUlEIjoiUUNZTzV2clIvZGhjRDZ6M2FMQlVNeWRuZlJyZFNRL1RWYW1IUFhYeTd0TT0iLCJTaWduYXR1cmUiOiJQL2NrenloVUJoSk5RQ24zMnluM1VTdGpLencxU04xNW9MclVhTU9XaW9scXBOTTBzNVFSNURHVEVDT1FzQk13ODdQdTc1TGE1OGtJTHRIcW1BVzhDQT09In0=");
+    auto auth_res = DecodeAuthorization("eyJBdXRob3JpemF0aW9uIjp7IklEIjoiMFYzRXhUdmlBdFNxTGZOd2FpQXlHNHpaRUJJOGpIYnp5bFdNeU5FZ1JEZz0iLCJBY2Nlc3NUeXBlIjoic3BlZWQtYm9vc3QtdGVzdCIsIkV4cGlyZXMiOiIyMDE5LTAxLTE0VDE3OjIyOjIzLjE2ODc2NDEyOVoifSwiU2lnbmluZ0tleUlEIjoiUUNZTzV2clIvZGhjRDZ6M2FMQlVNeWRuZlJyZFNRL1RWYW1IUFhYeTd0TT0iLCJTaWduYXR1cmUiOiJQL2NrenloVUJoSk5RQ24zMnluM1VTdGpLencxU04xNW9MclVhTU9XaW9scXBOTTBzNVFSNURHVEVDT1FzQk13ODdQdTc1TGE1OGtJTHRIcW1BVzhDQT09In0=");
+    ASSERT_TRUE(auth_res);
 
     Purchases ps = {
-            {"id1", "tc1", "d1", datetime::DateTime::Now(), datetime::DateTime::Now(), nonstd::make_optional(auth)},
+            {"id1", "tc1", "d1", datetime::DateTime::Now(), datetime::DateTime::Now(), *auth_res},
             {"id2", "tc2", "d2", nonstd::nullopt, nonstd::nullopt, nonstd::nullopt}};
 
     err = pc.user_data().SetPurchases(ps);
@@ -347,26 +348,34 @@ TEST_F(TestPsiCash, DecodeAuthorization) {
     const auto encoded1 = "eyJBdXRob3JpemF0aW9uIjp7IklEIjoiMFYzRXhUdmlBdFNxTGZOd2FpQXlHNHpaRUJJOGpIYnp5bFdNeU5FZ1JEZz0iLCJBY2Nlc3NUeXBlIjoic3BlZWQtYm9vc3QtdGVzdCIsIkV4cGlyZXMiOiIyMDE5LTAxLTE0VDE3OjIyOjIzLjE2ODc2NDEyOVoifSwiU2lnbmluZ0tleUlEIjoiUUNZTzV2clIvZGhjRDZ6M2FMQlVNeWRuZlJyZFNRL1RWYW1IUFhYeTd0TT0iLCJTaWduYXR1cmUiOiJQL2NrenloVUJoSk5RQ24zMnluM1VTdGpLencxU04xNW9MclVhTU9XaW9scXBOTTBzNVFSNURHVEVDT1FzQk13ODdQdTc1TGE1OGtJTHRIcW1BVzhDQT09In0=";
     const auto encoded2 = "eyJBdXRob3JpemF0aW9uIjp7IklEIjoibFRSWnBXK1d3TFJqYkpzOGxBUFVaQS8zWnhmcGdwNDFQY0dkdlI5a0RVST0iLCJBY2Nlc3NUeXBlIjoic3BlZWQtYm9vc3QtdGVzdCIsIkV4cGlyZXMiOiIyMDE5LTAxLTE0VDIxOjQ2OjMwLjcxNzI2NTkyNFoifSwiU2lnbmluZ0tleUlEIjoiUUNZTzV2clIvZGhjRDZ6M2FMQlVNeWRuZlJyZFNRL1RWYW1IUFhYeTd0TT0iLCJTaWduYXR1cmUiOiJtV1Z5Tm9ZU0pFRDNXU3I3bG1OeEtReEZza1M5ZWlXWG1lcDVvVWZBSHkwVmYrSjZaQW9WajZrN3ZVTDNrakIreHZQSTZyaVhQc3FzWENRNkx0eFdBQT09In0=";
 
-    auto auth1 = psicash::DecodeAuthorization(encoded1);
-    auto auth2 = psicash::DecodeAuthorization(encoded2);
+    auto auth_res1 = psicash::DecodeAuthorization(encoded1);
+    auto auth_res2 = psicash::DecodeAuthorization(encoded2);
 
-    ASSERT_TRUE(auth1 == auth1);
-    ASSERT_FALSE(auth1 == auth2);
-    ASSERT_EQ(auth1.id, "0V3ExTviAtSqLfNwaiAyG4zZEBI8jHbzylWMyNEgRDg=");
-    ASSERT_EQ(auth1.access_type, "speed-boost-test");
+    ASSERT_TRUE(auth_res1);
+    ASSERT_TRUE(auth_res2);
+    ASSERT_TRUE(*auth_res1 == *auth_res1);
+    ASSERT_FALSE(*auth_res1 == *auth_res2);
+    ASSERT_EQ(auth_res1->id, "0V3ExTviAtSqLfNwaiAyG4zZEBI8jHbzylWMyNEgRDg=");
+    ASSERT_EQ(auth_res1->access_type, "speed-boost-test");
 
     psicash::datetime::DateTime want_date;
     ASSERT_TRUE(want_date.FromISO8601("2019-01-14T17:22:23.168764129Z"));
-    ASSERT_EQ(auth1.expires, want_date) << auth1.expires.ToISO8601();
+    ASSERT_EQ(auth_res1->expires, want_date) << auth_res1->expires.ToISO8601();
 
     auto invalid_base64 = "BAD-BASE64-$^#&*(@===============";
-    ASSERT_THROW(psicash::DecodeAuthorization(invalid_base64), json::exception);
+    auto auth_res_fail = psicash::DecodeAuthorization(invalid_base64);
+    ASSERT_FALSE(auth_res_fail);
+    ASSERT_TRUE(auth_res_fail.error().Critical());
 
     auto invalid_json = "dGhpcyBpcyBub3QgdmFsaWQgSlNPTg==";
-    ASSERT_THROW(psicash::DecodeAuthorization(invalid_json), json::exception);
+    auth_res_fail = psicash::DecodeAuthorization(invalid_json);
+    ASSERT_FALSE(auth_res_fail);
+    ASSERT_TRUE(auth_res_fail.error().Critical());
 
     auto incorrect_json = "eyJ2YWxpZCI6ICJqc29uIiwgImJ1dCI6ICJub3QgYSB2YWxpZCBhdXRob3JpemF0aW9uIn0=";
-    ASSERT_THROW(psicash::DecodeAuthorization(incorrect_json), json::exception);
+    auth_res_fail = psicash::DecodeAuthorization(incorrect_json);
+    ASSERT_FALSE(auth_res_fail);
+    ASSERT_TRUE(auth_res_fail.error().Critical());
 }
 
 TEST_F(TestPsiCash, ActiveAuthorizations) {
@@ -389,9 +398,14 @@ TEST_F(TestPsiCash, ActiveAuthorizations) {
     const auto encoded1 = "eyJBdXRob3JpemF0aW9uIjp7IklEIjoiMFYzRXhUdmlBdFNxTGZOd2FpQXlHNHpaRUJJOGpIYnp5bFdNeU5FZ1JEZz0iLCJBY2Nlc3NUeXBlIjoic3BlZWQtYm9vc3QtdGVzdCIsIkV4cGlyZXMiOiIyMDE5LTAxLTE0VDE3OjIyOjIzLjE2ODc2NDEyOVoifSwiU2lnbmluZ0tleUlEIjoiUUNZTzV2clIvZGhjRDZ6M2FMQlVNeWRuZlJyZFNRL1RWYW1IUFhYeTd0TT0iLCJTaWduYXR1cmUiOiJQL2NrenloVUJoSk5RQ24zMnluM1VTdGpLencxU04xNW9MclVhTU9XaW9scXBOTTBzNVFSNURHVEVDT1FzQk13ODdQdTc1TGE1OGtJTHRIcW1BVzhDQT09In0=";
     const auto encoded2 = "eyJBdXRob3JpemF0aW9uIjp7IklEIjoibFRSWnBXK1d3TFJqYkpzOGxBUFVaQS8zWnhmcGdwNDFQY0dkdlI5a0RVST0iLCJBY2Nlc3NUeXBlIjoic3BlZWQtYm9vc3QtdGVzdCIsIkV4cGlyZXMiOiIyMDE5LTAxLTE0VDIxOjQ2OjMwLjcxNzI2NTkyNFoifSwiU2lnbmluZ0tleUlEIjoiUUNZTzV2clIvZGhjRDZ6M2FMQlVNeWRuZlJyZFNRL1RWYW1IUFhYeTd0TT0iLCJTaWduYXR1cmUiOiJtV1Z5Tm9ZU0pFRDNXU3I3bG1OeEtReEZza1M5ZWlXWG1lcDVvVWZBSHkwVmYrSjZaQW9WajZrN3ZVTDNrakIreHZQSTZyaVhQc3FzWENRNkx0eFdBQT09In0=";
 
+    auto auth_res1 = psicash::DecodeAuthorization(encoded1);
+    auto auth_res2 = psicash::DecodeAuthorization(encoded2);
+    ASSERT_TRUE(auth_res1);
+    ASSERT_TRUE(auth_res2);
+
     purchases = {{"future_no_auth", "tc1", "d1", after_now, nonstd::nullopt, nonstd::nullopt},
-                 {"past_auth", "tc2", "d2", before_now, nonstd::nullopt, nonstd::make_optional(psicash::DecodeAuthorization(encoded1))},
-                 {"future_auth", "tc3", "d3", after_now, nonstd::nullopt, nonstd::make_optional(psicash::DecodeAuthorization(encoded2))}};
+                 {"past_auth", "tc2", "d2", before_now, nonstd::nullopt, *auth_res1},
+                 {"future_auth", "tc3", "d3", after_now, nonstd::nullopt, *auth_res2}};
 
     err = pc.user_data().SetPurchases(purchases);
     ASSERT_FALSE(err);

psicash_test.cpp

@@ -261,9 +261,11 @@ TEST_F(TestUserData, Purchases)
     // Set then get
     auto dt1 = datetime::DateTime::Now().Add(datetime::Duration(1));
     auto dt2 = datetime::DateTime::Now().Add(datetime::Duration(2));
-    auto auth1 = psicash::DecodeAuthorization("eyJBdXRob3JpemF0aW9uIjp7IklEIjoibFRSWnBXK1d3TFJqYkpzOGxBUFVaQS8zWnhmcGdwNDFQY0dkdlI5a0RVST0iLCJBY2Nlc3NUeXBlIjoic3BlZWQtYm9vc3QtdGVzdCIsIkV4cGlyZXMiOiIyMDE5LTAxLTE0VDIxOjQ2OjMwLjcxNzI2NTkyNFoifSwiU2lnbmluZ0tleUlEIjoiUUNZTzV2clIvZGhjRDZ6M2FMQlVNeWRuZlJyZFNRL1RWYW1IUFhYeTd0TT0iLCJTaWduYXR1cmUiOiJtV1Z5Tm9ZU0pFRDNXU3I3bG1OeEtReEZza1M5ZWlXWG1lcDVvVWZBSHkwVmYrSjZaQW9WajZrN3ZVTDNrakIreHZQSTZyaVhQc3FzWENRNkx0eFdBQT09In0=");
+    auto auth_res1 = psicash::DecodeAuthorization("eyJBdXRob3JpemF0aW9uIjp7IklEIjoibFRSWnBXK1d3TFJqYkpzOGxBUFVaQS8zWnhmcGdwNDFQY0dkdlI5a0RVST0iLCJBY2Nlc3NUeXBlIjoic3BlZWQtYm9vc3QtdGVzdCIsIkV4cGlyZXMiOiIyMDE5LTAxLTE0VDIxOjQ2OjMwLjcxNzI2NTkyNFoifSwiU2lnbmluZ0tleUlEIjoiUUNZTzV2clIvZGhjRDZ6M2FMQlVNeWRuZlJyZFNRL1RWYW1IUFhYeTd0TT0iLCJTaWduYXR1cmUiOiJtV1Z5Tm9ZU0pFRDNXU3I3bG1OeEtReEZza1M5ZWlXWG1lcDVvVWZBSHkwVmYrSjZaQW9WajZrN3ZVTDNrakIreHZQSTZyaVhQc3FzWENRNkx0eFdBQT09In0=");
+    ASSERT_TRUE(auth_res1);
+
     Purchases want = {
-        {"id1", "tc1", "d1", dt1, dt2, auth1},
+        {"id1", "tc1", "d1", dt1, dt2, *auth_res1},
         {"id2", "tc2", "d2", nullopt, nullopt, nullopt}};
 
     err = ud.SetPurchases(want);