From 8b1dfd73435fb06ac6f87f28d2ee2aab8df5d8e4 Mon Sep 17 00:00:00 2001
From: Christian Blichmann <cblichmann@google.com>
Date: Fri, 30 Jul 2021 03:54:45 -0700
Subject: [PATCH] Fix factory method `sapi::v::Proto<>::FromMessage`

This was missing a friend declaration in order to actually compile.
It's now being used in the "stringop" example, so we test it as well.

Drive-by:
- Do not copy the proto's bytes the constructor, but use `std::move`
PiperOrigin-RevId: 387774353
Change-Id: Ic8824af911ac744e2e68130e1f4673c4dddd4939
---
 sandboxed_api/BUILD.bazel                        |  1 +
 sandboxed_api/CMakeLists.txt                     |  1 +
 sandboxed_api/examples/stringop/main_stringop.cc |  6 +++---
 sandboxed_api/var_proto.h                        | 11 +++++++----
 4 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/sandboxed_api/BUILD.bazel b/sandboxed_api/BUILD.bazel
index 0a6fa5d..c93271e 100644
--- a/sandboxed_api/BUILD.bazel
+++ b/sandboxed_api/BUILD.bazel
@@ -164,6 +164,7 @@ cc_library(
         "@com_google_absl//absl/strings",
         "@com_google_absl//absl/strings:str_format",
         "@com_google_absl//absl/synchronization",
+        "@com_google_absl//absl/utility",
         "@com_google_glog//:glog",
     ],
 )
diff --git a/sandboxed_api/CMakeLists.txt b/sandboxed_api/CMakeLists.txt
index 220aae8..ab94d65 100644
--- a/sandboxed_api/CMakeLists.txt
+++ b/sandboxed_api/CMakeLists.txt
@@ -156,6 +156,7 @@ target_link_libraries(sapi_vars
           absl::str_format
           absl::strings
           absl::synchronization
+          absl::utility
           sandbox2::comms
           sapi::base
           sapi::call
diff --git a/sandboxed_api/examples/stringop/main_stringop.cc b/sandboxed_api/examples/stringop/main_stringop.cc
index 7556114..6970afa 100644
--- a/sandboxed_api/examples/stringop/main_stringop.cc
+++ b/sandboxed_api/examples/stringop/main_stringop.cc
@@ -69,12 +69,12 @@ TEST(StringopTest, ProtobufStringReversal) {
 
   stringop::StringReverse proto;
   proto.set_input("Hello");
-  sapi::v::Proto<stringop::StringReverse> pp(proto);
+  auto pp = sapi::v::Proto<stringop::StringReverse>::FromMessage(proto);
   SAPI_ASSERT_OK_AND_ASSIGN(int return_value,
-                            api.pb_reverse_string(pp.PtrBoth()));
+                            api.pb_reverse_string(pp->PtrBoth()));
   EXPECT_THAT(return_value, Ne(0)) << "pb_reverse_string() failed";
 
-  SAPI_ASSERT_OK_AND_ASSIGN(auto pb_result, pp.GetMessage());
+  SAPI_ASSERT_OK_AND_ASSIGN(auto pb_result, pp->GetMessage());
   LOG(INFO) << "Result PB: " << pb_result.DebugString();
   EXPECT_THAT(pb_result.output(), StrEq("olleH"));
 }
diff --git a/sandboxed_api/var_proto.h b/sandboxed_api/var_proto.h
index 33753a3..b726668 100644
--- a/sandboxed_api/var_proto.h
+++ b/sandboxed_api/var_proto.h
@@ -24,6 +24,7 @@
 #include "absl/base/macros.h"
 #include "absl/memory/memory.h"
 #include "absl/status/statusor.h"
+#include "absl/utility/utility.h"
 #include "sandboxed_api/proto_helper.h"
 #include "sandboxed_api/util/status_macros.h"
 #include "sandboxed_api/var_lenval.h"
@@ -44,7 +45,7 @@ class Proto : public Pointable, public Var {
 
   static absl::StatusOr<Proto<T>> FromMessage(const T& proto) {
     SAPI_ASSIGN_OR_RETURN(std::vector<uint8_t> len_val, SerializeProto(proto));
-    return Proto(len_val);
+    return absl::StatusOr<Proto<T>>(absl::in_place, proto);
   }
 
   size_t GetSize() const final { return wrapped_var_.GetSize(); }
@@ -68,8 +69,8 @@ class Proto : public Pointable, public Var {
 
   ABSL_DEPRECATED("Use GetMessage() instead")
   std::unique_ptr<T> GetProtoCopy() const {
-    if (auto result_or = GetMessage(); result_or.ok()) {
-      return absl::make_unique<T>(std::move(result_or).value());
+    if (auto proto = GetMessage(); proto.ok()) {
+      return absl::make_unique<T>(*std::move(proto));
     }
     return nullptr;
   }
@@ -102,7 +103,9 @@ class Proto : public Pointable, public Var {
   }
 
  private:
-  explicit Proto(std::vector<uint8_t> data) : wrapped_var_(data) {}
+  friend class absl::StatusOr<Proto<T>>;
+
+  explicit Proto(std::vector<uint8_t> data) : wrapped_var_(std::move(data)) {}
 
   // The management of reading/writing the data to the sandboxee is handled by
   // the LenVal class.