Skip to content

Commit

Permalink
server: Return nullopt when process_context is nullptr (#14181)
Browse files Browse the repository at this point in the history
The InstanceImpl::processContext() function returns a ProcessContextOptRef
a.k.a. absl::optional<std::reference_wrapper<ProcessContext>>.  When
InstanceImpl::process_context_ is nullptr, the returned value an
absl::optional with a value of std::reference_wrapper<>(nullptr),
better known as an illegal reference to nullptr.  While everything is
fine in the InstanceImpl, anyone who tries to use the returned
reference will be greeted with a crash.

This commit follows the lead of the initializer of InstanceImpl::api_
and returns absl::nullopt when InstanceImpl::process_context_ is
nullptr.

Risk: Low
Testing: bazel test //test/server:server_test
Documentation: N/A
Release Notes: N/A

Signed-off-by: Justin Mazzola Paluska <[email protected]>
  • Loading branch information
justin-mp authored Nov 28, 2020
1 parent 021e815 commit 1209a69
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 1 deletion.
8 changes: 8 additions & 0 deletions source/server/server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,14 @@ void InstanceImpl::flushStatsInternal() {

bool InstanceImpl::healthCheckFailed() { return !live_.load(); }

ProcessContextOptRef InstanceImpl::processContext() {
if (process_context_ == nullptr) {
return absl::nullopt;
}

return *process_context_;
}

namespace {
// Loads a bootstrap object, potentially at a specific version (upgrading if necessary).
void loadBootstrap(absl::optional<uint32_t> bootstrap_version,
Expand Down
2 changes: 1 addition & 1 deletion source/server/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ class InstanceImpl final : Logger::Loggable<Logger::Id::main>,
Stats::Store& stats() override { return stats_store_; }
Grpc::Context& grpcContext() override { return grpc_context_; }
Http::Context& httpContext() override { return http_context_; }
ProcessContextOptRef processContext() override { return *process_context_; }
ProcessContextOptRef processContext() override;
ThreadLocal::Instance& threadLocal() override { return thread_local_; }
const LocalInfo::LocalInfo& localInfo() const override { return *local_info_; }
TimeSource& timeSource() override { return time_source_; }
Expand Down
10 changes: 10 additions & 0 deletions test/server/server_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1411,6 +1411,16 @@ TEST_P(ServerInstanceImplTest, DisabledExtension) {
ASSERT_TRUE(disabled_filter_found);
}

TEST_P(ServerInstanceImplTest, NullProcessContextTest) {
// These are already the defaults. Repeated here for clarity.
process_object_ = nullptr;
process_context_ = nullptr;

initialize("test/server/test_data/server/empty_bootstrap.yaml");
ProcessContextOptRef context = server_->processContext();
EXPECT_FALSE(context.has_value());
}

} // namespace
} // namespace Server
} // namespace Envoy

0 comments on commit 1209a69

Please sign in to comment.