Skip to content

Commit

Permalink
pipe: no exceptions
Browse files Browse the repository at this point in the history
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
  • Loading branch information
alyssawilk committed Jun 12, 2024
1 parent 6f78c25 commit 2c96486
Show file tree
Hide file tree
Showing 37 changed files with 189 additions and 151 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ namespace {

class FuzzerMocks {
public:
FuzzerMocks() : addr_(std::make_shared<Network::Address::PipeInstance>("/test/test.sock")) {
FuzzerMocks() : addr_(*Network::Address::PipeInstance::create("/test/test.sock")) {

ON_CALL(decoder_callbacks_, connection())
.WillByDefault(Return(OptRef<const Network::Connection>{connection_}));
Expand Down
50 changes: 30 additions & 20 deletions source/common/network/address_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -359,29 +359,34 @@ void Ipv6Instance::initHelper(const sockaddr_in6& address, bool v6only) {
friendly_name_ = fmt::format("[{}]:{}", ip_.friendly_address_, ip_.port());
}

PipeInstance::PipeInstance(const sockaddr_un* address, socklen_t ss_len, mode_t mode,
const SocketInterface* sock_interface)
: InstanceBase(Type::Pipe, sockInterfaceOrDefault(sock_interface)) {
if (address->sun_path[0] == '\0') {
#if !defined(__linux__)
throwEnvoyExceptionOrPanic("Abstract AF_UNIX sockets are only supported on linux.");
#endif
RELEASE_ASSERT(static_cast<unsigned int>(ss_len) >= offsetof(struct sockaddr_un, sun_path) + 1,
"");
pipe_.abstract_namespace_ = true;
pipe_.address_length_ = ss_len - offsetof(struct sockaddr_un, sun_path);
}
absl::Status status = initHelper(address, mode);
throwOnError(status);
absl::StatusOr<std::unique_ptr<PipeInstance>>
PipeInstance::create(const sockaddr_un* address, socklen_t ss_len, mode_t mode,
const SocketInterface* sock_interface) {
absl::Status creation_status = absl::OkStatus();
auto ret = std::unique_ptr<PipeInstance>(
new PipeInstance(creation_status, address, ss_len, mode, sock_interface));
RETURN_IF_NOT_OK(creation_status);
return ret;
}

absl::StatusOr<std::unique_ptr<PipeInstance>>
PipeInstance::create(const std::string& pipe_path, mode_t mode,
const SocketInterface* sock_interface) {
absl::Status creation_status = absl::OkStatus();
auto ret = std::unique_ptr<PipeInstance>(
new PipeInstance(pipe_path, mode, sock_interface, creation_status));
RETURN_IF_NOT_OK(creation_status);
return ret;
}

PipeInstance::PipeInstance(const std::string& pipe_path, mode_t mode,
const SocketInterface* sock_interface)
const SocketInterface* sock_interface, absl::Status& creation_status)
: InstanceBase(Type::Pipe, sockInterfaceOrDefault(sock_interface)) {
if (pipe_path.size() >= sizeof(pipe_.address_.sun_path)) {
throwEnvoyExceptionOrPanic(
creation_status = absl::InvalidArgumentError(
fmt::format("Path \"{}\" exceeds maximum UNIX domain socket path size of {}.", pipe_path,
sizeof(pipe_.address_.sun_path)));
return;
}
memset(&pipe_.address_, 0, sizeof(pipe_.address_));
pipe_.address_.sun_family = AF_UNIX;
Expand All @@ -392,10 +397,13 @@ PipeInstance::PipeInstance(const std::string& pipe_path, mode_t mode,
// be null terminated. The friendly name is the address path with embedded nulls replaced with
// '@' for consistency with the first character.
#if !defined(__linux__)
throwEnvoyExceptionOrPanic("Abstract AF_UNIX sockets are only supported on linux.");
creation_status =
absl::InvalidArgumentError("Abstract AF_UNIX sockets are only supported on linux.");
return;
#endif
if (mode != 0) {
throwEnvoyExceptionOrPanic("Cannot set mode for Abstract AF_UNIX sockets");
creation_status = absl::InvalidArgumentError("Cannot set mode for Abstract AF_UNIX sockets");
return;
}
pipe_.abstract_namespace_ = true;
pipe_.address_length_ = pipe_path.size();
Expand All @@ -407,9 +415,11 @@ PipeInstance::PipeInstance(const std::string& pipe_path, mode_t mode,
friendly_name_ = friendlyNameFromAbstractPath(
absl::string_view(pipe_.address_.sun_path, pipe_.address_length_));
} else {
// Throw an error if the pipe path has an embedded null character.
// return an error if the pipe path has an embedded null character.
if (pipe_path.size() != strlen(pipe_path.c_str())) {
throwEnvoyExceptionOrPanic("UNIX domain socket pathname contains embedded null characters");
creation_status = absl::InvalidArgumentError(
"UNIX domain socket pathname contains embedded null characters");
return;
}
StringUtil::strlcpy(&pipe_.address_.sun_path[0], pipe_path.c_str(),
sizeof(pipe_.address_.sun_path));
Expand Down
14 changes: 9 additions & 5 deletions source/common/network/address_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class InstanceFactory {
public:
template <typename InstanceType, typename... Args>
static StatusOr<InstanceConstSharedPtr> createInstancePtr(Args&&... args) {
absl::Status status;
absl::Status status = absl::OkStatus();
// Use new instead of make_shared here because the instance constructors are private and must be
// called directly here.
std::shared_ptr<InstanceType> instance(new InstanceType(status, std::forward<Args>(args)...));
Expand Down Expand Up @@ -301,14 +301,16 @@ class PipeInstance : public InstanceBase {
/**
* Construct from an existing unix address.
*/
explicit PipeInstance(const sockaddr_un* address, socklen_t ss_len, mode_t mode = 0,
const SocketInterface* sock_interface = nullptr);
static absl::StatusOr<std::unique_ptr<PipeInstance>>
create(const sockaddr_un* address, socklen_t ss_len, mode_t mode = 0,
const SocketInterface* sock_interface = nullptr);

/**
* Construct from a string pipe path.
*/
explicit PipeInstance(const std::string& pipe_path, mode_t mode = 0,
const SocketInterface* sock_interface = nullptr);
static absl::StatusOr<std::unique_ptr<PipeInstance>>
create(const std::string& pipe_path, mode_t mode = 0,
const SocketInterface* sock_interface = nullptr);

static absl::Status validateProtocolSupported() { return absl::OkStatus(); }

Expand All @@ -330,6 +332,8 @@ class PipeInstance : public InstanceBase {
absl::string_view addressType() const override { return "default"; }

private:
explicit PipeInstance(const std::string& pipe_path, mode_t mode,
const SocketInterface* sock_interface, absl::Status& creation_status);
/**
* Construct from an existing unix address.
* Store the error status code in passed in parameter instead of throwing.
Expand Down
2 changes: 1 addition & 1 deletion source/common/network/resolver_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ resolveProtoAddress(const envoy::config::core::v3::Address& address) {
case envoy::config::core::v3::Address::AddressCase::kSocketAddress:
return resolveProtoSocketAddress(address.socket_address());
case envoy::config::core::v3::Address::AddressCase::kPipe:
return std::make_shared<PipeInstance>(address.pipe().path(), address.pipe().mode());
return PipeInstance::create(address.pipe().path(), address.pipe().mode());
case envoy::config::core::v3::Address::AddressCase::kEnvoyInternalAddress:
switch (address.envoy_internal_address().address_name_specifier_case()) {
case envoy::config::core::v3::EnvoyInternalAddress::AddressNameSpecifierCase::
Expand Down
23 changes: 19 additions & 4 deletions source/common/network/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ absl::StatusOr<Address::InstanceConstSharedPtr> Utility::resolveUrl(const std::s
} else if (urlIsUdpScheme(url)) {
address = parseInternetAddressAndPortNoThrow(url.substr(UDP_SCHEME.size()));
} else if (urlIsUnixScheme(url)) {
return std::make_shared<Address::PipeInstance>(url.substr(UNIX_SCHEME.size()));
return Address::PipeInstance::create(url.substr(UNIX_SCHEME.size()));
} else {
return absl::InvalidArgumentError(absl::StrCat("unknown protocol scheme: ", url));
}
Expand Down Expand Up @@ -442,9 +442,14 @@ Utility::protobufAddressToAddressNoThrow(const envoy::config::core::v3::Address&
return Utility::parseInternetAddressNoThrow(proto_address.socket_address().address(),
proto_address.socket_address().port_value(),
!proto_address.socket_address().ipv4_compat());
case envoy::config::core::v3::Address::AddressCase::kPipe:
return std::make_shared<Address::PipeInstance>(proto_address.pipe().path(),
proto_address.pipe().mode());
case envoy::config::core::v3::Address::AddressCase::kPipe: {
auto ret =
Address::PipeInstance::create(proto_address.pipe().path(), proto_address.pipe().mode());
if (ret.status().ok()) {
return ret;
}
return nullptr;
}
case envoy::config::core::v3::Address::AddressCase::kEnvoyInternalAddress:
return std::make_shared<Address::EnvoyInternalInstance>(
proto_address.envoy_internal_address().server_listener_name(),
Expand Down Expand Up @@ -810,5 +815,15 @@ ResolvedUdpSocketConfig::ResolvedUdpSocketConfig(
}
}

const envoy::config::core::v3::UdpSocketConfig& config, bool prefer_gro_default)
: max_rx_datagram_size_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, max_rx_datagram_size,
DEFAULT_UDP_MAX_DATAGRAM_SIZE)),
prefer_gro_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, prefer_gro, prefer_gro_default)) {
if (prefer_gro_ && !Api::OsSysCallsSingleton::get().supportsUdpGro()) {
ENVOY_LOG_MISC(
warn, "GRO requested but not supported by the OS. Check OS config or disable prefer_gro.");
}
}

} // namespace Network
} // namespace Envoy
8 changes: 5 additions & 3 deletions source/server/hot_restarting_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,11 @@ sockaddr_un RpcStream::createDomainSocketAddress(uint64_t id, const std::string&
id = id % MaxConcurrentProcesses;
sockaddr_un address;
initDomainSocketAddress(&address);
Network::Address::PipeInstance addr(fmt::format("{}_{}_{}", socket_path, role, base_id_ + id),
socket_mode, nullptr);
safeMemcpy(&address, &(addr.getSockAddr()));
auto addr = THROW_OR_RETURN_VALUE(
Network::Address::PipeInstance::create(
fmt::format("{}_{}_{}", socket_path, role, base_id_ + id), socket_mode, nullptr),
std::unique_ptr<Network::Address::PipeInstance>);
safeMemcpy(&address, &(addr->getSockAddr()));
fchmod(domain_socket_, socket_mode);

return address;
Expand Down
9 changes: 6 additions & 3 deletions test/common/formatter/substitution_formatter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,8 @@ TEST(SubstitutionFormatterTest, streamInfoFormatter) {
ProtoEq(ValueUtil::stringValue("[::1]:19443")));

// Validate for Pipe
address = Network::Address::InstanceConstSharedPtr{new Network::Address::PipeInstance("/foo")};
address =
Network::Address::InstanceConstSharedPtr{*Network::Address::PipeInstance::create("/foo")};
stream_info.upstreamInfo()->setUpstreamLocalAddress(address);
EXPECT_EQ("/foo", upstream_format.formatWithContext({}, stream_info));
EXPECT_THAT(upstream_format.formatValueWithContext({}, stream_info),
Expand Down Expand Up @@ -647,7 +648,8 @@ TEST(SubstitutionFormatterTest, streamInfoFormatter) {
ProtoEq(ValueUtil::numberValue(19443)));

// Validate for Pipe
address = Network::Address::InstanceConstSharedPtr{new Network::Address::PipeInstance("/foo")};
address =
Network::Address::InstanceConstSharedPtr{*Network::Address::PipeInstance::create("/foo")};
stream_info.upstreamInfo()->setUpstreamLocalAddress(address);
EXPECT_EQ("", upstream_format.formatWithContext({}, stream_info));
EXPECT_THAT(upstream_format.formatValueWithContext({}, stream_info),
Expand Down Expand Up @@ -848,7 +850,8 @@ TEST(SubstitutionFormatterTest, streamInfoFormatter) {
ProtoEq(ValueUtil::numberValue(9443)));

// Validate for Pipe
address = Network::Address::InstanceConstSharedPtr{new Network::Address::PipeInstance("/foo")};
address =
Network::Address::InstanceConstSharedPtr{*Network::Address::PipeInstance::create("/foo")};
stream_info.downstream_connection_info_provider_->setLocalAddress(address);
EXPECT_EQ("", upstream_format.formatWithContext({}, stream_info));
EXPECT_THAT(upstream_format.formatValueWithContext({}, stream_info),
Expand Down
2 changes: 1 addition & 1 deletion test/common/http/conn_manager_utility_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -899,7 +899,7 @@ TEST_F(ConnectionManagerUtilityTest, ExternalAddressExternalRequestDontUseRemote
// Verify that if XFF is invalid we fall back to remote address, even if it is a pipe.
TEST_F(ConnectionManagerUtilityTest, PipeAddressInvalidXFFtDontUseRemote) {
connection_.stream_info_.downstream_connection_info_provider_->setRemoteAddress(
std::make_shared<Network::Address::PipeInstance>("/blah"));
*Network::Address::PipeInstance::create("/blah"));
ON_CALL(config_, useRemoteAddress()).WillByDefault(Return(false));
TestRequestHeaderMapImpl headers{{"x-forwarded-for", "blah"}};

Expand Down
4 changes: 2 additions & 2 deletions test/common/http/utility_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -420,8 +420,8 @@ TEST(HttpUtility, appendXff) {

{
TestRequestHeaderMapImpl headers{{"x-forwarded-for", "10.0.0.1"}};
Network::Address::PipeInstance address("/foo");
Utility::appendXff(headers, address);
auto address = *Network::Address::PipeInstance::create("/foo");
Utility::appendXff(headers, *address);
EXPECT_EQ("10.0.0.1", headers.get_("x-forwarded-for"));
}
}
Expand Down
4 changes: 2 additions & 2 deletions test/common/listener_manager/filter_chain_benchmark_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,14 @@ class MockConnectionSocket : public Network::ConnectionSocket {

if (absl::StartsWith(destination_address, "/")) {
res->connection_info_provider_->setLocalAddress(
std::make_shared<Network::Address::PipeInstance>(destination_address));
*Network::Address::PipeInstance::create(destination_address));
} else {
res->connection_info_provider_->setLocalAddress(
Network::Utility::parseInternetAddressNoThrow(destination_address, destination_port));
}
if (absl::StartsWith(source_address, "/")) {
res->connection_info_provider_->setRemoteAddress(
std::make_shared<Network::Address::PipeInstance>(source_address));
*Network::Address::PipeInstance::create(source_address));
} else {
res->connection_info_provider_->setRemoteAddress(
Network::Utility::parseInternetAddressNoThrow(source_address, source_port));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class FilterChainManagerImplTest : public testing::TestWithParam<bool> {
sockets_.push_back(mock_socket);

if (absl::StartsWith(destination_address, "/")) {
local_address_ = std::make_shared<Network::Address::PipeInstance>(destination_address);
local_address_ = *Network::Address::PipeInstance::create(destination_address);
} else {
local_address_ =
Network::Utility::parseInternetAddressNoThrow(destination_address, destination_port);
Expand All @@ -94,7 +94,7 @@ class FilterChainManagerImplTest : public testing::TestWithParam<bool> {
.WillByDefault(ReturnRef(application_protocols));

if (absl::StartsWith(source_address, "/")) {
remote_address_ = std::make_shared<Network::Address::PipeInstance>(source_address);
remote_address_ = *Network::Address::PipeInstance::create(source_address);
} else {
remote_address_ = Network::Utility::parseInternetAddressNoThrow(source_address, source_port);
}
Expand Down
2 changes: 1 addition & 1 deletion test/common/listener_manager/listener_manager_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2787,7 +2787,7 @@ name: foo
TEST_P(ListenerManagerImplTest, NotSupportedDatagramUds) {
ProdListenerComponentFactory real_listener_factory(server_);
EXPECT_THROW_WITH_MESSAGE(real_listener_factory.createListenSocket(
std::make_shared<Network::Address::PipeInstance>("/foo"),
*Network::Address::PipeInstance::create("/foo"),
Network::Socket::Type::Datagram, nullptr, default_bind_type, {}, 0),
EnvoyException,
"socket type SocketType::Datagram not supported for pipes");
Expand Down
7 changes: 3 additions & 4 deletions test/common/listener_manager/listener_manager_impl_test.h
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ class ListenerManagerImplTest : public testing::TestWithParam<bool> {
const std::string& source_address, uint16_t source_port,
std::string direct_source_address = "") {
if (absl::StartsWith(destination_address, "/")) {
local_address_ = std::make_shared<Network::Address::PipeInstance>(destination_address);
local_address_ = *Network::Address::PipeInstance::create(destination_address);
} else {
local_address_ =
Network::Utility::parseInternetAddressNoThrow(destination_address, destination_port);
Expand All @@ -240,7 +240,7 @@ class ListenerManagerImplTest : public testing::TestWithParam<bool> {
.WillByDefault(ReturnRef(application_protocols));

if (absl::StartsWith(source_address, "/")) {
remote_address_ = std::make_shared<Network::Address::PipeInstance>(source_address);
remote_address_ = *Network::Address::PipeInstance::create(source_address);
} else {
remote_address_ = Network::Utility::parseInternetAddressNoThrow(source_address, source_port);
}
Expand All @@ -250,8 +250,7 @@ class ListenerManagerImplTest : public testing::TestWithParam<bool> {
direct_source_address = source_address;
}
if (absl::StartsWith(direct_source_address, "/")) {
direct_remote_address_ =
std::make_shared<Network::Address::PipeInstance>(direct_source_address);
direct_remote_address_ = *Network::Address::PipeInstance::create(direct_source_address);
} else {
direct_remote_address_ =
Network::Utility::parseInternetAddressNoThrow(direct_source_address, source_port);
Expand Down
Loading

0 comments on commit 2c96486

Please sign in to comment.