Skip to content

Send optional entity-body with direct responses #2429

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.

Sign up for GitHub

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

Merged
alyssawilk merged 5 commits into from
Jan 25, 2018
Merged

Send optional entity-body with direct responses #2429

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9aa8efd
Send optional entity-body with direct responses
Jan 22, 2018
7a6404f
Read response body from file only during config load
Jan 23, 2018
b0fe87b
Add integration test, fix ASAN/protobuf false positive, and clean up …
Jan 24, 2018
0094f29
Small code cleanups
Jan 25, 2018
f7c9678
Merge remote-tracking branch 'upstream/master' into direct-response/2315
Jan 25, 2018
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
14 changes: 14 additions & 0 deletions include/envoy/router/router.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,20 @@ class DirectResponseEntry {
* or an empty string otherwise.
*/
virtual std::string newPath(const Http::HeaderMap& headers) const PURE;

/**
* Returns the response body to send with direct responses.
* @return Optional<std::string> the response body specified in the route configuration,
* or an invalid Optional if no response body is specified.
*/
virtual Optional<std::string> responseBody() const PURE;

/**
* Returns the pathname of the file containing the response body to send with direct responses.
* @return Optional<std::string> the response body pathname specified in the route configuration,
* or an invalid Optional if no response body pathname is specified.
*/
virtual Optional<std::string> responseBodyFilename() const PURE;
};

/**
Expand Down
9 changes: 9 additions & 0 deletions source/common/filesystem/filesystem_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "common/filesystem/filesystem_impl.h"

#include <dirent.h>
#include <sys/stat.h>

#include <chrono>
#include <cstdint>
Expand Down Expand Up @@ -39,6 +40,14 @@ bool directoryExists(const std::string& path) {
return dir_exists;
}

ssize_t fileSize(const std::string& path) {
struct stat info;
if (stat(path.c_str(), &info) != 0) {
return -1;
}
return info.st_size;
}

std::string fileReadToEnd(const std::string& path) {
std::ios::sync_with_stdio(false);

Expand Down
8 changes: 8 additions & 0 deletions source/common/filesystem/filesystem_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <chrono>
#include <condition_variable>
#include <cstdint>
#include <cstdlib>
#include <mutex>
#include <string>

Expand Down Expand Up @@ -41,6 +42,13 @@ bool fileExists(const std::string& path);
*/
bool directoryExists(const std::string& path);

/**
* @return ssize_t the size in bytes of the specified file, or -1 if the file size
* cannot be determined for any reason, including without limitation
* the non-existence of the file.
*/
ssize_t fileSize(const std::string& path);

