diff --git a/sandboxed_api/client.cc b/sandboxed_api/client.cc index f76e067..df2b24d 100644 --- a/sandboxed_api/client.cc +++ b/sandboxed_api/client.cc @@ -122,7 +122,7 @@ class FunctionCallPreparer { // There is no way to figure out whether the protobuf structure has // changed or not, so we always serialize the protobuf again and replace // the LenValStruct content. - std::vector serialized = SerializeProto(*proto).ValueOrDie(); + std::vector serialized = SerializeProto(*proto).value(); // Reallocate the LV memory to match its length. if (lvs->size != serialized.size()) { void* newdata = realloc(lvs->data, serialized.size()); diff --git a/sandboxed_api/examples/zlib/main_zlib.cc b/sandboxed_api/examples/zlib/main_zlib.cc index 24f433e..30e5042 100644 --- a/sandboxed_api/examples/zlib/main_zlib.cc +++ b/sandboxed_api/examples/zlib/main_zlib.cc @@ -15,6 +15,8 @@ #include #include +#include + #include #include "absl/base/macros.h" #include "sandboxed_api/util/flag.h" @@ -40,11 +42,13 @@ int main(int argc, char** argv) { sapi::Sandbox sandbox(sapi::zlib::zlib_sapi_embed_create()); sapi::zlib::ZlibApi api(&sandbox); - if (!sandbox.Init().ok()) { - LOG(FATAL) << "Couldn't initialize the Sandboxed API Lib"; + if (auto status = sandbox.Init(); !status.ok()) { + LOG(FATAL) << "Couldn't initialize Sandboxed API for zlib: " + << status.message(); } - int ret, flush; + sapi::StatusOr ret; + int flush; unsigned have; sapi::v::Struct strm; @@ -64,11 +68,10 @@ int main(int argc, char** argv) { // Allocate deflate state. *strm.mutable_data() = sapi::zlib::z_stream{}; - ret = api.deflateInit_(strm.PtrBoth(), Z_DEFAULT_COMPRESSION, - version.PtrBefore(), sizeof(sapi::zlib::z_stream)) - .ValueOrDie(); - if (ret != Z_OK) { - return ret; + if (ret = api.deflateInit_(strm.PtrBoth(), Z_DEFAULT_COMPRESSION, + version.PtrBefore(), sizeof(sapi::zlib::z_stream)); + *ret != Z_OK) { + return *ret; } LOG(INFO) << "Starting decompression"; @@ -81,7 +84,7 @@ int main(int argc, char** argv) { } if (ferror(stdin)) { LOG(INFO) << "Error reading from stdin"; - (void)api.deflateEnd(strm.PtrBoth()).ValueOrDie(); + api.deflateEnd(strm.PtrBoth()).IgnoreError(); return Z_ERRNO; } flush = feof(stdin) ? Z_FINISH : Z_NO_FLUSH; @@ -95,10 +98,8 @@ int main(int argc, char** argv) { strm.mutable_data()->next_out = reinterpret_cast(output.GetRemote()); - ret = api.deflate(strm.PtrBoth(), flush) - .ValueOrDie(); // no bad return value. - - assert(ret != Z_STREAM_ERROR); // state not clobbered. + ret = api.deflate(strm.PtrBoth(), flush); // no bad return value. + assert(*ret != Z_STREAM_ERROR); // state not clobbered. have = kChunk - strm.data().avail_out; if (!sandbox.TransferFromSandboxee(&output).ok()) { @@ -107,7 +108,7 @@ int main(int argc, char** argv) { if (fwrite(output.GetLocal(), 1, have, stdout) != have || ferror(stdout)) { // Not really necessary as strm did not change from last transfer. - (void)api.deflateEnd(strm.PtrBoth()).ValueOrDie(); + api.deflateEnd(strm.PtrBoth()).IgnoreError(); return Z_ERRNO; } } while (strm.data().avail_out == 0); @@ -115,10 +116,10 @@ int main(int argc, char** argv) { // done when last data in file processed. } while (flush != Z_FINISH); - assert(ret == Z_STREAM_END); // stream will be complete. + assert(*ret == Z_STREAM_END); // stream will be complete. // clean up and return. - (void)api.deflateEnd(strm.PtrBoth()).ValueOrDie(); + api.deflateEnd(strm.PtrBoth()).IgnoreError(); - return 0; + return EXIT_SUCCESS; } diff --git a/sandboxed_api/rpcchannel.cc b/sandboxed_api/rpcchannel.cc index cd57c6f..72a9250 100644 --- a/sandboxed_api/rpcchannel.cc +++ b/sandboxed_api/rpcchannel.cc @@ -96,9 +96,7 @@ absl::Status RPCChannel::Reallocate(void* old_addr, size_t size, absl::StrCat("Reallocate() failed on the remote side: ", fret_or.status().message())); } - auto fret = std::move(fret_or).ValueOrDie(); - - *new_addr = reinterpret_cast(fret.int_val); + *new_addr = reinterpret_cast(fret_or->int_val); return absl::OkStatus(); } diff --git a/sandboxed_api/sandbox2/examples/network_proxy/networkproxy_bin.cc b/sandboxed_api/sandbox2/examples/network_proxy/networkproxy_bin.cc index 779480f..c22947d 100644 --- a/sandboxed_api/sandbox2/examples/network_proxy/networkproxy_bin.cc +++ b/sandboxed_api/sandbox2/examples/network_proxy/networkproxy_bin.cc @@ -26,7 +26,7 @@ ABSL_FLAG(bool, connect_with_handler, true, "Connect using automatic mode."); namespace { -sandbox2::NetworkProxyClient* proxy_client; +static sandbox2::NetworkProxyClient* g_proxy_client; ssize_t ReadFromFd(int fd, uint8_t* buf, size_t size) { ssize_t received = 0; @@ -72,7 +72,7 @@ sapi::StatusOr CreateAddres(int port) { } absl::Status ConnectManually(int s, const struct sockaddr_in6& saddr) { - return proxy_client->Connect( + return g_proxy_client->Connect( s, reinterpret_cast(&saddr), sizeof(saddr)); } @@ -118,19 +118,19 @@ int main(int argc, char** argv) { sandbox2_client.SandboxMeHere(); if (absl::GetFlag(FLAGS_connect_with_handler)) { - absl::Status status = sandbox2_client.InstallNetworkProxyHandler(); - if (!status.ok()) { + if (auto status = sandbox2_client.InstallNetworkProxyHandler(); + !status.ok()) { LOG(ERROR) << "InstallNetworkProxyHandler() failed: " << status.message(); return 1; } } else { - proxy_client = sandbox2_client.GetNetworkProxyClient(); + g_proxy_client = sandbox2_client.GetNetworkProxyClient(); } // Receive port number of the server int port; if (!comms.RecvInt32(&port)) { - LOG(ERROR) << "sandboxee_comms->RecvUint32(&crc4) failed"; + LOG(ERROR) << "Failed to receive port number"; return 2; } @@ -139,13 +139,11 @@ int main(int argc, char** argv) { LOG(ERROR) << sock_s.status().message(); return 3; } - sandbox2::file_util::fileops::FDCloser client{sock_s.ValueOrDie()}; + sandbox2::file_util::fileops::FDCloser client(sock_s.value()); - absl::Status status = CommunicationTest(client.get()); - if (!status.ok()) { - LOG(ERROR) << sock_s.status().message(); + if (auto status = CommunicationTest(client.get()); !status.ok()) { + LOG(ERROR) << status.message(); return 4; } - return 0; } diff --git a/sandboxed_api/sandbox2/forkserver.cc b/sandboxed_api/sandbox2/forkserver.cc index 75b29f4..7b09a1a 100644 --- a/sandboxed_api/sandbox2/forkserver.cc +++ b/sandboxed_api/sandbox2/forkserver.cc @@ -458,13 +458,10 @@ pid_t ForkServer::ServeRequest() { // Send sandboxee pid absl::Status status = SendPid(fd_closer1.get()); SAPI_RAW_CHECK(status.ok(), "sending pid: %s", status.message()); + } else if (auto pid_or = ReceivePid(fd_closer0.get()); !pid_or.ok()) { + SAPI_RAW_LOG(ERROR, "receiving pid: %s", pid_or.status().message()); } else { - auto pid_or = ReceivePid(fd_closer0.get()); - if (!pid_or.ok()) { - SAPI_RAW_LOG(ERROR, "receiving pid: %s", pid_or.status().message()); - } else { - sandboxee_pid = pid_or.ValueOrDie(); - } + sandboxee_pid = pid_or.value(); } } else { sandboxee_pid = util::ForkWithFlags(clone_flags); @@ -493,13 +490,12 @@ pid_t ForkServer::ServeRequest() { sandboxee_pid = -1; // And the actual sandboxee is forked from the init process, so we need to // receive the actual PID. - auto pid_or = ReceivePid(fd_closer0.get()); - if (!pid_or.ok()) { + if (auto pid_or = ReceivePid(fd_closer0.get()); !pid_or.ok()) { SAPI_RAW_LOG(ERROR, "%s", pid_or.status().message()); kill(init_pid, SIGKILL); init_pid = -1; } else { - sandboxee_pid = pid_or.ValueOrDie(); + sandboxee_pid = pid_or.value(); } } diff --git a/sandboxed_api/sandbox2/mounts.cc b/sandboxed_api/sandbox2/mounts.cc index 9eb0b6e..2f734e2 100644 --- a/sandboxed_api/sandbox2/mounts.cc +++ b/sandboxed_api/sandbox2/mounts.cc @@ -124,9 +124,9 @@ absl::Status ValidateInterpreter(absl::string_view interpreter) { std::string ResolveLibraryPath(absl::string_view lib_name, const std::vector& search_paths) { for (const auto& search_path : search_paths) { - auto path_or = ExistingPathInsideDir(search_path, lib_name); - if (path_or.ok()) { - return path_or.ValueOrDie(); + if (auto path_or = ExistingPathInsideDir(search_path, lib_name); + path_or.ok()) { + return path_or.value(); } } return ""; @@ -290,14 +290,10 @@ void LogContainer(const std::vector& container) { absl::Status Mounts::AddMappingsForBinary(const std::string& path, absl::string_view ld_library_path) { - auto elf_or = ElfFile::ParseFromFile( - path, ElfFile::kGetInterpreter | ElfFile::kLoadImportedLibraries); - if (!elf_or.ok()) { - return absl::FailedPreconditionError( - absl::StrCat("Could not parse ELF file: ", elf_or.status().message())); - } - auto elf = elf_or.ValueOrDie(); - const std::string interpreter = elf.interpreter(); + SAPI_ASSIGN_OR_RETURN(auto elf, ElfFile::ParseFromFile( + path, ElfFile::kGetInterpreter | + ElfFile::kLoadImportedLibraries)); + const std::string& interpreter = elf.interpreter(); if (interpreter.empty()) { SAPI_RAW_VLOG(1, "The file %s is not a dynamic executable", path); diff --git a/sandboxed_api/sandbox2/mounts_test.cc b/sandboxed_api/sandbox2/mounts_test.cc index 22241d7..b3d9e61 100644 --- a/sandboxed_api/sandbox2/mounts_test.cc +++ b/sandboxed_api/sandbox2/mounts_test.cc @@ -88,16 +88,16 @@ TEST(MountTreeTest, TestAddTmpFs) { TEST(MountTreeTest, TestMultipleInsertionFileSymlink) { Mounts mounts; - auto result_or = CreateNamedTempFileAndClose( - file::JoinPath(GetTestTempPath(), "testdir_")); - ASSERT_THAT(result_or.status(), IsOk()); - std::string path = result_or.ValueOrDie(); - result_or = CreateNamedTempFileAndClose( - file::JoinPath(GetTestTempPath(), "testdir_")); - ASSERT_THAT(result_or.status(), IsOk()); - std::string symlink_path = result_or.ValueOrDie(); + SAPI_ASSERT_OK_AND_ASSIGN(std::string path, + CreateNamedTempFileAndClose( + file::JoinPath(GetTestTempPath(), "testdir_"))); + SAPI_ASSERT_OK_AND_ASSIGN(std::string symlink_path, + CreateNamedTempFileAndClose( + file::JoinPath(GetTestTempPath(), "testdir_"))); + ASSERT_THAT(unlink(symlink_path.c_str()), Eq(0)); ASSERT_THAT(symlink(path.c_str(), symlink_path.c_str()), Eq(0)); + EXPECT_THAT(mounts.AddFileAt(path, "/a"), IsOk()); EXPECT_THAT(mounts.AddFileAt(path, "/a"), IsOk()); EXPECT_THAT(mounts.AddFileAt(symlink_path, "/a"), IsOk()); @@ -106,15 +106,15 @@ TEST(MountTreeTest, TestMultipleInsertionFileSymlink) { TEST(MountTreeTest, TestMultipleInsertionDirSymlink) { Mounts mounts; - auto result_or = CreateTempDir(file::JoinPath(GetTestTempPath(), "testdir_")); - ASSERT_THAT(result_or.status(), IsOk()); - std::string path = result_or.ValueOrDie(); - result_or = CreateNamedTempFileAndClose( - file::JoinPath(GetTestTempPath(), "testdir_")); - ASSERT_THAT(result_or.status(), IsOk()); - std::string symlink_path = result_or.ValueOrDie(); + SAPI_ASSERT_OK_AND_ASSIGN(std::string path, CreateTempDir(file::JoinPath( + GetTestTempPath(), "testdir_"))); + SAPI_ASSERT_OK_AND_ASSIGN(std::string symlink_path, + CreateNamedTempFileAndClose( + file::JoinPath(GetTestTempPath(), "testdir_"))); + ASSERT_THAT(unlink(symlink_path.c_str()), Eq(0)); ASSERT_THAT(symlink(path.c_str(), symlink_path.c_str()), Eq(0)); + EXPECT_THAT(mounts.AddDirectoryAt(path, "/a"), IsOk()); EXPECT_THAT(mounts.AddDirectoryAt(path, "/a"), IsOk()); EXPECT_THAT(mounts.AddDirectoryAt(symlink_path, "/a"), IsOk()); @@ -144,7 +144,7 @@ TEST(MountTreeTest, TestEvilNullByte) { Mounts mounts; // create the filename with a null byte this way as g4 fix forces newlines // otherwise. - std::string filename{"/a/b"}; + std::string filename = "/a/b"; filename[2] = '\0'; EXPECT_THAT(mounts.AddFile(filename), diff --git a/sandboxed_api/sandbox2/namespace_test.cc b/sandboxed_api/sandbox2/namespace_test.cc index d2461f0..89da97b 100644 --- a/sandboxed_api/sandbox2/namespace_test.cc +++ b/sandboxed_api/sandbox2/namespace_test.cc @@ -62,9 +62,8 @@ TEST(NamespaceTest, FileNamespaceWorks) { TEST(NamespaceTest, ReadOnlyIsRespected) { // Mount temporary file as RO and check that it actually is RO. - auto [name, fd] = - CreateNamedTempFile(GetTestTempPath("temp_file")).ValueOrDie(); - file_util::fileops::FDCloser temp_closer{fd}; + auto [name, fd] = CreateNamedTempFile(GetTestTempPath("temp_file")).value(); + file_util::fileops::FDCloser temp_closer(fd); const std::string path = GetTestSourcePath("sandbox2/testcases/namespace"); { diff --git a/sandboxed_api/sandbox2/network_proxy/server.cc b/sandboxed_api/sandbox2/network_proxy/server.cc index c456306..c183812 100644 --- a/sandboxed_api/sandbox2/network_proxy/server.cc +++ b/sandboxed_api/sandbox2/network_proxy/server.cc @@ -104,13 +104,13 @@ void NetworkProxyServer::NotifySuccess() { } void NetworkProxyServer::NotifyViolation(const struct sockaddr* saddr) { - sapi::StatusOr result = AddrToString(saddr); - if (result.ok()) { - violation_msg_ = result.ValueOrDie(); + if (sapi::StatusOr result = AddrToString(saddr); result.ok()) { + violation_msg_ = std::move(result).value(); } else { violation_msg_ = std::string(result.status().message()); } violation_occurred_.store(true, std::memory_order_release); pthread_kill(monitor_thread_id_, SIGCHLD); } + } // namespace sandbox2 diff --git a/sandboxed_api/sandbox2/policybuilder.cc b/sandboxed_api/sandbox2/policybuilder.cc index 4161c3a..d6f9a58 100644 --- a/sandboxed_api/sandbox2/policybuilder.cc +++ b/sandboxed_api/sandbox2/policybuilder.cc @@ -723,7 +723,7 @@ PolicyBuilder& PolicyBuilder::AddFileAt(absl::string_view outside, SetError(fixed_outside_or.status()); return *this; } - auto fixed_outside = std::move(fixed_outside_or.ValueOrDie()); + auto fixed_outside = std::move(fixed_outside_or).value(); if (absl::StartsWith(fixed_outside, "/proc/self")) { SetError(absl::InvalidArgumentError( @@ -733,13 +733,12 @@ PolicyBuilder& PolicyBuilder::AddFileAt(absl::string_view outside, return *this; } - auto status = mounts_.AddFileAt(fixed_outside, inside, is_ro); - if (!status.ok()) { + if (auto status = mounts_.AddFileAt(fixed_outside, inside, is_ro); + !status.ok()) { SetError( absl::InternalError(absl::StrCat("Could not add file ", outside, " => ", inside, ": ", status.message()))); } - return *this; } @@ -752,10 +751,10 @@ PolicyBuilder& PolicyBuilder::AddLibrariesForBinary( SetError(fixed_path_or.status()); return *this; } - auto fixed_path = std::move(fixed_path_or.ValueOrDie()); + auto fixed_path = std::move(fixed_path_or).value(); - auto status = mounts_.AddMappingsForBinary(fixed_path, ld_library_path); - if (!status.ok()) { + if (auto status = mounts_.AddMappingsForBinary(fixed_path, ld_library_path); + !status.ok()) { SetError(absl::InternalError(absl::StrCat( "Could not add libraries for ", fixed_path, ": ", status.message()))); } @@ -782,7 +781,7 @@ PolicyBuilder& PolicyBuilder::AddDirectoryAt(absl::string_view outside, SetError(fixed_outside_or.status()); return *this; } - auto fixed_outside = std::move(fixed_outside_or.ValueOrDie()); + auto fixed_outside = std::move(fixed_outside_or).value(); if (absl::StartsWith(fixed_outside, "/proc/self")) { SetError(absl::InvalidArgumentError( absl::StrCat("Cannot add /proc/self mounts, you need to mount the " @@ -791,25 +790,22 @@ PolicyBuilder& PolicyBuilder::AddDirectoryAt(absl::string_view outside, return *this; } - auto status = mounts_.AddDirectoryAt(fixed_outside, inside, is_ro); - if (!status.ok()) { + if (auto status = mounts_.AddDirectoryAt(fixed_outside, inside, is_ro); + !status.ok()) { SetError(absl::InternalError(absl::StrCat("Could not add directory ", outside, " => ", inside, ": ", status.message()))); } - return *this; } PolicyBuilder& PolicyBuilder::AddTmpfs(absl::string_view inside, size_t sz) { EnableNamespaces(); - auto status = mounts_.AddTmpfs(inside, sz); - if (!status.ok()) { + if (auto status = mounts_.AddTmpfs(inside, sz); !status.ok()) { SetError(absl::InternalError(absl::StrCat("Could not mount tmpfs ", inside, ": ", status.message()))); } - return *this; } diff --git a/sandboxed_api/sandbox2/policybuilder.h b/sandboxed_api/sandbox2/policybuilder.h index d3bf6d6..4dcc095 100644 --- a/sandboxed_api/sandbox2/policybuilder.h +++ b/sandboxed_api/sandbox2/policybuilder.h @@ -390,7 +390,7 @@ class PolicyBuilder final { // once. // This function will abort if an error happened in any off the PolicyBuilder // methods. - std::unique_ptr BuildOrDie() { return TryBuild().ValueOrDie(); } + std::unique_ptr BuildOrDie() { return TryBuild().value(); } // Adds a bind-mount for a file from outside the namespace to inside. This // will also create parent directories inside the namespace if needed. diff --git a/sandboxed_api/sandbox2/policybuilder_test.cc b/sandboxed_api/sandbox2/policybuilder_test.cc index 4b72aa3..a86befd 100644 --- a/sandboxed_api/sandbox2/policybuilder_test.cc +++ b/sandboxed_api/sandbox2/policybuilder_test.cc @@ -142,17 +142,15 @@ TEST_F(PolicyBuilderTest, TestValidateAbsolutePath) { "/a/b/c/d/", "/a/bAAAAAAAAAAAAAAAAAAAAAA/c/d/", }) { - auto path_or = PolicyBuilderPeer::ValidateAbsolutePath(bad_path); - EXPECT_THAT(path_or.status(), StatusIs(absl::StatusCode::kInvalidArgument)); + EXPECT_THAT(PolicyBuilderPeer::ValidateAbsolutePath(bad_path), + StatusIs(absl::StatusCode::kInvalidArgument)); } for (auto const& good_path : {"/", "/a/b/c/d", "/a/b/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"}) { - auto path_or = PolicyBuilderPeer::ValidateAbsolutePath(good_path); - EXPECT_THAT(path_or, IsOk()); - if (path_or.ok()) { - EXPECT_THAT(path_or.ValueOrDie(), StrEq(good_path)); - } + SAPI_ASSERT_OK_AND_ASSIGN(std::string path, + PolicyBuilderPeer::ValidateAbsolutePath(good_path)); + EXPECT_THAT(path, StrEq(good_path)); } } diff --git a/sandboxed_api/sandbox2/sandbox2.cc b/sandboxed_api/sandbox2/sandbox2.cc index 78e6d0b..559bceb 100644 --- a/sandboxed_api/sandbox2/sandbox2.cc +++ b/sandboxed_api/sandbox2/sandbox2.cc @@ -58,7 +58,7 @@ sapi::StatusOr Sandbox2::AwaitResultWithTimeout( } Result Sandbox2::AwaitResult() { - return AwaitResultWithTimeout(absl::InfiniteDuration()).ValueOrDie(); + return AwaitResultWithTimeout(absl::InfiniteDuration()).value(); } bool Sandbox2::RunAsync() { diff --git a/sandboxed_api/sandbox2/stack_trace.cc b/sandboxed_api/sandbox2/stack_trace.cc index 6b8fd32..182534b 100644 --- a/sandboxed_api/sandbox2/stack_trace.cc +++ b/sandboxed_api/sandbox2/stack_trace.cc @@ -145,7 +145,7 @@ std::unique_ptr StackTracePeer::GetPolicy(pid_t target_pid, LOG(ERROR) << "Creating stack unwinder sandbox policy failed"; return nullptr; } - std::unique_ptr policy = std::move(policy_or).ValueOrDie(); + std::unique_ptr policy = std::move(policy_or).value(); auto keep_capabilities = absl::make_unique>(); keep_capabilities->push_back(CAP_SYS_PTRACE); policy->AllowUnsafeKeepCapabilities(std::move(keep_capabilities)); diff --git a/sandboxed_api/sandbox2/stack_trace_test.cc b/sandboxed_api/sandbox2/stack_trace_test.cc index 39e52d1..bbc198a 100644 --- a/sandboxed_api/sandbox2/stack_trace_test.cc +++ b/sandboxed_api/sandbox2/stack_trace_test.cc @@ -39,13 +39,13 @@ ABSL_DECLARE_FLAG(bool, sandbox_libunwind_crash_handler); +namespace sandbox2 { +namespace { + using ::testing::Eq; using ::testing::HasSubstr; using ::testing::Not; -namespace sandbox2 { -namespace { - // Temporarily overrides a flag, restores the original flag value when it goes // out of scope. template @@ -71,7 +71,7 @@ void SymbolizationWorksCommon( std::vector args = {path, "1"}; auto executor = absl::make_unique(path, args); - std::string temp_filename = CreateNamedTempFileAndClose("/tmp/").ValueOrDie(); + std::string temp_filename = CreateNamedTempFileAndClose("/tmp/").value(); file_util::fileops::CopyFile("/proc/cpuinfo", temp_filename, 0444); struct TempCleanup { ~TempCleanup() { remove(capture->c_str()); } diff --git a/sandboxed_api/sandbox2/syscall_defs.cc b/sandboxed_api/sandbox2/syscall_defs.cc index c4b2952..3613375 100644 --- a/sandboxed_api/sandbox2/syscall_defs.cc +++ b/sandboxed_api/sandbox2/syscall_defs.cc @@ -60,7 +60,7 @@ std::string SyscallTable::Entry::GetArgumentDescription(uint64_t value, case kPath: if (auto path_or = util::ReadCPathFromPid(pid, value); path_or.ok()) { absl::StrAppendFormat(&ret, " ['%s']", - absl::CHexEscape(path_or.ValueOrDie())); + absl::CHexEscape(path_or.value())); } else { absl::StrAppend(&ret, " [unreadable path]"); } diff --git a/sandboxed_api/sandbox2/testcases/buffer.cc b/sandboxed_api/sandbox2/testcases/buffer.cc index de4c548..01fbeec 100644 --- a/sandboxed_api/sandbox2/testcases/buffer.cc +++ b/sandboxed_api/sandbox2/testcases/buffer.cc @@ -29,13 +29,12 @@ int main(int argc, char** argv) { int testno = atoi(argv[1]); // NOLINT switch (testno) { - case 1: // Dup and map to static FD - { + case 1: { // Dup and map to static FD auto buffer_or = sandbox2::Buffer::CreateFromFd(3); if (!buffer_or) { return EXIT_FAILURE; } - auto buffer = std::move(buffer_or).ValueOrDie(); + auto buffer = std::move(buffer_or).value(); uint8_t* buf = buffer->data(); // Test that we can read data from the executor. if (buf[0] != 'A') { @@ -46,8 +45,7 @@ int main(int argc, char** argv) { return EXIT_SUCCESS; } - case 2: // Send and receive FD - { + case 2: { // Send and receive FD sandbox2::Comms comms(sandbox2::Comms::kSandbox2ClientCommsFD); int fd; if (!comms.RecvFD(&fd)) { @@ -57,7 +55,7 @@ int main(int argc, char** argv) { if (!buffer_or) { return EXIT_FAILURE; } - auto buffer = std::move(buffer_or).ValueOrDie(); + auto buffer = std::move(buffer_or).value(); uint8_t* buf = buffer->data(); // Test that we can read data from the executor. if (buf[0] != 'A') { @@ -70,6 +68,6 @@ int main(int argc, char** argv) { default: absl::FPrintF(stderr, "Unknown test: %d\n", testno); - return EXIT_FAILURE; } + return EXIT_FAILURE; } diff --git a/sandboxed_api/sandbox2/testcases/symbolize.cc b/sandboxed_api/sandbox2/testcases/symbolize.cc index 606b473..59c79bb 100644 --- a/sandboxed_api/sandbox2/testcases/symbolize.cc +++ b/sandboxed_api/sandbox2/testcases/symbolize.cc @@ -40,7 +40,7 @@ void RunWritable() { std::string tmpname; int tmp_fd; - std::tie(tmpname, tmp_fd) = sandbox2::CreateNamedTempFile("tmp").ValueOrDie(); + std::tie(tmpname, tmp_fd) = sandbox2::CreateNamedTempFile("tmp").value(); SAPI_RAW_PCHECK(fchmod(tmp_fd, S_IRWXU) == 0, "Fchmod on temporary file"); char buf[4096]; diff --git a/sandboxed_api/sandbox2/unwind/unwind.cc b/sandboxed_api/sandbox2/unwind/unwind.cc index 10050be..781156c 100644 --- a/sandboxed_api/sandbox2/unwind/unwind.cc +++ b/sandboxed_api/sandbox2/unwind/unwind.cc @@ -165,7 +165,7 @@ void RunLibUnwindAndSymbolizer(pid_t pid, std::string* stack_trace_out, SAPI_RAW_LOG(ERROR, "Could not parse /proc/%d/maps", pid); return; } - auto maps = std::move(maps_or).ValueOrDie(); + auto maps = std::move(maps_or).value(); // Get symbols for each file entry in the maps entry. // This is not a very efficient way, so we might want to optimize it. @@ -190,7 +190,7 @@ void RunLibUnwindAndSymbolizer(pid_t pid, std::string* stack_trace_out, elf_or.status().message()); continue; } - auto elf = std::move(elf_or).ValueOrDie(); + auto elf = std::move(elf_or).value(); for (const auto& symbol : elf.symbols()) { if (elf.position_independent()) { diff --git a/sandboxed_api/sandbox2/util/minielf_test.cc b/sandboxed_api/sandbox2/util/minielf_test.cc index d601d8b..db6b188 100644 --- a/sandboxed_api/sandbox2/util/minielf_test.cc +++ b/sandboxed_api/sandbox2/util/minielf_test.cc @@ -23,12 +23,6 @@ #include "sandboxed_api/sandbox2/util/maps_parser.h" #include "sandboxed_api/util/status_matchers.h" -using sapi::IsOk; -using ::testing::Eq; -using ::testing::IsTrue; -using ::testing::Not; -using ::testing::StrEq; - extern "C" void ExportedFunctionName() { // Don't do anything - used to generate a symbol. } @@ -36,6 +30,11 @@ extern "C" void ExportedFunctionName() { namespace sandbox2 { namespace { +using ::testing::Eq; +using ::testing::IsTrue; +using ::testing::Not; +using ::testing::StrEq; + TEST(MinielfTest, Chrome70) { SAPI_ASSERT_OK_AND_ASSIGN( ElfFile elf, @@ -58,9 +57,7 @@ TEST(MinielfTest, SymbolResolutionWorks) { fread(maps_buffer, 1, sizeof(maps_buffer), f); fclose(f); - auto maps_or = ParseProcMaps(maps_buffer); - ASSERT_THAT(maps_or.status(), IsOk()); - std::vector maps = maps_or.ValueOrDie(); + SAPI_ASSERT_OK_AND_ASSIGN(std::vector maps, ParseProcMaps(maps_buffer)); // Find maps entry that covers this entry. uint64_t function_address = reinterpret_cast(ExportedFunctionName); diff --git a/sandboxed_api/sandbox2/util/temp_file.cc b/sandboxed_api/sandbox2/util/temp_file.cc index a9a363e..9caac66 100644 --- a/sandboxed_api/sandbox2/util/temp_file.cc +++ b/sandboxed_api/sandbox2/util/temp_file.cc @@ -48,7 +48,7 @@ sapi::StatusOr CreateNamedTempFileAndClose( if (result_or.ok()) { std::string path; int fd; - std::tie(path, fd) = result_or.ValueOrDie(); + std::tie(path, fd) = std::move(result_or).value(); close(fd); return path; } diff --git a/sandboxed_api/sandbox2/util/temp_file_test.cc b/sandboxed_api/sandbox2/util/temp_file_test.cc index 2ff115c..7172b68 100644 --- a/sandboxed_api/sandbox2/util/temp_file_test.cc +++ b/sandboxed_api/sandbox2/util/temp_file_test.cc @@ -24,40 +24,39 @@ #include "sandboxed_api/sandbox2/util/path.h" #include "sandboxed_api/util/status_matchers.h" -using sapi::IsOk; -using sapi::StatusIs; -using testing::Eq; -using testing::IsTrue; -using testing::Ne; -using testing::StartsWith; - namespace sandbox2 { namespace { +using ::sapi::IsOk; +using ::sapi::StatusIs; +using ::testing::Eq; +using ::testing::IsTrue; +using ::testing::Ne; +using ::testing::StartsWith; + TEST(TempFileTest, CreateTempDirTest) { const std::string prefix = GetTestTempPath("MakeTempDirTest_"); - auto result_or = CreateTempDir(prefix); - ASSERT_THAT(result_or.status(), IsOk()); - std::string path = result_or.ValueOrDie(); + SAPI_ASSERT_OK_AND_ASSIGN(std::string path, CreateTempDir(prefix)); + EXPECT_THAT(path, StartsWith(prefix)); EXPECT_THAT(file_util::fileops::Exists(path, false), IsTrue()); - result_or = CreateTempDir("non_existing_dir/prefix"); - EXPECT_THAT(result_or, StatusIs(absl::StatusCode::kUnknown)); + EXPECT_THAT(CreateTempDir("non_existing_dir/prefix"), + StatusIs(absl::StatusCode::kUnknown)); } TEST(TempFileTest, MakeTempFileTest) { const std::string prefix = GetTestTempPath("MakeTempDirTest_"); + auto result_or = CreateNamedTempFile(prefix); ASSERT_THAT(result_or.status(), IsOk()); - std::string path; - int fd; - std::tie(path, fd) = result_or.ValueOrDie(); + auto [path, fd] = std::move(result_or).value(); + EXPECT_THAT(path, StartsWith(prefix)); EXPECT_THAT(file_util::fileops::Exists(path, false), IsTrue()); EXPECT_THAT(fcntl(fd, F_GETFD), Ne(-1)); EXPECT_THAT(close(fd), Eq(0)); - result_or = CreateNamedTempFile("non_existing_dir/prefix"); - EXPECT_THAT(result_or, StatusIs(absl::StatusCode::kUnknown)); + EXPECT_THAT(CreateNamedTempFile("non_existing_dir/prefix"), + StatusIs(absl::StatusCode::kUnknown)); } } // namespace diff --git a/sandboxed_api/sapi_test.cc b/sandboxed_api/sapi_test.cc index be8c21b..9c953b0 100644 --- a/sandboxed_api/sapi_test.cc +++ b/sandboxed_api/sapi_test.cc @@ -200,59 +200,51 @@ TEST(SandboxTest, RestartTransactionSandboxFD) { // Make sure we can recover from a dying sandbox. TEST(SandboxTest, RestartSandboxAfterCrash) { - sapi::BasicTransaction st{absl::make_unique()}; + SumSandbox sandbox; + ASSERT_THAT(sandbox.Init(), IsOk()); + SumApi api(&sandbox); - auto test_body = [](sapi::Sandbox* sandbox) -> absl::Status { - SumApi sumapi(sandbox); - // Crash the sandbox. - EXPECT_THAT(sumapi.crash(), StatusIs(absl::StatusCode::kUnavailable)); - EXPECT_THAT(sumapi.sum(1, 2).status(), - StatusIs(absl::StatusCode::kUnavailable)); - EXPECT_THAT(sandbox->AwaitResult().final_status(), - Eq(sandbox2::Result::SIGNALED)); + // Crash the sandbox. + EXPECT_THAT(api.crash(), StatusIs(absl::StatusCode::kUnavailable)); + EXPECT_THAT(api.sum(1, 2).status(), StatusIs(absl::StatusCode::kUnavailable)); + EXPECT_THAT(sandbox.AwaitResult().final_status(), + Eq(sandbox2::Result::SIGNALED)); - // Restart the sandbox. - EXPECT_THAT(sandbox->Restart(false), IsOk()); - // The sandbox should now be responsive again. - auto result_or = sumapi.sum(1, 2); - EXPECT_THAT(result_or.ValueOrDie(), Eq(3)); - return absl::OkStatus(); - }; + // Restart the sandbox. + ASSERT_THAT(sandbox.Restart(false), IsOk()); - EXPECT_THAT(st.Run(test_body), IsOk()); + // The sandbox should now be responsive again. + SAPI_ASSERT_OK_AND_ASSIGN(int result, api.sum(1, 2)); + EXPECT_THAT(result, Eq(3)); } TEST(SandboxTest, RestartSandboxAfterViolation) { - sapi::BasicTransaction st{absl::make_unique()}; + SumSandbox sandbox; + ASSERT_THAT(sandbox.Init(), IsOk()); + SumApi api(&sandbox); - auto test_body = [](sapi::Sandbox* sandbox) -> absl::Status { - SumApi sumapi(sandbox); - // Violate the sandbox policy. - EXPECT_THAT(sumapi.violate(), StatusIs(absl::StatusCode::kUnavailable)); - EXPECT_THAT(sumapi.sum(1, 2).status(), - StatusIs(absl::StatusCode::kUnavailable)); - EXPECT_THAT(sandbox->AwaitResult().final_status(), - Eq(sandbox2::Result::VIOLATION)); + // Violate the sandbox policy. + EXPECT_THAT(api.violate(), StatusIs(absl::StatusCode::kUnavailable)); + EXPECT_THAT(api.sum(1, 2).status(), StatusIs(absl::StatusCode::kUnavailable)); + EXPECT_THAT(sandbox.AwaitResult().final_status(), + Eq(sandbox2::Result::VIOLATION)); - // Restart the sandbox. - EXPECT_THAT(sandbox->Restart(false), IsOk()); - // The sandbox should now be responsive again. - auto result_or = sumapi.sum(1, 2); - EXPECT_THAT(result_or, IsOk()); - EXPECT_THAT(result_or.ValueOrDie(), Eq(3)); - return absl::OkStatus(); - }; + // Restart the sandbox. + ASSERT_THAT(sandbox.Restart(false), IsOk()); - EXPECT_THAT(st.Run(test_body), IsOk()); + // The sandbox should now be responsive again. + SAPI_ASSERT_OK_AND_ASSIGN(int result, api.sum(1, 2)); + EXPECT_THAT(result, Eq(3)); } TEST(SandboxTest, NoRaceInAwaitResult) { - auto sandbox = absl::make_unique(); - ASSERT_THAT(sandbox->Init(), IsOk()); - StringopApi api(sandbox.get()); + StringopSandbox sandbox; + ASSERT_THAT(sandbox.Init(), IsOk()); + StringopApi api(&sandbox); + EXPECT_THAT(api.violate(), StatusIs(absl::StatusCode::kUnavailable)); - absl::SleepFor(absl::Milliseconds(200)); // make sure we lose the race - const auto& result = sandbox->AwaitResult(); + absl::SleepFor(absl::Milliseconds(200)); // Make sure we lose the race + const auto& result = sandbox.AwaitResult(); EXPECT_THAT(result.final_status(), Eq(sandbox2::Result::VIOLATION)); } diff --git a/sandboxed_api/util/status_matchers.h b/sandboxed_api/util/status_matchers.h index 0cc86c6..239a5f7 100644 --- a/sandboxed_api/util/status_matchers.h +++ b/sandboxed_api/util/status_matchers.h @@ -29,8 +29,8 @@ #define SAPI_ASSERT_OK_AND_ASSIGN_IMPL(statusor, lhs, rexpr) \ auto statusor = (rexpr); \ - ASSERT_THAT(statusor.status(), sapi::IsOk()); \ - lhs = std::move(statusor).ValueOrDie(); + ASSERT_THAT(statusor.status(), ::sapi::IsOk()); \ + lhs = std::move(statusor).value(); namespace sapi { namespace internal { diff --git a/sandboxed_api/var_proto.h b/sandboxed_api/var_proto.h index 5024ce4..4d1dac2 100644 --- a/sandboxed_api/var_proto.h +++ b/sandboxed_api/var_proto.h @@ -39,7 +39,7 @@ class Proto : public Pointable, public Var { ABSL_DEPRECATED("Use Proto<>::FromMessage() instead") explicit Proto(const T& proto) - : wrapped_var_(SerializeProto(proto).ValueOrDie()) {} + : wrapped_var_(SerializeProto(proto).value()) {} static sapi::StatusOr> FromMessage(const T& proto) { SAPI_ASSIGN_OR_RETURN(std::vector len_val, SerializeProto(proto)); @@ -67,9 +67,8 @@ class Proto : public Pointable, public Var { ABSL_DEPRECATED("Use GetMessage() instead") std::unique_ptr GetProtoCopy() const { - auto result_or = GetMessage(); - if (result_or.ok()) { - return absl::make_unique(std::move(result_or).ValueOrDie()); + if (auto result_or = GetMessage(); result_or.ok()) { + return absl::make_unique(std::move(result_or).value()); } return nullptr; }