Merge pull request envoyproxy#133 from kyessenov/harden_context_14

Cherry-pick of envoyproxy#9862
JimmyCYJ · Jan 30, 2020 · b5c068e · b5c068e
2 parents 6d5f90f + 793809c
commit b5c068e
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 4 deletions.
diff --git a/source/extensions/filters/common/expr/context.cc b/source/extensions/filters/common/expr/context.cc
@@ -77,6 +77,9 @@ absl::optional<CelValue> RequestWrapper::operator[](CelValue key) const {
     } else {
       return CelValue::CreateInt64(info_.bytesReceived());
     }
+  } else if (value == TotalSize) {
+    return CelValue::CreateInt64(info_.bytesReceived() +
+                                 (headers_.value_ ? headers_.value_->byteSize().value() : 0));
   } else if (value == Duration) {
     auto duration = info_.requestComplete();
     if (duration.has_value()) {
@@ -106,8 +109,6 @@ absl::optional<CelValue> RequestWrapper::operator[](CelValue key) const {
       return convertHeaderEntry(headers_.value_->RequestId());
     } else if (value == UserAgent) {
       return convertHeaderEntry(headers_.value_->UserAgent());
-    } else if (value == TotalSize) {
-      return CelValue::CreateInt64(info_.bytesReceived() + headers_.value_->byteSize().value());
     }
   }
   return {};
@@ -132,8 +133,9 @@ absl::optional<CelValue> ResponseWrapper::operator[](CelValue key) const {
   } else if (value == Flags) {
     return CelValue::CreateInt64(info_.responseFlags());
   } else if (value == TotalSize) {
-    return CelValue::CreateInt64(info_.bytesSent() + headers_.value_->byteSize().value() +
-                                 trailers_.value_->byteSize().value());
+    return CelValue::CreateInt64(info_.bytesSent() +
+                                 (headers_.value_ ? headers_.value_->byteSize().value() : 0) +
+                                 (trailers_.value_ ? trailers_.value_->byteSize().value() : 0));
   }
   return {};
 }

diff --git a/test/extensions/filters/common/expr/context_test.cc b/test/extensions/filters/common/expr/context_test.cc
@@ -32,12 +32,14 @@ TEST(Context, EmptyHeadersAttributes) {
 
 TEST(Context, RequestAttributes) {
   NiceMock<StreamInfo::MockStreamInfo> info;
+  NiceMock<StreamInfo::MockStreamInfo> empty_info;
   Http::TestHeaderMapImpl header_map{
       {":method", "POST"},           {":scheme", "http"},      {":path", "/meow?yes=1"},
       {":authority", "kittens.com"}, {"referer", "dogs.com"},  {"user-agent", "envoy-mobile"},
       {"content-length", "10"},      {"x-request-id", "blah"},
   };
   RequestWrapper request(&header_map, info);
+  RequestWrapper empty_request(nullptr, empty_info);
 
   EXPECT_CALL(info, bytesReceived()).WillRepeatedly(Return(10));
   // "2018-04-03T23:06:09.123Z".
@@ -66,6 +68,12 @@ TEST(Context, RequestAttributes) {
     ASSERT_TRUE(value.value().IsString());
     EXPECT_EQ("http", value.value().StringOrDie().value());
   }
+
+  {
+    auto value = empty_request[CelValue::CreateString(Scheme)];
+    EXPECT_FALSE(value.has_value());
+  }
+
   {
     auto value = request[CelValue::CreateString(Host)];
     EXPECT_TRUE(value.has_value());
@@ -130,6 +138,14 @@ TEST(Context, RequestAttributes) {
     EXPECT_EQ(138, value.value().Int64OrDie());
   }
 
+  {
+    auto value = empty_request[CelValue::CreateString(TotalSize)];
+    EXPECT_TRUE(value.has_value());
+    ASSERT_TRUE(value.value().IsInt64());
+    // this includes the headers size
+    EXPECT_EQ(0, value.value().Int64OrDie());
+  }
+
   {
     auto value = request[CelValue::CreateString(Time)];
     EXPECT_TRUE(value.has_value());
@@ -187,11 +203,13 @@ TEST(Context, RequestFallbackAttributes) {
 
 TEST(Context, ResponseAttributes) {
   NiceMock<StreamInfo::MockStreamInfo> info;
+  NiceMock<StreamInfo::MockStreamInfo> empty_info;
   const std::string header_name = "test-header";
   const std::string trailer_name = "test-trailer";
   Http::TestHeaderMapImpl header_map{{header_name, "a"}};
   Http::TestHeaderMapImpl trailer_map{{trailer_name, "b"}};
   ResponseWrapper response(&header_map, &trailer_map, info);
+  ResponseWrapper empty_response(nullptr, nullptr, empty_info);
 
   EXPECT_CALL(info, responseCode()).WillRepeatedly(Return(404));
   EXPECT_CALL(info, bytesSent()).WillRepeatedly(Return(123));
@@ -222,6 +240,13 @@ TEST(Context, ResponseAttributes) {
   }
 
 
+  {
+    auto value = empty_response[CelValue::CreateString(TotalSize)];
+    EXPECT_TRUE(value.has_value());
+    ASSERT_TRUE(value.value().IsInt64());
+    EXPECT_EQ(0, value.value().Int64OrDie());
+  }
+
   {
     auto value = response[CelValue::CreateString(Code)];
     EXPECT_TRUE(value.has_value());
@@ -259,6 +284,7 @@ TEST(Context, ResponseAttributes) {
     ASSERT_TRUE(header.value().IsString());
     EXPECT_EQ("b", header.value().StringOrDie().value());
   }
+
   {
     auto value = response[CelValue::CreateString(Flags)];
     EXPECT_TRUE(value.has_value());