/**
* @return full file content as a string.
* Be aware, this is not most highly performing file reading method.
Expand Down
1 change: 1 addition & 0 deletions source/common/router/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ envoy_cc_library(
"//source/common/common:hex_lib",
"//source/common/common:logger_lib",
"//source/common/common:utility_lib",
"//source/common/filesystem:filesystem_lib",
"//source/common/grpc:common_lib",
"//source/common/http:codes_lib",
"//source/common/http:header_map_lib",
Expand Down
4 changes: 3 additions & 1 deletion source/common/router/config_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,9 @@ RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost,
response_headers_parser_(HeaderParser::configure(route.route().response_headers_to_add(),
route.route().response_headers_to_remove())),
opaque_config_(parseOpaqueConfig(route)), decorator_(parseDecorator(route)),
direct_response_code_(ConfigUtility::parseDirectResponseCode(route)) {
direct_response_code_(ConfigUtility::parseDirectResponseCode(route)),
direct_response_body_(ConfigUtility::parseDirectResponseBody(route)),
direct_response_file_(ConfigUtility::parseDirectResponseFilename(route)) {

@alyssawilk alyssawilk Jan 23, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd suggest checking for file existence and readability and throwing an error on failure.

if (route.route().has_metadata_match()) {
const auto filter_it = route.route().metadata_match().filter_metadata().find(
Envoy::Config::MetadataFilters::get().ENVOY_LB);
Expand Down
6 changes: 6 additions & 0 deletions source/common/router/config_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ class SslRedirector : public DirectResponseEntry {
// Router::DirectResponseEntry
std::string newPath(const Http::HeaderMap& headers) const override;
Http::Code responseCode() const override { return Http::Code::MovedPermanently; }
Optional<std::string> responseBody() const override { return Optional<std::string>(); }

@alyssawilk alyssawilk Jan 23, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Any reason to not kill off the optional and insead return EMPTY_STRING by default? Especially as on config parsing we return an empty optional if the body is empty in any case.

Optional<std::string> responseBodyFilename() const override { return Optional<std::string>(); }
};

class SslRedirectRoute : public Route {
Expand Down Expand Up @@ -334,6 +336,8 @@ class RouteEntryImplBase : public RouteEntry,
// Router::DirectResponseEntry
std::string newPath(const Http::HeaderMap& headers) const override;
Http::Code responseCode() const override { return direct_response_code_.value(); }
Optional<std::string> responseBody() const override { return direct_response_body_; }
Optional<std::string> responseBodyFilename() const override { return direct_response_file_; }

// Router::Route
const DirectResponseEntry* directResponseEntry() const override;
Expand Down Expand Up @@ -492,6 +496,8 @@ class RouteEntryImplBase : public RouteEntry,

const DecoratorConstPtr decorator_;
const Optional<Http::Code> direct_response_code_;
const Optional<std::string> direct_response_body_;
const Optional<std::string> direct_response_file_;
};

/**
Expand Down
27 changes: 27 additions & 0 deletions source/common/router/config_utility.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,33 @@ Optional<Http::Code> ConfigUtility::parseDirectResponseCode(const envoy::api::v2
return Optional<Http::Code>();
}

Optional<std::string> ConfigUtility::parseDirectResponseBody(const envoy::api::v2::Route& route) {
if (route.has_direct_response() && route.direct_response().has_body()) {
const auto& body = route.direct_response().body();
std::string inline_bytes = body.inline_bytes();
if (!inline_bytes.empty()) {
return inline_bytes;
}
std::string inline_string = body.inline_string();
if (!inline_string.empty()) {
return inline_string;
}
}
return Optional<std::string>();
}

Optional<std::string>
ConfigUtility::parseDirectResponseFilename(const envoy::api::v2::Route& route) {
if (route.has_direct_response() && route.direct_response().has_body()) {
const auto& body = route.direct_response().body();
std::string filename = body.filename();
if (!filename.empty()) {
return filename;
}
}
return Optional<std::string>();
}

@PiotrSikora PiotrSikora Jan 26, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry for being late to the party, but:

  1. Why did everybody think that 4KB limit is a good idea?
  2. Iff it's a good idea, then why does it only apply to response body from file, but not to response body inlined in the config?

cc @mattklein123 @alyssawilk

@mattklein123 mattklein123 Jan 26, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The exact number was chosen by @brian-pane. He can comment. It's probably arbitrary. The main idea here is to provide some sanity check to avoid people from streaming huge files which won't reliably work without a lot of effort around buffering, flow control, etc. We should probably apply the same limit whether inline or by file I agree. @brian-pane can you do a follow up?

@brian-pane brian-pane Jan 26, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@PiotrSikora the idea of using a fixed limit arose from a Slack discussion in #envoy-dev on Jan 5th. My original plan was to use sendfile, but Matt had (quite reasonable) concerns about how much complexity that would add to the proxy core, so we settled on reading the file into memory but limiting its size.

@brian-pane brian-pane Jan 26, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

BTW, PR 2458 now contains a fix to apply the same 4KB limit to both inline and file-sourced response bodies.


Http::Code ConfigUtility::parseClusterNotFoundResponseCode(
const envoy::api::v2::RouteAction::ClusterNotFoundResponseCode& code) {
switch (code) {
Expand Down
18 changes: 18 additions & 0 deletions source/common/router/config_utility.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,24 @@ class ConfigUtility {
*/
static Optional<Http::Code> parseDirectResponseCode(const envoy::api::v2::Route& route);

