Changed README, Bazel deps and different parts of code according to the review

pull/49/head
Bohdan Tyshchenko 2020-08-31 02:19:00 -07:00
parent 465cf9c4bb
commit 9803d0549f
16 changed files with 132 additions and 127 deletions

View File

@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
licenses(["unencumbered"]) # code authored by Google
licenses(["notice"])
load(
"@com_google_sandboxed_api//sandboxed_api/bazel:sapi.bzl",

View File

@ -1,11 +1,11 @@
# Guetzli Sandboxed
# Guetzli Sandbox
This is an example implementation of a sandbox for the [Guetzli](https://github.com/google/guetzli) library using [Sandboxed API](https://github.com/google/sandboxed-api).
Please read Guetzli's [documentation](https://github.com/google/guetzli#introduction) to learn more about it.
## Implementation details
Because Guetzli provides C++ API and SAPI requires functions to be `extern "C"` a wrapper library has been written for the compatibility. SAPI provides a Transaction class, which is a convenient way to create a wrapper for your sandboxed API that handles internal errors. Original Guetzli has command-line utility to encode images, so fully compatible utility that uses sandboxed Guetzli is provided.
Because Guetzli provides a C++ API and SAPI requires functions to be `extern "C"`, a wrapper library has been written for the compatibility. SAPI provides a Transaction class, which is a convenient way to create a wrapper for your sandboxed API that handles internal errors. The original Guetzli has a command-line utility to encode images, so a fully compatible utility that uses sandboxed Guetzli is provided.
Wrapper around Guetzli uses file descriptors to pass data to the sandbox. This approach restricts the sandbox from using open() syscall and also helps to prevent making copies of data, because you need to synchronize it between processes.
The wrapper around Guetzli uses file descriptors to pass data to the sandbox. This approach restricts the sandbox from using the `open()` syscall and also helps to prevent making copies of data, because you need to synchronize it between processes.
## Build Guetzli Sandboxed
Right now Sandboxed API support only Linux systems, so you need one to build it. Guetzli sandboxed uses [Bazel](https://bazel.build/) as a build system so you need to [install it](https://docs.bazel.build/versions/3.4.0/install.html) before building.
@ -13,7 +13,7 @@ Right now Sandboxed API support only Linux systems, so you need one to build it.
To build Guetzli sandboxed encoding utility you can use this command:
`bazel build //:guetzli_sandboxed`
Than you can use it in this way:
Then you can use it in this way:
```
guetzli_sandboxed [--quality Q] [--verbose] original.png output.jpg
guetzli_sandboxed [--quality Q] [--verbose] original.jpg output.jpg
@ -25,7 +25,7 @@ There are two different sets of unit tests which demonstrate how to use differen
* `tests/guetzli_sapi_test.cc` - example usage of Guetzli sandboxed API.
* `tests/guetzli_transaction_test.cc` - example usage of Guetzli transaction.
To run tests use following command:
To run tests use the following command:
`bazel test ...`
Also, there is an example of custom security policy for your sandbox in

View File

@ -71,14 +71,6 @@ http_archive(
url = "http://github.com/glennrp/libpng/archive/v1.2.57.zip",
)
http_archive(
name = "zlib_archive",
build_file = "zlib.BUILD",
sha256 = "8d7e9f698ce48787b6e1c67e6bff79e487303e66077e25cb9784ac8835978017",
strip_prefix = "zlib-1.2.10",
url = "http://zlib.net/fossils/zlib-1.2.10.tar.gz",
)
http_archive(
name = "jpeg_archive",
url = "http://www.ijg.org/files/jpegsrc.v9b.tar.gz",
@ -87,9 +79,22 @@ http_archive(
build_file = "jpeg.BUILD",
)
maybe(
git_repository,
name = "googletest",
remote = "https://github.com/google/googletest",
tag = "release-1.10.0",
http_archive(
name = "net_zlib",
build_file = "zlib.BUILD",
sha256 = "c3e5e9fdd5004dcb542feda5ee4f0ff0744628baf8ed2dd5d66f8ca1197cb1a1", # 2020-04-23
strip_prefix = "zlib-1.2.11",
urls = [
"https://mirror.bazel.build/zlib.net/zlib-1.2.11.tar.gz",
"https://www.zlib.net/zlib-1.2.11.tar.gz",
],
)
# GoogleTest/GoogleMock
maybe(
http_archive,
name = "com_google_googletest",
sha256 = "a6ab7c7d6fd4dd727f6012b5d85d71a73d3aa1274f529ecd4ad84eb9ec4ff767", # 2020-04-16
strip_prefix = "googletest-dcc92d0ab6c4ce022162a23566d44f673251eee4",
urls = ["https://github.com/google/googletest/archive/dcc92d0ab6c4ce022162a23566d44f673251eee4.zip"],
)

View File

@ -12,10 +12,10 @@
# See the License for the specific language governing permissions and
# limitations under the License.
licenses(["unencumbered"]) # code authored by Google
licenses(["notice"])
cc_library(
name = "butteraugli_lib",
name = "butteraugli",
srcs = [
"butteraugli/butteraugli.cc",
"butteraugli/butteraugli.h",

View File

@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
licenses(["unencumbered"]) # code authored by Google
licenses(["notice"])
cc_library(
name = "guetzli_lib",
@ -27,6 +27,6 @@ cc_library(
copts = [ "-Wno-sign-compare" ],
visibility= [ "//visibility:public" ],
deps = [
"@butteraugli//:butteraugli_lib",
"@butteraugli//:butteraugli",
],
)

View File

@ -1,4 +1,3 @@
# Description:
# The Independent JPEG Group's JPEG runtime library.

View File

@ -29,5 +29,5 @@ cc_library(
includes = ["."],
linkopts = ["-lm"],
visibility = ["//visibility:public"],
deps = ["@zlib_archive//:zlib"],
deps = ["@net_zlib//:zlib"],
)

View File

@ -1,6 +1,18 @@
package(default_visibility = ["//visibility:public"])
# Copyright 2019 Google LLC
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
licenses(["notice"]) # BSD/MIT-like license (for zlib)
licenses(["notice"])
cc_library(
name = "zlib",
@ -8,29 +20,37 @@ cc_library(
"adler32.c",
"compress.c",
"crc32.c",
"crc32.h",
"deflate.c",
"deflate.h",
"gzclose.c",
"gzguts.h",
"gzlib.c",
"gzread.c",
"gzwrite.c",
"infback.c",
"inffast.c",
"inffast.h",
"inffixed.h",
"inflate.c",
"inflate.h",
"inftrees.c",
"inftrees.h",
"trees.c",
"trees.h",
"uncompr.c",
"zconf.h",
"zutil.c",
"zutil.h",
],
hdrs = ["zlib.h"],
# Use -Dverbose=-1 to turn off zlib's trace logging. (#3280)
copts = [
"-w",
"-Dverbose=-1",
],
includes = ["."],
textual_hdrs = [
"crc32.h",
"deflate.h",
"gzguts.h",
"inffast.h",
"inffixed.h",
"inftrees.h",
"zconf.h",
"zutil.h",
],
visibility = ["//visibility:public"],
)

View File

@ -75,8 +75,8 @@ sapi::StatusOr<std::string> ReadFromFd(int fd) {
}
sapi::StatusOr<GuetzliInitData> PrepareDataForProcessing(
const ProcessingParams* processing_params) {
sapi::StatusOr<std::string> input = ReadFromFd(processing_params->remote_fd);
const ProcessingParams& processing_params) {
sapi::StatusOr<std::string> input = ReadFromFd(processing_params.remote_fd);
if (!input.ok()) {
return input.status();
@ -84,11 +84,11 @@ sapi::StatusOr<GuetzliInitData> PrepareDataForProcessing(
guetzli::Params guetzli_params;
guetzli_params.butteraugli_target = static_cast<float>(
guetzli::ButteraugliScoreForQuality(processing_params->quality));
guetzli::ButteraugliScoreForQuality(processing_params.quality));
guetzli::ProcessStats stats;
if (processing_params->verbose) {
if (processing_params.verbose) {
stats.debug_output_file = stderr;
}
@ -234,7 +234,7 @@ bool CheckMemoryLimitExceeded(int memlimit_mb, int xsize, int ysize) {
extern "C" bool ProcessJpeg(const ProcessingParams* processing_params,
sapi::LenValStruct* output) {
auto processing_data = PrepareDataForProcessing(processing_params);
auto processing_data = PrepareDataForProcessing(*processing_params);
if (!processing_data.ok()) {
std::cerr << processing_data.status().ToString() << std::endl;
@ -269,7 +269,7 @@ extern "C" bool ProcessJpeg(const ProcessingParams* processing_params,
extern "C" bool ProcessRgb(const ProcessingParams* processing_params,
sapi::LenValStruct* output) {
auto processing_data = PrepareDataForProcessing(processing_params);
auto processing_data = PrepareDataForProcessing(*processing_params);
if (!processing_data.ok()) {
std::cerr << processing_data.status().ToString() << std::endl;

View File

@ -19,8 +19,7 @@
#include "guetzli_sapi.sapi.h"
namespace guetzli {
namespace sandbox {
namespace guetzli::sandbox {
class GuetzliSapiSandbox : public GuetzliSandbox {
public:
@ -43,7 +42,6 @@ class GuetzliSapiSandbox : public GuetzliSandbox {
}
};
} // namespace sandbox
} // namespace guetzli
} // namespace guetzli::sandbox
#endif // GUETZLI_SANDBOXED_GUETZLI_SANDBOX_H_

View File

@ -19,6 +19,7 @@
#include <unistd.h>
#include <iostream>
#include <cstdio>
#include "sandboxed_api/sandbox2/util/fileops.h"
#include "sandboxed_api/util/statusor.h"

View File

@ -22,8 +22,7 @@
#include <iostream>
#include <memory>
namespace guetzli {
namespace sandbox {
namespace guetzli::sandbox {
absl::Status GuetzliTransaction::Main() {
sapi::v::Fd in_fd(open(params_.in_file, O_RDONLY));
@ -53,22 +52,17 @@ absl::Status GuetzliTransaction::Main() {
};
auto result = image_type_ == ImageType::kJpeg ?
api.ProcessJpeg(processing_params.PtrBefore(), output.PtrBoth()) :
api.ProcessRgb(processing_params.PtrBefore(), output.PtrBoth());
api.ProcessJpeg(processing_params.PtrBefore(), output.PtrBefore()) :
api.ProcessRgb(processing_params.PtrBefore(), output.PtrBefore());
if (!result.value_or(false)) {
std::stringstream error_stream;
error_stream << "Error processing "
<< (image_type_ == ImageType::kJpeg ? "jpeg" : "rgb") << " data"
<< std::endl;
return absl::FailedPreconditionError(
error_stream.str()
absl::StrCat("Error processing ",
(image_type_ == ImageType::kJpeg ? "jpeg" : "rgb"), " data")
);
}
sapi::v::Fd out_fd(open(".", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR));
if (out_fd.GetValue() < 0) {
return absl::FailedPreconditionError(
"Error creating temp output file"
@ -83,7 +77,7 @@ absl::Status GuetzliTransaction::Main() {
}
auto write_result = api.WriteDataToFd(out_fd.GetRemoteFd(),
output.PtrBefore());
output.PtrNone());
if (!write_result.value_or(false)) {
return absl::FailedPreconditionError(
@ -99,20 +93,19 @@ absl::Status GuetzliTransaction::Main() {
absl::Status GuetzliTransaction::LinkOutFile(int out_fd) const {
if (access(params_.out_file, F_OK) != -1) {
if (remove(params_.out_file) < 0) {
std::stringstream error;
error << "Error deleting existing output file: " << params_.out_file;
return absl::FailedPreconditionError(error.str());
return absl::FailedPreconditionError(
absl::StrCat("Error deleting existing output file: ", params_.out_file)
);
}
}
}
std::stringstream path;
path << "/proc/self/fd/" << out_fd;
std::string path = absl::StrCat("/proc/self/fd/", out_fd);
if (linkat(AT_FDCWD, path.str().c_str(), AT_FDCWD, params_.out_file,
if (linkat(AT_FDCWD, path.c_str(), AT_FDCWD, params_.out_file,
AT_SYMLINK_FOLLOW) < 0) {
std::stringstream error;
error << "Error linking: " << params_.out_file;
return absl::FailedPreconditionError(error.str());
return absl::FailedPreconditionError(
absl::StrCat("Error linking: ", params_.out_file)
);
}
return absl::OkStatus();
@ -140,5 +133,4 @@ sapi::StatusOr<ImageType> GuetzliTransaction::GetImageTypeFromFd(int fd) const {
ImageType::kPng : ImageType::kJpeg;
}
} // namespace sandbox
} // namespace guetzli
} // namespace guetzli::sandbox

View File

@ -22,8 +22,7 @@
#include "guetzli_sandbox.h"
namespace guetzli {
namespace sandbox {
namespace guetzli::sandbox {
enum class ImageType {
kJpeg,
@ -42,12 +41,12 @@ struct TransactionParams {
// Create a new one for each processing operation
class GuetzliTransaction : public sapi::Transaction {
public:
GuetzliTransaction(TransactionParams params, int retry_count = 0)
: sapi::Transaction(std::make_unique<GuetzliSapiSandbox>())
, params_(std::move(params))
explicit GuetzliTransaction(TransactionParams params, int retry_count = 0)
: sapi::Transaction(std::make_unique<GuetzliSapiSandbox>()),
params_(std::move(params))
{
sapi::Transaction::set_retry_count(retry_count);
sapi::Transaction::SetTimeLimit(0); // Infinite time limit
set_retry_count(retry_count);
SetTimeLimit(absl::InfiniteDuration());
}
private:
@ -60,7 +59,6 @@ class GuetzliTransaction : public sapi::Transaction {
ImageType image_type_ = ImageType::kJpeg;
};
} // namespace sandbox
} // namespace guetzli
} // namespace guetzli::sandbox
#endif // GUETZLI_SANDBOXED_GUETZLI_TRANSACTION_H_

View File

@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
licenses(["unencumbered"]) # code authored by Google
licenses(["notice"])
cc_test(
name = "transaction_tests",
@ -20,10 +20,10 @@ cc_test(
visibility=["//visibility:public"],
deps = [
"//:guetzli_sapi",
"@googletest//:gtest_main"
"@com_google_googletest//:gtest_main",
],
size = "large",
data = glob(["testdata/*"])
data = glob(["testdata/*"]),
)
cc_test(
@ -32,8 +32,8 @@ cc_test(
visibility=["//visibility:public"],
deps = [
"//:guetzli_sapi",
"@googletest//:gtest_main"
"@com_google_googletest//:gtest_main",
],
size = "large",
data = glob(["testdata/*"])
data = glob(["testdata/*"]),
)

View File

@ -28,24 +28,22 @@
#include "guetzli_sandbox.h"
namespace guetzli {
namespace sandbox {
namespace tests {
namespace guetzli::sandbox::tests {
namespace {
constexpr const char* kInPngFilename = "bees.png";
constexpr const char* kInJpegFilename = "nature.jpg";
constexpr const char* kPngReferenceFilename = "bees_reference.jpg";
constexpr const char* kJpegReferenceFIlename = "nature_reference.jpg";
constexpr absl::string_view kInPngFilename = "bees.png";
constexpr absl::string_view kInJpegFilename = "nature.jpg";
constexpr absl::string_view kPngReferenceFilename = "bees_reference.jpg";
constexpr absl::string_view kJpegReferenceFIlename = "nature_reference.jpg";
constexpr int kDefaultQualityTarget = 95;
constexpr int kDefaultMemlimitMb = 6000;
constexpr const char* kRelativePathToTestdata =
constexpr absl::string_view kRelativePathToTestdata =
"/guetzli_sandboxed/tests/testdata/";
std::string GetPathToInputFile(const char* filename) {
std::string GetPathToInputFile(absl::string_view filename) {
return absl::StrCat(getenv("TEST_SRCDIR"), kRelativePathToTestdata, filename);
}
@ -67,7 +65,7 @@ class GuetzliSapiTest : public ::testing::Test {
protected:
void SetUp() override {
sandbox_ = std::make_unique<GuetzliSapiSandbox>();
sandbox_->Init().IgnoreError();
ASSERT_EQ(sandbox_->Init(), absl::OkStatus());
api_ = std::make_unique<GuetzliApi>(sandbox_.get());
}
@ -129,6 +127,4 @@ TEST_F(GuetzliSapiTest, ProcessJpeg) {
<< "Processed data doesn't match reference output";
}
} // namespace tests
} // namespace sandbox
} // namespace guetzli
} // namespace guetzli::sandbox::tests

View File

@ -26,18 +26,16 @@
#include "guetzli_transaction.h"
namespace guetzli {
namespace sandbox {
namespace tests {
namespace guetzli::sandbox::tests {
namespace {
constexpr const char* kInPngFilename = "bees.png";
constexpr const char* kInJpegFilename = "nature.jpg";
constexpr const char* kOutJpegFilename = "out_jpeg.jpg";
constexpr const char* kOutPngFilename = "out_png.png";
constexpr const char* kPngReferenceFilename = "bees_reference.jpg";
constexpr const char* kJpegReferenceFIlename = "nature_reference.jpg";
constexpr absl::string_view kInPngFilename = "bees.png";
constexpr absl::string_view kInJpegFilename = "nature.jpg";
constexpr absl::string_view kOutJpegFilename = "out_jpeg.jpg";
constexpr absl::string_view kOutPngFilename = "out_png.png";
constexpr absl::string_view kPngReferenceFilename = "bees_reference.jpg";
constexpr absl::string_view kJpegReferenceFIlename = "nature_reference.jpg";
constexpr int kPngExpectedSize = 38'625;
constexpr int kJpegExpectedSize = 10'816;
@ -45,10 +43,10 @@ constexpr int kJpegExpectedSize = 10'816;
constexpr int kDefaultQualityTarget = 95;
constexpr int kDefaultMemlimitMb = 6000;
constexpr const char* kRelativePathToTestdata =
constexpr absl::string_view kRelativePathToTestdata =
"/guetzli_sandboxed/tests/testdata/";
std::string GetPathToFile(const char* filename) {
std::string GetPathToFile(absl::string_view filename) {
return absl::StrCat(getenv("TEST_SRCDIR"), kRelativePathToTestdata, filename);
}
@ -99,26 +97,26 @@ TEST(GuetzliTransactionTest, TestTransactionJpg) {
};
{
GuetzliTransaction transaction(std::move(params));
auto result = transaction.Run();
absl::Status result = transaction.Run();
ASSERT_TRUE(result.ok()) << result.ToString();
}
auto reference_data = ReadFromFile(GetPathToFile(kJpegReferenceFIlename));
std::string reference_data = ReadFromFile(
GetPathToFile(kJpegReferenceFIlename));
FileRemover file_remover(out_path.c_str());
ASSERT_TRUE(file_remover.get() != -1) << "Error opening output file";
auto output_size = lseek(file_remover.get(), 0, SEEK_END);
off_t output_size = lseek(file_remover.get(), 0, SEEK_END);
ASSERT_EQ(reference_data.size(), output_size)
<< "Different sizes of reference and returned data";
ASSERT_EQ(lseek(file_remover.get(), 0, SEEK_SET), 0)
<< "Error repositioning out file";
std::unique_ptr<char[]> buf(new char[output_size]);
auto status = read(file_remover.get(), buf.get(), output_size);
std::string output;
output.resize(output_size);
ssize_t status = read(file_remover.get(), output.data(), output_size);
ASSERT_EQ(status, output_size) << "Error reading data from temp output file";
ASSERT_TRUE(
std::equal(buf.get(), buf.get() + output_size, reference_data.begin()))
<< "Returned data doesn't match reference";
ASSERT_EQ(output, reference_data) << "Returned data doesn't match reference";
}
TEST(GuetzliTransactionTest, TestTransactionPng) {
@ -134,28 +132,26 @@ TEST(GuetzliTransactionTest, TestTransactionPng) {
};
{
GuetzliTransaction transaction(std::move(params));
auto result = transaction.Run();
absl::Status result = transaction.Run();
ASSERT_TRUE(result.ok()) << result.ToString();
}
auto reference_data = ReadFromFile(GetPathToFile(kPngReferenceFilename));
std::string reference_data = ReadFromFile(
GetPathToFile(kPngReferenceFilename));
FileRemover file_remover(out_path.c_str());
ASSERT_TRUE(file_remover.get() != -1) << "Error opening output file";
auto output_size = lseek(file_remover.get(), 0, SEEK_END);
off_t output_size = lseek(file_remover.get(), 0, SEEK_END);
ASSERT_EQ(reference_data.size(), output_size)
<< "Different sizes of reference and returned data";
ASSERT_EQ(lseek(file_remover.get(), 0, SEEK_SET), 0)
<< "Error repositioning out file";
std::unique_ptr<char[]> buf(new char[output_size]);
auto status = read(file_remover.get(), buf.get(), output_size);
std::string output;
output.resize(output_size);
ssize_t status = read(file_remover.get(), output.data(), output_size);
ASSERT_EQ(status, output_size) << "Error reading data from temp output file";
ASSERT_TRUE(
std::equal(buf.get(), buf.get() + output_size, reference_data.begin()))
<< "Returned data doesn't match refernce";
ASSERT_EQ(output, reference_data) << "Returned data doesn't match refernce";
}
} // namespace tests
} // namespace sandbox
} // namespace guetzli
} // namespace guetzli::sandbox::tests