From 8c64e475bc0591e2cad3ec2eaec5a9c73ed709fb Mon Sep 17 00:00:00 2001 From: Lizan Zhou Date: Tue, 15 Dec 2020 22:00:59 -0800 Subject: [PATCH 1/2] api: relex inline_string length limitation in DataSource Signed-off-by: Lizan Zhou --- api/envoy/config/core/v3/base.proto | 2 +- api/envoy/config/core/v4alpha/base.proto | 2 +- .../envoy/config/core/v3/base.proto | 2 +- .../envoy/config/core/v4alpha/base.proto | 2 +- source/common/config/datasource.cc | 15 +++-- test/common/config/datasource_test.cc | 30 ++++++++++ .../local_reply_integration_test.cc | 55 +++++++++++++++++++ 7 files changed, 100 insertions(+), 8 deletions(-) diff --git a/api/envoy/config/core/v3/base.proto b/api/envoy/config/core/v3/base.proto index 5b5339ea5bc5..524e2c9283f0 100644 --- a/api/envoy/config/core/v3/base.proto +++ b/api/envoy/config/core/v3/base.proto @@ -334,7 +334,7 @@ message DataSource { bytes inline_bytes = 2 [(validate.rules).bytes = {min_len: 1}]; // String inlined in the configuration. - string inline_string = 3 [(validate.rules).string = {min_len: 1}]; + string inline_string = 3; } } diff --git a/api/envoy/config/core/v4alpha/base.proto b/api/envoy/config/core/v4alpha/base.proto index 27b0b356b1a7..7ca71ed12e4e 100644 --- a/api/envoy/config/core/v4alpha/base.proto +++ b/api/envoy/config/core/v4alpha/base.proto @@ -332,7 +332,7 @@ message DataSource { bytes inline_bytes = 2 [(validate.rules).bytes = {min_len: 1}]; // String inlined in the configuration. - string inline_string = 3 [(validate.rules).string = {min_len: 1}]; + string inline_string = 3; } } diff --git a/generated_api_shadow/envoy/config/core/v3/base.proto b/generated_api_shadow/envoy/config/core/v3/base.proto index 1184c89de6e2..8213176e6c31 100644 --- a/generated_api_shadow/envoy/config/core/v3/base.proto +++ b/generated_api_shadow/envoy/config/core/v3/base.proto @@ -332,7 +332,7 @@ message DataSource { bytes inline_bytes = 2 [(validate.rules).bytes = {min_len: 1}]; // String inlined in the configuration. - string inline_string = 3 [(validate.rules).string = {min_len: 1}]; + string inline_string = 3; } } diff --git a/generated_api_shadow/envoy/config/core/v4alpha/base.proto b/generated_api_shadow/envoy/config/core/v4alpha/base.proto index 95ca4f77a2bc..8937a6a0bca9 100644 --- a/generated_api_shadow/envoy/config/core/v4alpha/base.proto +++ b/generated_api_shadow/envoy/config/core/v4alpha/base.proto @@ -339,7 +339,7 @@ message DataSource { bytes inline_bytes = 2 [(validate.rules).bytes = {min_len: 1}]; // String inlined in the configuration. - string inline_string = 3 [(validate.rules).string = {min_len: 1}]; + string inline_string = 3; } } diff --git a/source/common/config/datasource.cc b/source/common/config/datasource.cc index 776061a61be2..3f60da031a28 100644 --- a/source/common/config/datasource.cc +++ b/source/common/config/datasource.cc @@ -15,20 +15,27 @@ static constexpr uint32_t RetryCount = 1; std::string read(const envoy::config::core::v3::DataSource& source, bool allow_empty, Api::Api& api) { + std::string data; switch (source.specifier_case()) { case envoy::config::core::v3::DataSource::SpecifierCase::kFilename: - return api.fileSystem().fileReadToEnd(source.filename()); + data = api.fileSystem().fileReadToEnd(source.filename()); + break; case envoy::config::core::v3::DataSource::SpecifierCase::kInlineBytes: - return source.inline_bytes(); + data = source.inline_bytes(); + break; case envoy::config::core::v3::DataSource::SpecifierCase::kInlineString: - return source.inline_string(); + data = source.inline_string(); + break; default: if (!allow_empty) { throw EnvoyException( fmt::format("Unexpected DataSource::specifier_case(): {}", source.specifier_case())); } - return ""; } + if (!allow_empty && data.empty()) { + throw EnvoyException("DataSource cannot be empty"); + } + return data; } absl::optional getPath(const envoy::config::core::v3::DataSource& source) { diff --git a/test/common/config/datasource_test.cc b/test/common/config/datasource_test.cc index 70860a7fefd2..8e2900b38c4a 100644 --- a/test/common/config/datasource_test.cc +++ b/test/common/config/datasource_test.cc @@ -105,6 +105,36 @@ TEST_F(AsyncDataSourceTest, LoadLocalDataSource) { EXPECT_EQ(async_data, "xxxxxx"); } +TEST_F(AsyncDataSourceTest, LoadLocalEmptyDataSource) { + AsyncDataSourcePb config; + + std::string yaml = R"EOF( + local: + inline_string: "" + )EOF"; + TestUtility::loadFromYamlAndValidate(yaml, config); + EXPECT_TRUE(config.has_local()); + + std::string async_data; + + EXPECT_CALL(init_manager_, add(_)).WillOnce(Invoke([this](const Init::Target& target) { + init_target_handle_ = target.createHandle("test"); + })); + + local_data_provider_ = std::make_unique( + init_manager_, config.local(), true, *api_, [&](const std::string& data) { + EXPECT_EQ(init_manager_.state(), Init::Manager::State::Initializing); + EXPECT_EQ(data, ""); + async_data = data; + }); + + EXPECT_CALL(init_manager_, state()).WillOnce(Return(Init::Manager::State::Initializing)); + EXPECT_CALL(init_watcher_, ready()); + + init_target_handle_->initialize(init_watcher_); + EXPECT_EQ(async_data, ""); +} + TEST_F(AsyncDataSourceTest, LoadRemoteDataSourceNoCluster) { AsyncDataSourcePb config; diff --git a/test/integration/local_reply_integration_test.cc b/test/integration/local_reply_integration_test.cc index 74da5ae0c25a..8bf22764eb10 100644 --- a/test/integration/local_reply_integration_test.cc +++ b/test/integration/local_reply_integration_test.cc @@ -411,4 +411,59 @@ TEST_P(LocalReplyIntegrationTest, ShouldFormatResponseToCustomString) { EXPECT_EQ(response->body(), "513 - customized body text"); } +// Should return formatted text/plain response. +TEST_P(LocalReplyIntegrationTest, ShouldFormatResponseToEmptyBody) { + const std::string yaml = R"EOF( +mappers: +- filter: + status_code_filter: + comparison: + op: EQ + value: + default_value: 503 + runtime_key: key_b + status_code: 513 + body: + inline_string: "" +body_format: + text_format_source: + inline_string: "" +)EOF"; + setLocalReplyConfig(yaml); + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto encoder_decoder = codec_client_->startRequest( + Http::TestRequestHeaderMapImpl{{":method", "POST"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {":authority", "host"}, + {"test-header", "exact-match-value-2"}}); + auto response = std::move(encoder_decoder.second); + + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); + + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); + ASSERT_TRUE(upstream_request_->waitForHeadersComplete()); + ASSERT_TRUE(fake_upstream_connection_->close()); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); + response->waitForEndStream(); + + if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) { + ASSERT_TRUE(codec_client_->waitForDisconnect()); + } else { + codec_client_->close(); + } + + EXPECT_FALSE(upstream_request_->complete()); + EXPECT_EQ(0U, upstream_request_->bodyLength()); + + EXPECT_TRUE(response->complete()); + + EXPECT_EQ("513", response->headers().Status()->value().getStringView()); + + EXPECT_EQ(response->body(), ""); +} + } // namespace Envoy From 8bf5302cba64a1bb2ed7a59e41edf0b917da237d Mon Sep 17 00:00:00 2001 From: Lizan Zhou Date: Thu, 17 Dec 2020 16:02:37 -0800 Subject: [PATCH 2/2] relax inline_bytes as well Signed-off-by: Lizan Zhou --- api/envoy/config/core/v3/base.proto | 2 +- api/envoy/config/core/v4alpha/base.proto | 2 +- generated_api_shadow/envoy/config/core/v3/base.proto | 2 +- generated_api_shadow/envoy/config/core/v4alpha/base.proto | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/api/envoy/config/core/v3/base.proto b/api/envoy/config/core/v3/base.proto index 524e2c9283f0..74a7d55a7374 100644 --- a/api/envoy/config/core/v3/base.proto +++ b/api/envoy/config/core/v3/base.proto @@ -331,7 +331,7 @@ message DataSource { string filename = 1 [(validate.rules).string = {min_len: 1}]; // Bytes inlined in the configuration. - bytes inline_bytes = 2 [(validate.rules).bytes = {min_len: 1}]; + bytes inline_bytes = 2; // String inlined in the configuration. string inline_string = 3; diff --git a/api/envoy/config/core/v4alpha/base.proto b/api/envoy/config/core/v4alpha/base.proto index 7ca71ed12e4e..6a967b1ae5f2 100644 --- a/api/envoy/config/core/v4alpha/base.proto +++ b/api/envoy/config/core/v4alpha/base.proto @@ -329,7 +329,7 @@ message DataSource { string filename = 1 [(validate.rules).string = {min_len: 1}]; // Bytes inlined in the configuration. - bytes inline_bytes = 2 [(validate.rules).bytes = {min_len: 1}]; + bytes inline_bytes = 2; // String inlined in the configuration. string inline_string = 3; diff --git a/generated_api_shadow/envoy/config/core/v3/base.proto b/generated_api_shadow/envoy/config/core/v3/base.proto index 8213176e6c31..807045fde4c9 100644 --- a/generated_api_shadow/envoy/config/core/v3/base.proto +++ b/generated_api_shadow/envoy/config/core/v3/base.proto @@ -329,7 +329,7 @@ message DataSource { string filename = 1 [(validate.rules).string = {min_len: 1}]; // Bytes inlined in the configuration. - bytes inline_bytes = 2 [(validate.rules).bytes = {min_len: 1}]; + bytes inline_bytes = 2; // String inlined in the configuration. string inline_string = 3; diff --git a/generated_api_shadow/envoy/config/core/v4alpha/base.proto b/generated_api_shadow/envoy/config/core/v4alpha/base.proto index 8937a6a0bca9..78fb00882e2c 100644 --- a/generated_api_shadow/envoy/config/core/v4alpha/base.proto +++ b/generated_api_shadow/envoy/config/core/v4alpha/base.proto @@ -336,7 +336,7 @@ message DataSource { string filename = 1 [(validate.rules).string = {min_len: 1}]; // Bytes inlined in the configuration. - bytes inline_bytes = 2 [(validate.rules).bytes = {min_len: 1}]; + bytes inline_bytes = 2; // String inlined in the configuration. string inline_string = 3;