Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove HeaderString::c_str() and migrate callers to getStringView() #6564

Merged
merged 10 commits into from
Apr 16, 2019
27 changes: 17 additions & 10 deletions include/envoy/http/header_map.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,6 @@ class HeaderString {
*/
char* buffer() { return buffer_.dynamic_; }

/**
* @return a null terminated C string.
*/
const char* c_str() const { return buffer_.ref_; }

/**
* @return an absl::string_view.
*/
Expand All @@ -143,14 +138,24 @@ class HeaderString {

/**
* @return whether a substring exists in the string.
*
* TODO(dnoe): Eliminate this by migrating callers to use string_view find
* directly (#6580)
*/
bool find(const char* str) const { return strstr(c_str(), str); }
bool find(const char* str) const {
return getStringView().find(absl::string_view(str)) != absl::string_view::npos;
}

/**
* Set the value of the string by copying data into it. This overwrites any existing string.
*/
void setCopy(const char* data, uint32_t size);

/**
* Set the value of the string by copying data into it. This overwrites any existing string.
*/
void setCopy(absl::string_view view);

/**
* Set the value of the string to an integer. This overwrites any existing string.
*/
Expand All @@ -173,8 +178,10 @@ class HeaderString {
*/
Type type() const { return type_; }

bool operator==(const char* rhs) const { return 0 == strcmp(c_str(), rhs); }
bool operator!=(const char* rhs) const { return 0 != strcmp(c_str(), rhs); }
bool operator==(const char* rhs) const { return getStringView() == absl::string_view(rhs); }
bool operator==(absl::string_view rhs) const { return getStringView() == rhs; }
bool operator!=(const char* rhs) const { return getStringView() != absl::string_view(rhs); }
bool operator!=(absl::string_view rhs) const { return getStringView() != rhs; }

private:
union Buffer {
Expand Down Expand Up @@ -524,8 +531,8 @@ class HeaderMap {
friend std::ostream& operator<<(std::ostream& os, const HeaderMap& headers) {
headers.iterate(
[](const HeaderEntry& header, void* context) -> HeaderMap::Iterate {
*static_cast<std::ostream*>(context)
<< "'" << header.key().c_str() << "', '" << header.value().c_str() << "'\n";
*static_cast<std::ostream*>(context) << "'" << header.key().getStringView() << "', '"
<< header.value().getStringView() << "'\n";
return HeaderMap::Iterate::Continue;
},
&os);
Expand Down
2 changes: 1 addition & 1 deletion source/common/access_log/access_log_formatter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ std::string HeaderFormatter::format(const Http::HeaderMap& headers) const {
if (!header) {
header_value_string = UnspecifiedValueString;
} else {
header_value_string = header->value().c_str();
header_value_string = std::string(header->value().getStringView());
}

if (max_length_ && header_value_string.length() > max_length_.value()) {
Expand Down
3 changes: 2 additions & 1 deletion source/common/access_log/access_log_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,10 @@ bool RuntimeFilter::evaluate(const StreamInfo::StreamInfo&, const Http::HeaderMa
const Http::HeaderMap&, const Http::HeaderMap&) {
const Http::HeaderEntry* uuid = request_header.RequestId();
uint64_t random_value;
// TODO(dnoe): Migrate uuidModBy to take string_view (#6580)
if (use_independent_randomness_ || uuid == nullptr ||
!UuidUtils::uuidModBy(
uuid->value().c_str(), random_value,
std::string(uuid->value().getStringView()), random_value,
ProtobufPercentHelper::fractionalPercentDenominatorToInt(percent_.denominator()))) {
random_value = random_.random();
}
Expand Down
13 changes: 5 additions & 8 deletions source/common/common/utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -572,14 +572,13 @@ template <class Value> struct TrieLookupTable {
* exists.
* @return false when a value already exists for the given key.
*/
bool add(const char* key, Value value, bool overwrite_existing = true) {
bool add(absl::string_view key, Value value, bool overwrite_existing = true) {
TrieEntry<Value>* current = &root_;
while (uint8_t c = *key) {
for (uint8_t c : key) {
if (!current->entries_[c]) {
current->entries_[c] = std::make_unique<TrieEntry<Value>>();
}
current = current->entries_[c].get();
key++;
}
if (current->value_ && !overwrite_existing) {
return false;
Expand All @@ -593,13 +592,11 @@ template <class Value> struct TrieLookupTable {
* @param key the key used to find.
* @return the value associated with the key.
*/
Value find(const char* key) const {
Value find(absl::string_view key) const {
const TrieEntry<Value>* current = &root_;
while (uint8_t c = *key) {
for (uint8_t c : key) {
current = current->entries_[c].get();
if (current) {
key++;
} else {
if (current == nullptr) {
return nullptr;
}
}
Expand Down
32 changes: 17 additions & 15 deletions source/common/grpc/common.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ bool Common::hasGrpcContentType(const Http::HeaderMap& headers) {
absl::StartsWith(content_type->value().getStringView(),
Http::Headers::get().ContentTypeValues.Grpc) &&
(content_type->value().size() == Http::Headers::get().ContentTypeValues.Grpc.size() ||
content_type->value().c_str()[Http::Headers::get().ContentTypeValues.Grpc.size()] == '+');
content_type->value()
.getStringView()[Http::Headers::get().ContentTypeValues.Grpc.size()] == '+');
}

bool Common::isGrpcResponseHeader(const Http::HeaderMap& headers, bool end_stream) {
Expand All @@ -53,11 +54,13 @@ void Common::chargeStat(const Upstream::ClusterInfo& cluster, const std::string&
}
cluster.statsScope()
.counter(fmt::format("{}.{}.{}.{}", protocol, grpc_service, grpc_method,
grpc_status->value().c_str()))
grpc_status->value().getStringView()))
.inc();
uint64_t grpc_status_code;
const std::string grpc_status_string(grpc_status->value().getStringView());
// TODO(dnoe): Migrate to pure string_view (#6580)
const bool success =
StringUtil::atoull(grpc_status->value().c_str(), grpc_status_code) && grpc_status_code == 0;
StringUtil::atoull(grpc_status_string.c_str(), grpc_status_code) && grpc_status_code == 0;
chargeStat(cluster, protocol, grpc_service, grpc_method, success);
}

Expand Down Expand Up @@ -85,7 +88,9 @@ absl::optional<Status::GrpcStatus> Common::getGrpcStatus(const Http::HeaderMap&
if (!grpc_status_header || grpc_status_header->value().empty()) {
return absl::optional<Status::GrpcStatus>();
}
if (!StringUtil::atoull(grpc_status_header->value().c_str(), grpc_status_code) ||
// TODO(dnoe): Migrate to pure string_view (#6580)
std::string grpc_status_header_string(grpc_status_header->value().getStringView());
if (!StringUtil::atoull(grpc_status_header_string.c_str(), grpc_status_code) ||
grpc_status_code > Status::GrpcStatus::MaximumValid) {
return absl::optional<Status::GrpcStatus>(Status::GrpcStatus::InvalidCode);
}
Expand All @@ -94,15 +99,15 @@ absl::optional<Status::GrpcStatus> Common::getGrpcStatus(const Http::HeaderMap&

std::string Common::getGrpcMessage(const Http::HeaderMap& trailers) {
const auto entry = trailers.GrpcMessage();
return entry ? entry->value().c_str() : EMPTY_STRING;
return entry ? std::string(entry->value().getStringView()) : EMPTY_STRING;
}

bool Common::resolveServiceAndMethod(const Http::HeaderEntry* path, std::string* service,
std::string* method) {
if (path == nullptr || path->value().c_str() == nullptr) {
if (path == nullptr) {
return false;
}
const auto parts = StringUtil::splitToken(path->value().c_str(), "/");
const auto parts = StringUtil::splitToken(path->value().getStringView(), "/");
if (parts.size() != 2) {
return false;
}
Expand Down Expand Up @@ -138,8 +143,9 @@ std::chrono::milliseconds Common::getGrpcTimeout(Http::HeaderMap& request_header
Http::HeaderEntry* header_grpc_timeout_entry = request_headers.GrpcTimeout();
if (header_grpc_timeout_entry) {
uint64_t grpc_timeout;
const char* unit =
StringUtil::strtoull(header_grpc_timeout_entry->value().c_str(), grpc_timeout);
// TODO(dnoe): Migrate to pure string_view (#6580)
std::string grpc_timeout_string(header_grpc_timeout_entry->value().getStringView());
const char* unit = StringUtil::strtoull(grpc_timeout_string.c_str(), grpc_timeout);
if (unit != nullptr && *unit != '\0') {
switch (*unit) {
case 'H':
Expand Down Expand Up @@ -231,9 +237,7 @@ void Common::checkForHeaderOnlyError(Http::Message& http_response) {
throw Exception(absl::optional<uint64_t>(), "bad grpc-status header");
}

const Http::HeaderEntry* grpc_status_message = http_response.headers().GrpcMessage();
throw Exception(grpc_status_code.value(),
grpc_status_message ? grpc_status_message->value().c_str() : EMPTY_STRING);
throw Exception(grpc_status_code.value(), Common::getGrpcMessage(http_response.headers()));
}

void Common::validateResponse(Http::Message& http_response) {
Expand All @@ -255,9 +259,7 @@ void Common::validateResponse(Http::Message& http_response) {
}

if (grpc_status_code.value() != 0) {
const Http::HeaderEntry* grpc_status_message = http_response.trailers()->GrpcMessage();
throw Exception(grpc_status_code.value(),
grpc_status_message ? grpc_status_message->value().c_str() : EMPTY_STRING);
throw Exception(grpc_status_code.value(), Common::getGrpcMessage(*http_response.trailers()));
}
}

Expand Down
3 changes: 2 additions & 1 deletion source/common/grpc/google_async_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,8 @@ void GoogleAsyncStreamImpl::initialize(bool /*buffer_body_for_retry*/) {
initial_metadata.iterate(
[](const Http::HeaderEntry& header, void* ctxt) {
auto* client_context = static_cast<grpc::ClientContext*>(ctxt);
client_context->AddMetadata(header.key().c_str(), header.value().c_str());
client_context->AddMetadata(std::string(header.key().getStringView()),
std::string(header.value().getStringView()));
return Http::HeaderMap::Iterate::Continue;
},
&ctxt_);
Expand Down
2 changes: 1 addition & 1 deletion source/common/http/async_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ void AsyncStreamImpl::encodeTrailers(HeaderMapPtr&& trailers) {
}

void AsyncStreamImpl::sendHeaders(HeaderMap& headers, bool end_stream) {
if (Http::Headers::get().MethodValues.Head == headers.Method()->value().c_str()) {
if (Http::Headers::get().MethodValues.Head == headers.Method()->value().getStringView()) {
is_head_request_ = true;
}

Expand Down
13 changes: 8 additions & 5 deletions source/common/http/conn_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,8 @@ const Network::Connection* ConnectionManagerImpl::ActiveStream::connection() {
// e.g. many early returns do not currently handle connection: close properly.
void ConnectionManagerImpl::ActiveStream::decodeHeaders(HeaderMapPtr&& headers, bool end_stream) {
request_headers_ = std::move(headers);
if (Http::Headers::get().MethodValues.Head == request_headers_->Method()->value().c_str()) {
if (Http::Headers::get().MethodValues.Head ==
request_headers_->Method()->value().getStringView()) {
is_head_request_ = true;
}
ENVOY_STREAM_LOG(debug, "request headers complete (end_stream={}):\n{}", *this, end_stream,
Expand Down Expand Up @@ -661,7 +662,8 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(HeaderMapPtr&& headers,
// when the allow_absolute_url flag is enabled on the HCM.
// https://tools.ietf.org/html/rfc7230#section-5.3 We also need to check for the existence of
// :path because CONNECT does not have a path, and we don't support that currently.
if (!request_headers_->Path() || request_headers_->Path()->value().c_str()[0] != '/') {
if (!request_headers_->Path() || request_headers_->Path()->value().getStringView().empty() ||
request_headers_->Path()->value().getStringView()[0] != '/') {
connection_manager_.stats_.named_.downstream_rq_non_relative_path_.inc();
sendLocalReply(Grpc::Common::hasGrpcContentType(*request_headers_), Code::NotFound, "", nullptr,
is_head_request_, absl::nullopt);
Expand Down Expand Up @@ -779,7 +781,8 @@ void ConnectionManagerImpl::ActiveStream::traceRequest() {
// should be used to override the active span's operation.
if (req_operation_override) {
if (!req_operation_override->value().empty()) {
active_span_->setOperation(req_operation_override->value().c_str());
// TODO(dnoe): Migrate setOperation to take string_view (#6580)
active_span_->setOperation(std::string(req_operation_override->value().getStringView()));

// Clear the decorated operation so won't be used in the response header, as
// it has been overridden by the inbound decorator operation request header.
Expand Down Expand Up @@ -1289,7 +1292,7 @@ void ConnectionManagerImpl::ActiveStream::encodeHeaders(ActiveStreamEncoderFilte
// should be used to override the active span's operation.
if (resp_operation_override) {
if (!resp_operation_override->value().empty() && active_span_) {
active_span_->setOperation(resp_operation_override->value().c_str());
active_span_->setOperation(std::string(resp_operation_override->value().getStringView()));
}
// Remove header so not propagated to service.
headers.removeEnvoyDecoratorOperation();
Expand Down Expand Up @@ -1588,7 +1591,7 @@ bool ConnectionManagerImpl::ActiveStream::createFilterChain() {
}

if (connection_manager_.config_.filterFactory().createUpgradeFilterChain(
upgrade->value().c_str(), upgrade_map, *this)) {
upgrade->value().getStringView(), upgrade_map, *this)) {
state_.successful_upgrade_ = true;
connection_manager_.stats_.named_.downstream_cx_upgrades_total_.inc();
connection_manager_.stats_.named_.downstream_cx_upgrades_active_.inc();
Expand Down
3 changes: 2 additions & 1 deletion source/common/http/conn_manager_utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,8 @@ void ConnectionManagerUtility::mutateTracingRequestHeader(HeaderMap& request_hea
return;
}

std::string x_request_id = request_headers.RequestId()->value().c_str();
// TODO(dnoe): Migrate uuidModBy and others below to take string_view (#6580)
std::string x_request_id(request_headers.RequestId()->value().getStringView());
uint64_t result;
// Skip if x-request-id is corrupted.
if (!UuidUtils::uuidModBy(x_request_id, result, 10000)) {
Expand Down
Loading