Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions include/envoy/buffer/buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ class Instance {
virtual ~Instance() {}

/**
* Copy data into the buffer.
* Copy data into the buffer (deprecated, use absl::string_view variant
* instead).
* TODO(htuch): Cleanup deprecated call sites.
* @param data supplies the data address.
* @param size supplies the data size.
*/
Expand All @@ -74,7 +76,7 @@ class Instance {
* Copy a string into the buffer.
* @param data supplies the string to copy.
*/
virtual void add(const std::string& data) PURE;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove add(const void* data, uint64_t size) variant with this, just leave a TODO?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'll add TODOs. Too many call sites for this PR to cleanup (> 100).

virtual void add(absl::string_view data) PURE;

/**
* Copy another buffer into this buffer.
Expand Down
2 changes: 1 addition & 1 deletion include/envoy/http/filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ class StreamDecoderFilterCallbacks : public virtual StreamFilterCallbacks {
* response headers.
* @param grpc_status the gRPC status code to override the httpToGrpcStatus mapping with.
*/
virtual void sendLocalReply(Code response_code, const std::string& body_text,
virtual void sendLocalReply(Code response_code, absl::string_view body_text,
std::function<void(HeaderMap& headers)> modify_headers,
const absl::optional<Grpc::Status::GrpcStatus> grpc_status) PURE;

Expand Down
6 changes: 4 additions & 2 deletions include/envoy/http/header_map.h
Original file line number Diff line number Diff line change
Expand Up @@ -187,14 +187,16 @@ class HeaderEntry {
virtual const HeaderString& key() const PURE;

/**
* Set the header value by copying data into it.
* Set the header value by copying data into it (deprecated, use absl::string_view variant
* instead).
* TODO(htuch): Cleanup deprecated call sites.
*/
virtual void value(const char* value, uint32_t size) PURE;

/**
* Set the header value by copying data into it.
*/
virtual void value(const std::string& value) PURE;
virtual void value(absl::string_view value) PURE;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here for value(const char* value, uint32_t size)


/**
* Set the header value by copying an integer into it.
Expand Down
6 changes: 3 additions & 3 deletions source/common/buffer/buffer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ void OwnedImpl::addBufferFragment(BufferFragment& fragment) {
[](const void*, size_t, void* arg) { static_cast<BufferFragment*>(arg)->done(); }, &fragment);
}

void OwnedImpl::add(const std::string& data) {
evbuffer_add(buffer_.get(), data.c_str(), data.size());
void OwnedImpl::add(absl::string_view data) {
evbuffer_add(buffer_.get(), data.data(), data.size());
}

void OwnedImpl::add(const Instance& data) {
Expand Down Expand Up @@ -191,7 +191,7 @@ Api::SysCallIntResult OwnedImpl::write(int fd) {

OwnedImpl::OwnedImpl() : buffer_(evbuffer_new()) {}

OwnedImpl::OwnedImpl(const std::string& data) : OwnedImpl() { add(data); }
OwnedImpl::OwnedImpl(absl::string_view data) : OwnedImpl() { add(data); }

OwnedImpl::OwnedImpl(const Instance& data) : OwnedImpl() { add(data); }

Expand Down
4 changes: 2 additions & 2 deletions source/common/buffer/buffer_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,14 @@ class LibEventInstance : public Instance {
class OwnedImpl : public LibEventInstance {
public:
OwnedImpl();
OwnedImpl(const std::string& data);
OwnedImpl(absl::string_view data);
OwnedImpl(const Instance& data);
OwnedImpl(const void* data, uint64_t size);

// LibEventInstance
void add(const void* data, uint64_t size) override;
void addBufferFragment(BufferFragment& fragment) override;
void add(const std::string& data) override;
void add(absl::string_view data) override;
void add(const Instance& data) override;
void prepend(absl::string_view data) override;
void prepend(Instance& data) override;
Expand Down
2 changes: 1 addition & 1 deletion source/common/buffer/watermark_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ void WatermarkBuffer::add(const void* data, uint64_t size) {
checkHighWatermark();
}

void WatermarkBuffer::add(const std::string& data) {
void WatermarkBuffer::add(absl::string_view data) {
OwnedImpl::add(data);
checkHighWatermark();
}
Expand Down
2 changes: 1 addition & 1 deletion source/common/buffer/watermark_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class WatermarkBuffer : public OwnedImpl {
// Override all functions from Instance which can result in changing the size
// of the underlying buffer.
void add(const void* data, uint64_t size) override;
void add(const std::string& data) override;
void add(absl::string_view data) override;
void add(const Instance& data) override;
void prepend(absl::string_view data) override;
void prepend(Instance& data) override;
Expand Down
2 changes: 1 addition & 1 deletion source/common/http/async_client_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ class AsyncStreamImpl : public AsyncClient::Stream,
HeaderMap& addDecodedTrailers() override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }
void addDecodedData(Buffer::Instance&, bool) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }
const Buffer::Instance* decodingBuffer() override { return buffered_body_.get(); }
void sendLocalReply(Code code, const std::string& body,
void sendLocalReply(Code code, absl::string_view body,
std::function<void(HeaderMap& headers)> modify_headers,
const absl::optional<Grpc::Status::GrpcStatus> grpc_status) override {
Utility::sendLocalReply(
Expand Down
2 changes: 1 addition & 1 deletion source/common/http/conn_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -992,7 +992,7 @@ void ConnectionManagerImpl::ActiveStream::refreshCachedRoute() {
}

void ConnectionManagerImpl::ActiveStream::sendLocalReply(
bool is_grpc_request, Code code, const std::string& body,
bool is_grpc_request, Code code, absl::string_view body,
const std::function<void(HeaderMap& headers)>& modify_headers, bool is_head_request,
const absl::optional<Grpc::Status::GrpcStatus> grpc_status) {
ASSERT(response_headers_ == nullptr);
Expand Down
4 changes: 2 additions & 2 deletions source/common/http/conn_manager_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
const Buffer::Instance* decodingBuffer() override {
return parent_.buffered_request_data_.get();
}
void sendLocalReply(Code code, const std::string& body,
void sendLocalReply(Code code, absl::string_view body,
std::function<void(HeaderMap& headers)> modify_headers,
const absl::optional<Grpc::Status::GrpcStatus> grpc_status) override {
parent_.sendLocalReply(is_grpc_request_, code, body, modify_headers, parent_.is_head_request_,
Expand Down Expand Up @@ -284,7 +284,7 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
void maybeEndDecode(bool end_stream);
void addEncodedData(ActiveStreamEncoderFilter& filter, Buffer::Instance& data, bool streaming);
HeaderMap& addEncodedTrailers();
void sendLocalReply(bool is_grpc_request, Code code, const std::string& body,
void sendLocalReply(bool is_grpc_request, Code code, absl::string_view body,
const std::function<void(HeaderMap& headers)>& modify_headers,
bool is_head_request,
const absl::optional<Grpc::Status::GrpcStatus> grpc_status);
Expand Down
4 changes: 2 additions & 2 deletions source/common/http/header_map_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,8 @@ void HeaderMapImpl::HeaderEntryImpl::value(const char* value, uint32_t size) {
value_.setCopy(value, size);
}

void HeaderMapImpl::HeaderEntryImpl::value(const std::string& value) {
this->value(value.c_str(), static_cast<uint32_t>(value.size()));
void HeaderMapImpl::HeaderEntryImpl::value(absl::string_view value) {
this->value(value.data(), static_cast<uint32_t>(value.size()));
}

void HeaderMapImpl::HeaderEntryImpl::value(uint64_t value) { value_.setInteger(value); }
Expand Down
2 changes: 1 addition & 1 deletion source/common/http/header_map_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ class HeaderMapImpl : public HeaderMap, NonCopyable {
// HeaderEntry
const HeaderString& key() const override { return key_; }
void value(const char* value, uint32_t size) override;
void value(const std::string& value) override;
void value(absl::string_view value) override;
void value(uint64_t value) override;
void value(const HeaderEntry& header) override;
const HeaderString& value() const override { return value_; }
Expand Down
4 changes: 2 additions & 2 deletions source/common/http/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ Utility::parseHttp1Settings(const envoy::api::v2::core::Http1ProtocolOptions& co
}

void Utility::sendLocalReply(bool is_grpc, StreamDecoderFilterCallbacks& callbacks,
const bool& is_reset, Code response_code, const std::string& body_text,
const bool& is_reset, Code response_code, absl::string_view body_text,
const absl::optional<Grpc::Status::GrpcStatus> grpc_status,
bool is_head_request) {
sendLocalReply(is_grpc,
Expand All @@ -259,7 +259,7 @@ void Utility::sendLocalReply(bool is_grpc, StreamDecoderFilterCallbacks& callbac
void Utility::sendLocalReply(
bool is_grpc, std::function<void(HeaderMapPtr&& headers, bool end_stream)> encode_headers,
std::function<void(Buffer::Instance& data, bool end_stream)> encode_data, const bool& is_reset,
Code response_code, const std::string& body_text,
Code response_code, absl::string_view body_text,
const absl::optional<Grpc::Status::GrpcStatus> grpc_status, bool is_head_request) {
// encode_headers() may reset the stream, so the stream must not be reset before calling it.
ASSERT(!is_reset);
Expand Down
4 changes: 2 additions & 2 deletions source/common/http/utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ Http1Settings parseHttp1Settings(const envoy::api::v2::core::Http1ProtocolOption
* @param is_head_request tells if this is a response to a HEAD request
*/
void sendLocalReply(bool is_grpc, StreamDecoderFilterCallbacks& callbacks, const bool& is_reset,
Code response_code, const std::string& body_text,
Code response_code, absl::string_view body_text,
const absl::optional<Grpc::Status::GrpcStatus> grpc_status,
bool is_head_request);

Expand All @@ -162,7 +162,7 @@ void sendLocalReply(bool is_grpc, StreamDecoderFilterCallbacks& callbacks, const
void sendLocalReply(bool is_grpc,
std::function<void(HeaderMapPtr&& headers, bool end_stream)> encode_headers,
std::function<void(Buffer::Instance& data, bool end_stream)> encode_data,
const bool& is_reset, Code response_code, const std::string& body_text,
const bool& is_reset, Code response_code, absl::string_view body_text,
const absl::optional<Grpc::Status::GrpcStatus> grpc_status,
bool is_head_request = false);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,9 @@ Http::FilterHeadersStatus JsonTranscoderFilter::decodeHeaders(Http::HeaderMap& h
if (!request_status.ok()) {
ENVOY_LOG(debug, "Transcoding request error {}", request_status.ToString());
error_ = true;
decoder_callbacks_->sendLocalReply(Http::Code::BadRequest, request_status.error_message(),
decoder_callbacks_->sendLocalReply(Http::Code::BadRequest,
absl::string_view(request_status.error_message().data(),
request_status.error_message().size()),
nullptr, absl::nullopt);

return Http::FilterHeadersStatus::StopIteration;
Expand Down Expand Up @@ -281,7 +283,9 @@ Http::FilterDataStatus JsonTranscoderFilter::decodeData(Buffer::Instance& data,
if (!request_status.ok()) {
ENVOY_LOG(debug, "Transcoding request error {}", request_status.ToString());
error_ = true;
decoder_callbacks_->sendLocalReply(Http::Code::BadRequest, request_status.error_message(),
decoder_callbacks_->sendLocalReply(Http::Code::BadRequest,
absl::string_view(request_status.error_message().data(),
request_status.error_message().size()),
nullptr, absl::nullopt);

return Http::FilterDataStatus::StopIterationNoBuffer;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class BufferWrapper : public Buffer::Instance {
std::string toString() const override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }
void add(const void*, uint64_t) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }
void addBufferFragment(Buffer::BufferFragment&) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }
void add(const std::string&) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }
void add(absl::string_view) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }
void add(const Buffer::Instance&) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }
void commit(Buffer::RawSlice*, uint64_t) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }
void prepend(absl::string_view) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,9 @@ class GrpcJsonTranscoderIntegrationTest
response_headers.insertStatus().value(200);
response_headers.insertContentType().value(std::string("application/grpc"));
if (grpc_response_messages.empty()) {
response_headers.insertGrpcStatus().value(grpc_status.error_code());
response_headers.insertGrpcMessage().value(grpc_status.error_message());
response_headers.insertGrpcStatus().value(static_cast<uint64_t>(grpc_status.error_code()));
response_headers.insertGrpcMessage().value(absl::string_view(
grpc_status.error_message().data(), grpc_status.error_message().size()));
upstream_request_->encodeHeaders(response_headers, true);
} else {
response_headers.addCopy(Http::LowerCaseString("trailer"), "Grpc-Status");
Expand All @@ -110,8 +111,9 @@ class GrpcJsonTranscoderIntegrationTest
upstream_request_->encodeData(*buffer, false);
}
Http::TestHeaderMapImpl response_trailers;
response_trailers.insertGrpcStatus().value(grpc_status.error_code());
response_trailers.insertGrpcMessage().value(grpc_status.error_message());
response_trailers.insertGrpcStatus().value(static_cast<uint64_t>(grpc_status.error_code()));
response_trailers.insertGrpcMessage().value(absl::string_view(
grpc_status.error_message().data(), grpc_status.error_message().size()));
upstream_request_->encodeTrailers(response_trailers);
}
EXPECT_TRUE(upstream_request_->complete());
Expand Down
2 changes: 1 addition & 1 deletion test/mocks/http/mocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ class MockStreamDecoderFilterCallbacks : public StreamDecoderFilterCallbacks,
MOCK_METHOD0(decoderBufferLimit, uint32_t());

// Http::StreamDecoderFilterCallbacks
void sendLocalReply(Code code, const std::string& body,
void sendLocalReply(Code code, absl::string_view body,
std::function<void(HeaderMap& headers)> modify_headers,
const absl::optional<Grpc::Status::GrpcStatus> grpc_status) override {
Utility::sendLocalReply(
Expand Down