/**
* Returns the content of the response body to send with direct responses from a route.
* @param route supplies the Route configuration.
* @return Optional<std::string> the response body in the route's direct_response if specified,
* or the HTTP status code from the route's redirect if specified,

@zuercher zuercher Jan 23, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this line applies.

* or an empty Option otherwise.
*/
static Optional<std::string> parseDirectResponseBody(const envoy::api::v2::Route& route);

/**
* Returns the pathname of the file containing the response body to send with direct
* responses from a route.
* @param route supplies the Route configuration.
* @return Optional<std::string> the response body filename in the route's direct_response
* if specified, or an empty Option otherwise.
*/
static Optional<std::string> parseDirectResponseFilename(const envoy::api::v2::Route& route);

/**
* Returns the HTTP Status Code enum parsed from proto.
* @param code supplies the ClusterNotFoundResponseCode enum.
Expand Down
20 changes: 18 additions & 2 deletions source/common/router/router.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "common/common/empty_string.h"
#include "common/common/enum_to_int.h"
#include "common/common/utility.h"
#include "common/filesystem/filesystem_impl.h"
#include "common/grpc/common.h"
#include "common/http/codes.h"
#include "common/http/header_map_impl.h"
Expand Down Expand Up @@ -219,8 +220,23 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e
return Http::FilterHeadersStatus::StopIteration;
}
config_.stats_.rq_direct_response_.inc();
sendLocalReply(route_->directResponseEntry()->responseCode(), "", false);
// TODO(brian-pane) support sending a response body and response_headers_to_add.
std::string body;

@alyssawilk alyssawilk Jan 24, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

const std::string& body = route_->directResponseEntry()->responseBody(); ?

Optional<std::string> inline_body = route_->directResponseEntry()->responseBody();
if (inline_body.valid()) {
body = inline_body.value();
}
Optional<std::string> path = route_->directResponseEntry()->responseBodyFilename();
if (path.valid()) {
const ssize_t kMaxFileSize = 4096;
ssize_t file_size = Filesystem::fileSize(path.value());
if (file_size < 0 || file_size > kMaxFileSize) {

@alyssawilk alyssawilk Jan 23, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

While file size can change (so totally check here) it's probably worth best-effort validation of file size at config load time. Also is the limitation documented anywhere?

@brian-pane brian-pane Jan 24, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm planning to add documentation for the feature with a follow-up PR that adds the one remaining part of the implementation: support for sending the response_headers_to_add along with direct responses like these.

sendLocalReply(Http::Code::InternalServerError, "", false);
return Http::FilterHeadersStatus::StopIteration;
}
body = Filesystem::fileReadToEnd(path.value());

@zuercher zuercher Jan 23, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This may throw if path is a directory or exists but is not readable. I expect that probably turns into an internal server error elsewhere, but wanted to point it out in case that's not the desired behavior.

@mattklein123 mattklein123 Jan 23, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would strongly advise not reading this file during serving. Instead, I would read it into memory during route config load, and then just sent the pre-read buffer. Length sanity checking, etc. can be handled at that point.

@brian-pane brian-pane Jan 23, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@mattklein123 are you proposing to watch for changes to the file (e.g., by adding an inotify fd to an existing epoll loop) and re-read when it changes?

@mattklein123 mattklein123 Jan 23, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm proposing just reading it once at config load time, and documenting very well the behavior. In general, I feel that this should be generally sufficient for the ways that people are likely to use this behavior. (More generally, I would prefer for Envoy to not become a generic web server, and if we go in that direction I feel that it should be done in a new filter and not as part of the router).

I think this is much easier to reason about from an error handling perspective.

@brian-pane brian-pane Jan 23, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

My concern is whether this would set a bad precedent by being the first Envoy feature that requires a manual restart or reload to pick up a configuration change. Or do we already have that issue with TLS cert and key files?

@mattklein123 mattklein123 Jan 23, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There are ways to workaround this, for example to rename the file as part of operations, etc. IMO this is an OK compromise for this feature as long as it is well documented. We could add watching in the future as a separate feature if desired.

If we want to move forward w/ this implementation we definitely must catch exceptions and handle, as well as understand potential perf issues (though the file will likely be in cache).

@brian-pane brian-pane Jan 23, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I'll go with the simple read-at-startup approach for now.

@alyssawilk alyssawilk Jan 24, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think read-at-startup is fine for this feature, brian you can ignore the rest of this :-P

That said, do we have a plan for files for v2 in general? This could be enabling pushing files via RPC, some config command which says "reload files and tell me if config is valid", disallowing files when you're getting live config or even documenting "rename rather than change contents of your files or you will shoot yourself in the foot". @htuch

@mattklein123 mattklein123 Jan 25, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@alyssawilk in general I think we do not currently have a great plan/roadmap for files. This is exemplified in #2241. IMO if we expect Envoy to really start heavily using files in substantial production deployments we need to put a lot more thought into the entire situation (e.g., there are definitely bugs in the current code where if someone deletes an xDS file after initial error checking Envoy will probably crash). We should probably track in a dedicated ticket.

@brian-pane brian-pane Jan 25, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Off the top of my head, I think an RPC to push file changes to Envoy would be a good thing. It would solve all the atomicity problems associated with people using cp rather than mv. In addition, if there were a RESTful push interface, it could provide a simpler alternative to the long-poll JSON-REST pull API.

And it would be nice to have a way to push TLS certificates and keys to Envoy without having to store them in a filesystem on the local host.

}
sendLocalReply(route_->directResponseEntry()->responseCode(), body, false);
// TODO(brian-pane) support sending response_headers_to_add.
return Http::FilterHeadersStatus::StopIteration;
}

Expand Down
8 changes: 8 additions & 0 deletions test/common/filesystem/filesystem_impl_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@ TEST(FileSystemImpl, directoryExists) {
EXPECT_FALSE(Filesystem::directoryExists("/dev/blahblah"));
}

TEST(FileSystemImpl, fileSize) {
EXPECT_EQ(0, Filesystem::fileSize("/dev/null"));
EXPECT_EQ(-1, Filesystem::fileSize("/dev/blahblahblah"));
const std::string data = "test string\ntest";
const std::string file_path = TestEnvironment::writeStringToFileForTest("test_envoy", data);
EXPECT_EQ(data.length(), Filesystem::fileSize(file_path));
}

TEST(FileSystemImpl, fileReadToEndSuccess) {
const std::string data = "test string\ntest";
const std::string file_path = TestEnvironment::writeStringToFileForTest("test_envoy", data);
Expand Down
1 change: 1 addition & 0 deletions test/common/router/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ envoy_cc_test(
"//test/mocks/runtime:runtime_mocks",
"//test/mocks/ssl:ssl_mocks",
"//test/mocks/upstream:upstream_mocks",
"//test/test_common:environment_lib",
"//test/test_common:utility_lib",
],
)
Expand Down
38 changes: 37 additions & 1 deletion test/common/router/config_impl_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2217,7 +2217,20 @@ name: foo
domains: [direct.example.com]
routes:
- match: { prefix: /gone }
direct_response: { status: 410 }
direct_response:
status: 410
body: { inline_bytes: "RXhhbXBsZSB0ZXh0IDE=" }
- match: { prefix: /error }
direct_response:
status: 500
body: { inline_string: "Example text 2" }
- match: { prefix: /no_body }
direct_response:
status: 200
- match: { prefix: /static }
direct_response:
status: 200
body: { filename: /etc/envoy/message }
- match: { prefix: / }
route: { cluster: www2 }
)EOF";
Expand Down Expand Up @@ -2267,6 +2280,29 @@ name: foo
Http::TestHeaderMapImpl headers =
genRedirectHeaders("direct.example.com", "/gone", true, false);
EXPECT_EQ(Http::Code::Gone, config.route(headers, 0)->directResponseEntry()->responseCode());
EXPECT_EQ("Example text 1",
config.route(headers, 0)->directResponseEntry()->responseBody().value());
}
{
Http::TestHeaderMapImpl headers =
genRedirectHeaders("direct.example.com", "/error", true, false);
EXPECT_EQ(Http::Code::InternalServerError,
config.route(headers, 0)->directResponseEntry()->responseCode());
EXPECT_EQ("Example text 2",
config.route(headers, 0)->directResponseEntry()->responseBody().value());
}
{
Http::TestHeaderMapImpl headers =
genRedirectHeaders("direct.example.com", "/no_body", true, false);
EXPECT_EQ(Http::Code::OK, config.route(headers, 0)->directResponseEntry()->responseCode());
EXPECT_FALSE(config.route(headers, 0)->directResponseEntry()->responseBody().valid());
}
{
Http::TestHeaderMapImpl headers =
genRedirectHeaders("direct.example.com", "/static", true, false);
EXPECT_EQ(Http::Code::OK, config.route(headers, 0)->directResponseEntry()->responseCode());
EXPECT_EQ("/etc/envoy/message",
config.route(headers, 0)->directResponseEntry()->responseBodyFilename().value());
}
{
Http::TestHeaderMapImpl headers =
Expand Down
77 changes: 76 additions & 1 deletion test/common/router/router_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "test/mocks/runtime/mocks.h"
#include "test/mocks/ssl/mocks.h"
#include "test/mocks/upstream/mocks.h"
#include "test/test_common/environment.h"
#include "test/test_common/printers.h"
#include "test/test_common/utility.h"

Expand All @@ -28,6 +29,7 @@ using testing::AtLeast;
using testing::Invoke;
using testing::MockFunction;
using testing::NiceMock;
using testing::Ref;
using testing::Return;
using testing::ReturnRef;
using testing::SaveArg;
Expand Down Expand Up @@ -1552,7 +1554,7 @@ TEST_F(RouterTest, RedirectFound) {
}

TEST_F(RouterTest, DirectResponse) {
MockDirectResponseEntry direct_response;
NiceMock<MockDirectResponseEntry> direct_response;
EXPECT_CALL(direct_response, responseCode()).WillRepeatedly(Return(Http::Code::OK));
EXPECT_CALL(*callbacks_.route_, directResponseEntry()).WillRepeatedly(Return(&direct_response));

Expand All @@ -1565,6 +1567,79 @@ TEST_F(RouterTest, DirectResponse) {
EXPECT_EQ(1UL, config_.stats_.rq_direct_response_.value());
}

TEST_F(RouterTest, DirectResponseWithBody) {

@alyssawilk alyssawilk Jan 23, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we get an integration test for one of these, just to make sure end to end with bodies works smoothly?

NiceMock<MockDirectResponseEntry> direct_response;
EXPECT_CALL(direct_response, responseCode()).WillRepeatedly(Return(Http::Code::OK));
EXPECT_CALL(direct_response, responseBody())
.WillRepeatedly(Return(Optional<std::string>("static response")));
EXPECT_CALL(*callbacks_.route_, directResponseEntry()).WillRepeatedly(Return(&direct_response));

Http::TestHeaderMapImpl response_headers{
{":status", "200"}, {"content-length", "15"}, {"content-type", "text/plain"}};
EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), false));
EXPECT_CALL(callbacks_, encodeData(_, true));
Http::TestHeaderMapImpl headers;
HttpTestUtility::addDefaultHeaders(headers);
router_.decodeHeaders(headers, true);
EXPECT_TRUE(verifyHostUpstreamStats(0, 0));
EXPECT_EQ(1UL, config_.stats_.rq_direct_response_.value());
}

TEST_F(RouterTest, DirectResponseWithBodyFromFile) {
{
auto pathname = TestEnvironment::writeStringToFileForTest("response_body",
"content to deliver from a file");
NiceMock<MockDirectResponseEntry> direct_response;
EXPECT_CALL(direct_response, responseCode()).WillRepeatedly(Return(Http::Code::OK));
EXPECT_CALL(direct_response, responseBodyFilename())
.WillRepeatedly(Return(Optional<std::string>(pathname)));
EXPECT_CALL(*callbacks_.route_, directResponseEntry()).WillRepeatedly(Return(&direct_response));
Http::TestHeaderMapImpl response_headers{
{":status", "200"}, {"content-length", "30"}, {"content-type", "text/plain"}};
EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), false));
EXPECT_CALL(callbacks_, encodeData(_, true));
Http::TestHeaderMapImpl headers;
HttpTestUtility::addDefaultHeaders(headers);
router_.decodeHeaders(headers, true);
EXPECT_TRUE(verifyHostUpstreamStats(0, 0));
EXPECT_EQ(1UL, config_.stats_.rq_direct_response_.value());
config_.stats_.rq_direct_response_.reset();
}
{
// Nonexistent file, should return a 500.
NiceMock<MockDirectResponseEntry> direct_response;
EXPECT_CALL(direct_response, responseCode()).WillRepeatedly(Return(Http::Code::OK));
EXPECT_CALL(direct_response, responseBodyFilename())
.WillRepeatedly(Return(Optional<std::string>("nonexistent file")));
EXPECT_CALL(*callbacks_.route_, directResponseEntry()).WillRepeatedly(Return(&direct_response));
Http::TestHeaderMapImpl response_headers{{":status", "500"}};
EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true));
Http::TestHeaderMapImpl headers;
HttpTestUtility::addDefaultHeaders(headers);
router_.decodeHeaders(headers, true);
EXPECT_TRUE(verifyHostUpstreamStats(0, 0));
EXPECT_EQ(1UL, config_.stats_.rq_direct_response_.value());
config_.stats_.rq_direct_response_.reset();
}
{
// File too big, should return a 500.
auto pathname =
TestEnvironment::writeStringToFileForTest("response_body", std::string(4097, '*'));
NiceMock<MockDirectResponseEntry> direct_response;
EXPECT_CALL(direct_response, responseCode()).WillRepeatedly(Return(Http::Code::OK));
EXPECT_CALL(direct_response, responseBodyFilename())
.WillRepeatedly(Return(Optional<std::string>(pathname)));
EXPECT_CALL(*callbacks_.route_, directResponseEntry()).WillRepeatedly(Return(&direct_response));
Http::TestHeaderMapImpl response_headers{{":status", "500"}};
EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true));
Http::TestHeaderMapImpl headers;
HttpTestUtility::addDefaultHeaders(headers);
router_.decodeHeaders(headers, true);
EXPECT_TRUE(verifyHostUpstreamStats(0, 0));
EXPECT_EQ(1UL, config_.stats_.rq_direct_response_.value());
}
}

TEST(RouterFilterUtilityTest, finalTimeout) {
{
NiceMock<MockRouteEntry> route;
Expand Down
2 changes: 2 additions & 0 deletions test/mocks/router/mocks.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ class MockDirectResponseEntry : public DirectResponseEntry {
// DirectResponseEntry
MOCK_CONST_METHOD1(newPath, std::string(const Http::HeaderMap& headers));
MOCK_CONST_METHOD0(responseCode, Http::Code());
MOCK_CONST_METHOD0(responseBody, Optional<std::string>());
MOCK_CONST_METHOD0(responseBodyFilename, Optional<std::string>());
};

class TestCorsPolicy : public CorsPolicy {
Expand Down