diff --git a/source/extensions/filters/http/cache/cache_filter.cc b/source/extensions/filters/http/cache/cache_filter.cc index 867c102a75f5d..3f57ccf30c554 100644 --- a/source/extensions/filters/http/cache/cache_filter.cc +++ b/source/extensions/filters/http/cache/cache_filter.cc @@ -90,8 +90,9 @@ Http::FilterHeadersStatus CacheFilter::encodeHeaders(Http::ResponseHeaderMap& he if (filter_state_ == FilterState::ValidatingCachedResponse && isResponseNotModified(headers)) { processSuccessfulValidation(headers); - // Stop the encoding stream until the cached response is fetched & added to the encoding stream. - return Http::FilterHeadersStatus::StopIteration; + // Continue encoding the headers but do not end the stream as the response body is yet to be + // injected. + return Http::FilterHeadersStatus::ContinueAndDontEndStream; } // Either a cache miss or a cache entry that is no longer valid. @@ -113,10 +114,6 @@ Http::FilterDataStatus CacheFilter::encodeData(Buffer::Instance& data, bool end_ // cached response was found and is being added to the encoding stream -- ignore it. return Http::FilterDataStatus::Continue; } - if (filter_state_ == FilterState::EncodeServingFromCache) { - // Stop the encoding stream until the cached response is fetched & added to the encoding stream. - return Http::FilterDataStatus::StopIterationAndBuffer; - } if (insert_) { ENVOY_STREAM_LOG(debug, "CacheFilter::encodeData inserting body", *encoder_callbacks_); // TODO(toddmgreer): Wait for the cache if necessary. @@ -126,6 +123,17 @@ Http::FilterDataStatus CacheFilter::encodeData(Buffer::Instance& data, bool end_ return Http::FilterDataStatus::Continue; } +void CacheFilter::onAboveWriteBufferHighWatermark() { ++high_watermark_calls_; } + +void CacheFilter::onBelowWriteBufferLowWatermark() { + ASSERT(high_watermark_calls_ > 0); + --high_watermark_calls_; + if (!remaining_ranges_.empty()) { + // Fetching the cached response body was stopped, continue if possible. + maybeGetBody(); + } +} + void CacheFilter::getHeaders(Http::RequestHeaderMap& request_headers) { ASSERT(lookup_, "CacheFilter is trying to call getHeaders with no LookupContext"); @@ -165,9 +173,21 @@ void CacheFilter::getHeaders(Http::RequestHeaderMap& request_headers) { }); } -void CacheFilter::getBody() { +void CacheFilter::maybeGetBody() { ASSERT(lookup_, "CacheFilter is trying to call getBody with no LookupContext"); ASSERT(!remaining_ranges_.empty(), "No reason to call getBody when there's no body to get."); + + if (high_watermark_calls_ > 0 || ongoing_fetch_) { + return; + } + ongoing_fetch_ = true; + + // Make sure we are not fetching a chunk of data larger than the encoding buffer limit. + uint64_t begin = remaining_ranges_[0].begin(); + uint64_t end = std::min(remaining_ranges_[0].end(), + remaining_ranges_[0].begin() + encoder_callbacks_->encoderBufferLimit()); + AdjustedByteRange range_to_fetch = {begin, end}; + // If the cache posts a callback to the dispatcher then the CacheFilter is destroyed for any // reason (e.g client disconnected and HTTP stream terminated), then there is no guarantee that // the posted callback will run before the filter is deleted. Hence, a weak_ptr to the CacheFilter @@ -177,8 +197,8 @@ void CacheFilter::getBody() { // The dispatcher needs to be captured because there's no guarantee that // decoder_callbacks_->dispatcher() is thread-safe. - lookup_->getBody(remaining_ranges_[0], [self, &dispatcher = decoder_callbacks_->dispatcher()]( - Buffer::InstancePtr&& body) { + lookup_->getBody(range_to_fetch, [self, &dispatcher = decoder_callbacks_->dispatcher()]( + Buffer::InstancePtr&& body) { // The callback is posted to the dispatcher to make sure it is called on the worker thread. // The lambda passed to dispatcher.post() needs to be copyable as it will be used to // initialize a std::function. Therefore, it cannot capture anything non-copyable. @@ -302,6 +322,8 @@ void CacheFilter::onBody(Buffer::InstancePtr&& body) { "bogus callback."); ASSERT(body, "Cache said it had a body, but isn't giving it to us."); + ongoing_fetch_ = false; + const uint64_t bytes_from_cache = body->length(); if (bytes_from_cache < remaining_ranges_[0].length()) { remaining_ranges_[0].trimFront(bytes_from_cache); @@ -318,14 +340,12 @@ void CacheFilter::onBody(Buffer::InstancePtr&& body) { filter_state_ == FilterState::DecodeServingFromCache ? decoder_callbacks_->encodeData(*body, end_stream) - : encoder_callbacks_->addEncodedData(*body, true); + : encoder_callbacks_->injectEncodedDataToFilterChain(*body, end_stream); if (!remaining_ranges_.empty()) { - getBody(); + maybeGetBody(); } else if (response_has_trailers_) { getTrailers(); - } else { - finalizeEncodingCachedResponse(); } } @@ -339,10 +359,12 @@ void CacheFilter::onTrailers(Http::ResponseTrailerMapPtr&& trailers) { if (filter_state_ == FilterState::DecodeServingFromCache) { decoder_callbacks_->encodeTrailers(std::move(trailers)); } else { - Http::ResponseTrailerMap& response_trailers = encoder_callbacks_->addEncodedTrailers(); - response_trailers = std::move(*trailers); + // The current API does not support this. + // TODO(yosrym93): When trailers support is implemented, a function in + // StreamEncoderFilterCallbacks will need to be implemented to inject trailers. See + // FilterHeadersStatus::ContinueAndDontEndStream docs in filter.h for more details. + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } - finalizeEncodingCachedResponse(); } void CacheFilter::processSuccessfulValidation(Http::ResponseHeaderMap& response_headers) { @@ -456,32 +478,26 @@ void CacheFilter::encodeCachedResponse() { CacheResponseCodeDetails::get().ResponseFromCacheFilter); // If the filter is encoding, 304 response headers and cached headers are merged in encodeHeaders. - // If the filter is decoding, we need to serve response headers from cache directly. + // If the filter is decoding, we need to serve cached response headers directly. if (filter_state_ == FilterState::DecodeServingFromCache) { decoder_callbacks_->encodeHeaders(std::move(lookup_result_->headers_), end_stream, CacheResponseCodeDetails::get().ResponseFromCacheFilter); } + // TODO(yosrym93): Make sure this is the right place to add the callbacks. + decoder_callbacks_->addDownstreamWatermarkCallbacks(*this); + if (lookup_result_->content_length_ > 0) { - // No range has been added, so we add entire body to the response. if (remaining_ranges_.empty()) { + // This is not a range request, encode the entire body. remaining_ranges_.emplace_back(0, lookup_result_->content_length_); } - getBody(); + maybeGetBody(); } else if (response_has_trailers_) { getTrailers(); } } -void CacheFilter::finalizeEncodingCachedResponse() { - if (filter_state_ == FilterState::EncodeServingFromCache) { - // encodeHeaders returned StopIteration waiting for finishing encoding the cached response -- - // continue encoding. - encoder_callbacks_->continueEncoding(); - } - filter_state_ = FilterState::ResponseServedFromCache; -} - } // namespace Cache } // namespace HttpFilters } // namespace Extensions diff --git a/source/extensions/filters/http/cache/cache_filter.h b/source/extensions/filters/http/cache/cache_filter.h index 935128e8154b6..feeb833deb308 100644 --- a/source/extensions/filters/http/cache/cache_filter.h +++ b/source/extensions/filters/http/cache/cache_filter.h @@ -22,26 +22,34 @@ namespace Cache { * A filter that caches responses and attempts to satisfy requests from cache. */ class CacheFilter : public Http::PassThroughFilter, + public Http::DownstreamWatermarkCallbacks, public Logger::Loggable, public std::enable_shared_from_this { public: CacheFilter(const envoy::extensions::filters::http::cache::v3alpha::CacheConfig& config, const std::string& stats_prefix, Stats::Scope& scope, TimeSource& time_source, HttpCache& http_cache); + // Http::StreamFilterBase void onDestroy() override; + // Http::StreamDecoderFilter Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap& headers, bool end_stream) override; + // Http::StreamEncoderFilter Http::FilterHeadersStatus encodeHeaders(Http::ResponseHeaderMap& headers, bool end_stream) override; Http::FilterDataStatus encodeData(Buffer::Instance& buffer, bool end_stream) override; + // Http::DownstreamWatermarkCallbacks + void onAboveWriteBufferHighWatermark() override; + void onBelowWriteBufferLowWatermark() override; + private: // Utility functions; make any necessary checks and call the corresponding lookup_ functions void getHeaders(Http::RequestHeaderMap& request_headers); - void getBody(); + void maybeGetBody(); void getTrailers(); // Callbacks for HttpCache to call when headers/body/trailers are ready. @@ -71,19 +79,13 @@ class CacheFilter : public Http::PassThroughFilter, // or during encoding if a cache entry was validated successfully. void encodeCachedResponse(); - // Precondition: finished adding a response from cache to the response encoding stream. - // Updates filter_state_ and continues the encoding stream if necessary. - void finalizeEncodingCachedResponse(); - TimeSource& time_source_; HttpCache& cache_; LookupContextPtr lookup_; InsertContextPtr insert_; LookupResultPtr lookup_result_; - // Tracks what body bytes still need to be read from the cache. This is - // currently only one Range, but will expand when full range support is added. Initialized by - // onHeaders for Range Responses, otherwise initialized by encodeCachedResponse. + // Tracks what body bytes still need to be read from the cache. std::vector remaining_ranges_; // TODO(#12901): The allow list could be constructed only once directly from the config, instead @@ -100,6 +102,10 @@ class CacheFilter : public Http::PassThroughFilter, // https://httpwg.org/specs/rfc7234.html#response.cacheability bool request_allows_inserts_ = false; + // These are used to keep track of whether we should fetch more data from the cache. + int high_watermark_calls_ = 0; + bool ongoing_fetch_ = false; + enum class FilterState { Initial, @@ -112,10 +118,6 @@ class CacheFilter : public Http::PassThroughFilter, // A cached response was successfully validated and it is being added to the encoding stream EncodeServingFromCache, - // The cached response was successfully added to the encoding stream (either during decoding or - // encoding). - ResponseServedFromCache, - // CacheFilter::onDestroy has been called, the filter will be destroyed soon. Any triggered // callbacks should be ignored. Destroyed diff --git a/test/extensions/filters/http/cache/cache_filter_integration_test.cc b/test/extensions/filters/http/cache/cache_filter_integration_test.cc index f8fb6a830960a..8d82aa7c5e573 100644 --- a/test/extensions/filters/http/cache/cache_filter_integration_test.cc +++ b/test/extensions/filters/http/cache/cache_filter_integration_test.cc @@ -22,6 +22,7 @@ class CacheIntegrationTest : public Event::TestUsingSimulatedTime, void initializeFilter(const std::string& config) { config_helper_.addFilter(config); + config_helper_.setBufferLimits(buffer_limit_, buffer_limit_); initialize(); codec_client_ = makeHttpConnection(makeClientConnection((lookupPort("http")))); } @@ -34,6 +35,7 @@ class CacheIntegrationTest : public Event::TestUsingSimulatedTime, "@type": "type.googleapis.com/envoy.source.extensions.filters.http.cache.SimpleHttpCacheConfig" )EOF"}; DateFormatter formatter_{"%a, %d %b %Y %H:%M:%S GMT"}; + const uint64_t buffer_limit_ = 1024; }; INSTANTIATE_TEST_SUITE_P(Protocols, CacheIntegrationTest, @@ -53,7 +55,10 @@ TEST_P(CacheIntegrationTest, MissInsertHit) { {":scheme", "http"}, {":authority", "MissInsertHit"}}; - const std::string response_body(42, 'a'); + // Use a body arbitrarily greater than the buffer limit to exercise fetching the cached body in + // several chunks. + const std::string response_body(buffer_limit_ * 3.5, 'a'); + Http::TestResponseHeaderMapImpl response_headers = { {":status", "200"}, {"date", formatter_.now(simTime())}, @@ -109,7 +114,10 @@ TEST_P(CacheIntegrationTest, ExpiredValidated) { {":scheme", "http"}, {":authority", "ExpiredValidated"}}; - const std::string response_body(42, 'a'); + // Use a body arbitrarily greater than the buffer limit to exercise fetching the cached body in + // several chunks. + const std::string response_body(buffer_limit_ * 3.5, 'a'); + Http::TestResponseHeaderMapImpl response_headers = { {":status", "200"}, {"date", formatter_.now(simTime())}, diff --git a/test/extensions/filters/http/cache/cache_filter_test.cc b/test/extensions/filters/http/cache/cache_filter_test.cc index e74890235b1ed..c31b466f456a0 100644 --- a/test/extensions/filters/http/cache/cache_filter_test.cc +++ b/test/extensions/filters/http/cache/cache_filter_test.cc @@ -32,10 +32,33 @@ class CacheFilterTest : public ::testing::Test { void SetUp() override { ON_CALL(decoder_callbacks_, dispatcher()).WillByDefault(::testing::ReturnRef(*dispatcher_)); - // Initialize the time source (otherwise it returns the real time) + ON_CALL(encoder_callbacks_, encoderBufferLimit()) + .WillByDefault(::testing::Return(buffer_limit_)); + // Initialize the time source (otherwise it returns the real time). time_source_.setSystemTime(std::chrono::hours(1)); - // Use the initialized time source to set the response date header - response_headers_.setDate(formatter_.now(time_source_)); + // Use the initialized time source to set the response date and last modified headers. + response_date_ = formatter_.now(time_source_); + response_headers_.setDate(response_date_); + response_last_modified_ = formatter_.now(time_source_); + } + + void setBufferLimit(uint64_t buffer_limit) { + buffer_limit_ = buffer_limit; + ON_CALL(encoder_callbacks_, encoderBufferLimit()) + .WillByDefault(::testing::Return(buffer_limit_)); + } + + uint64_t getBufferLimit() const { return buffer_limit_; } + + void generateExpectedDataChunks(const std::string& body) { + ASSERT(!body.empty()); + expected_data_chunks_.clear(); + + size_t i = 0; + while (i < body.size()) { + expected_data_chunks_.push_back(body.substr(i, buffer_limit_)); + i += buffer_limit_; + } } void testDecodeRequestMiss(CacheFilterSharedPtr filter) { @@ -84,17 +107,30 @@ class CacheFilterTest : public ::testing::Test { ::testing::Mock::VerifyAndClearExpectations(&decoder_callbacks_); } - void testDecodeRequestHitWithBody(CacheFilterSharedPtr filter, std::string body) { + // This function assumes that there is a body created using std::string(body_size, 'a'). + void testDecodeRequestHitWithBody(CacheFilterSharedPtr filter, uint64_t body_size) { + ASSERT(body_size > 0); // The filter should encode cached headers. EXPECT_CALL(decoder_callbacks_, encodeHeaders_(testing::AllOf(IsSupersetOfHeaders(response_headers_), HeaderHasValueRef(Http::Headers::get().Age, age)), false)); - // The filter should encode cached data. - EXPECT_CALL( - decoder_callbacks_, - encodeData(testing::Property(&Buffer::Instance::toString, testing::Eq(body)), true)); + // The filter should encode data in chunks sized according to the buffer limit. + const int chunks_count = (body_size + buffer_limit_ - 1) / buffer_limit_; + // The size of all chunks except the last one is equal to the buffer_limit_. + EXPECT_CALL(decoder_callbacks_, + encodeData(testing::Property(&Buffer::Instance::toString, + testing::Eq(std::string(getBufferLimit(), 'a'))), + false)) + .Times(chunks_count - 1); + + const uint64_t last_chunk_size = + body_size % buffer_limit_ == 0 ? buffer_limit_ : body_size % buffer_limit_; + EXPECT_CALL(decoder_callbacks_, + encodeData(testing::Property(&Buffer::Instance::toString, + testing::Eq(std::string(last_chunk_size, 'a'))), + true)); // The filter should stop decoding iteration when decodeHeaders is called as a cache lookup is // in progress. @@ -114,6 +150,59 @@ class CacheFilterTest : public ::testing::Test { ::testing::Mock::VerifyAndClearExpectations(&decoder_callbacks_); } + // This function tests successful validation and verifies that |filter| injects body data in + // correct chunks. + void testSuccessfulValidation(CacheFilterSharedPtr filter, const std::string& body) { + generateExpectedDataChunks(body); + + // Make request require validation. + request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, + Http::CustomHeaders::get().CacheControlValues.NoCache); + + // Decoding the request should find a cached response that requires validation. + // As far as decoding the request is concerned, this is the same as a cache miss with the + // exception of injecting validation precondition headers. + testDecodeRequestMiss(filter); + + // Make sure validation conditional headers are added. + const Http::TestRequestHeaderMapImpl injected_headers = { + {"if-none-match", etag_}, {"if-modified-since", response_last_modified_}}; + EXPECT_THAT(request_headers_, IsSupersetOfHeaders(injected_headers)); + + // Encode 304 response. + // Advance time to make sure the cached date is updated with the 304 date. + time_source_.advanceTimeWait(std::chrono::seconds(10)); + const std::string not_modified_date = formatter_.now(time_source_); + Http::TestResponseHeaderMapImpl not_modified_response_headers = {{":status", "304"}, + {"date", not_modified_date}}; + + // The filter should continue headers encoding without ending the stream as data will be + // injected. + EXPECT_EQ(filter->encodeHeaders(not_modified_response_headers, true), + Http::FilterHeadersStatus::ContinueAndDontEndStream); + + // Check for the cached response headers with updated date. + Http::TestResponseHeaderMapImpl updated_response_headers = response_headers_; + updated_response_headers.setDate(not_modified_date); + EXPECT_THAT(not_modified_response_headers, IsSupersetOfHeaders(updated_response_headers)); + + // The filter should inject data in chunks sized according to the buffer limit. + // Verify that each data chunk injected matches the expectation. + for (size_t i = 0u; i < expected_data_chunks_.size(); i++) { + EXPECT_CALL(encoder_callbacks_, injectEncodedDataToFilterChain( + testing::Property(&Buffer::Instance::toString, + testing::Eq(expected_data_chunks_[i])), + i == expected_data_chunks_.size() - 1u ? true : false)) + .Times(1); + } + + // The cache getBody callback should be posted to the dispatcher. + // Run events on the dispatcher so that the callback is invoked. + dispatcher_->run(Event::Dispatcher::RunType::Block); + + ::testing::Mock::VerifyAndClearExpectations(&encoder_callbacks_); + } + void waitBeforeSecondRequest() { time_source_.advanceTimeWait(delay_); } SimpleHttpCache simple_cache_; @@ -121,16 +210,26 @@ class CacheFilterTest : public ::testing::Test { NiceMock context_; Event::SimulatedTimeSystem time_source_; DateFormatter formatter_{"%a, %d %b %Y %H:%M:%S GMT"}; + Http::TestRequestHeaderMapImpl request_headers_{ {":path", "/"}, {":method", "GET"}, {"x-forwarded-proto", "https"}}; Http::TestResponseHeaderMapImpl response_headers_{{":status", "200"}, {"cache-control", "public,max-age=3600"}}; + NiceMock decoder_callbacks_; NiceMock encoder_callbacks_; + + // Etag and last modified date header values, used for cache validation tests. + std::string response_last_modified_, response_date_, etag_ = "abc123"; + Api::ApiPtr api_ = Api::createApiForTest(); Event::DispatcherPtr dispatcher_ = api_->allocateDispatcher("test_thread"); const Seconds delay_ = Seconds(10); const std::string age = std::to_string(delay_.count()); + +private: + uint64_t buffer_limit_ = 1024; + std::vector expected_data_chunks_; }; TEST_F(CacheFilterTest, UncacheableRequest) { @@ -218,7 +317,8 @@ TEST_F(CacheFilterTest, CacheHitNoBody) { TEST_F(CacheFilterTest, CacheHitWithBody) { request_headers_.setHost("CacheHitWithBody"); - const std::string body = "abc"; + const uint64_t body_size = 3; + const std::string body = std::string(body_size, 'a'); { // Create filter for request 1. @@ -239,7 +339,7 @@ TEST_F(CacheFilterTest, CacheHitWithBody) { // Create filter for request 2 CacheFilterSharedPtr filter = makeFilter(simple_cache_); - testDecodeRequestHitWithBody(filter, body); + testDecodeRequestHitWithBody(filter, body_size); filter->onDestroy(); } @@ -247,9 +347,8 @@ TEST_F(CacheFilterTest, CacheHitWithBody) { TEST_F(CacheFilterTest, SuccessfulValidation) { request_headers_.setHost("SuccessfulValidation"); - const std::string body = "abc"; - const std::string etag = "abc123"; - const std::string last_modified_date = formatter_.now(time_source_); + const std::string body = "123"; + { // Create filter for request 1 CacheFilterSharedPtr filter = makeFilter(simple_cache_); @@ -258,8 +357,9 @@ TEST_F(CacheFilterTest, SuccessfulValidation) { // Encode response // Add Etag & Last-Modified headers to the response for validation - response_headers_.setReferenceKey(Http::CustomHeaders::get().Etag, etag); - response_headers_.setReferenceKey(Http::CustomHeaders::get().LastModified, last_modified_date); + response_headers_.setReferenceKey(Http::CustomHeaders::get().Etag, etag_); + response_headers_.setReferenceKey(Http::CustomHeaders::get().LastModified, + response_last_modified_); Buffer::OwnedImpl buffer(body); response_headers_.setContentLength(body.size()); @@ -272,54 +372,7 @@ TEST_F(CacheFilterTest, SuccessfulValidation) { // Create filter for request 2 CacheFilterSharedPtr filter = makeFilter(simple_cache_); - // Make request require validation - request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, "no-cache"); - - // Decoding the request should find a cached response that requires validation. - // As far as decoding the request is concerned, this is the same as a cache miss with the - // exception of injecting validation precondition headers. - testDecodeRequestMiss(filter); - - // Make sure validation conditional headers are added - const Http::TestRequestHeaderMapImpl injected_headers = { - {"if-none-match", etag}, {"if-modified-since", last_modified_date}}; - EXPECT_THAT(request_headers_, IsSupersetOfHeaders(injected_headers)); - - // Encode 304 response - // Advance time to make sure the cached date is updated with the 304 date - const std::string not_modified_date = formatter_.now(time_source_); - Http::TestResponseHeaderMapImpl not_modified_response_headers = {{":status", "304"}, - {"date", not_modified_date}}; - - // The filter should stop encoding iteration when encodeHeaders is called as a cached response - // is being fetched and added to the encoding stream. StopIteration does not stop encodeData of - // the same filter from being called - EXPECT_EQ(filter->encodeHeaders(not_modified_response_headers, true), - Http::FilterHeadersStatus::StopIteration); - - // Check for the cached response headers with updated date - Http::TestResponseHeaderMapImpl updated_response_headers = response_headers_; - updated_response_headers.setDate(not_modified_date); - EXPECT_THAT(not_modified_response_headers, IsSupersetOfHeaders(updated_response_headers)); - - // A 304 response should not have a body, so encodeData should not be called - // However, if a body is present by mistake, encodeData should stop iteration until - // encoding the cached response is done - Buffer::OwnedImpl not_modified_body; - EXPECT_EQ(filter->encodeData(not_modified_body, true), - Http::FilterDataStatus::StopIterationAndBuffer); - - // The filter should add the cached response body to encoded data. - Buffer::OwnedImpl buffer(body); - EXPECT_CALL( - encoder_callbacks_, - addEncodedData(testing::Property(&Buffer::Instance::toString, testing::Eq(body)), true)); - - // The cache getBody callback should be posted to the dispatcher. - // Run events on the dispatcher so that the callback is invoked. - dispatcher_->run(Event::Dispatcher::RunType::Block); - - ::testing::Mock::VerifyAndClearExpectations(&encoder_callbacks_); + testSuccessfulValidation(filter, body); filter->onDestroy(); } @@ -327,9 +380,8 @@ TEST_F(CacheFilterTest, SuccessfulValidation) { TEST_F(CacheFilterTest, UnsuccessfulValidation) { request_headers_.setHost("UnsuccessfulValidation"); - const std::string body = "abc"; - const std::string etag = "abc123"; - const std::string last_modified_date = formatter_.now(time_source_); + const std::string body = std::string(3, 'a'); + { // Create filter for request 1 CacheFilterSharedPtr filter = makeFilter(simple_cache_); @@ -338,8 +390,9 @@ TEST_F(CacheFilterTest, UnsuccessfulValidation) { // Encode response // Add Etag & Last-Modified headers to the response for validation. - response_headers_.setReferenceKey(Http::CustomHeaders::get().Etag, etag); - response_headers_.setReferenceKey(Http::CustomHeaders::get().LastModified, last_modified_date); + response_headers_.setReferenceKey(Http::CustomHeaders::get().Etag, etag_); + response_headers_.setReferenceKey(Http::CustomHeaders::get().LastModified, + response_last_modified_); Buffer::OwnedImpl buffer(body); response_headers_.setContentLength(body.size()); @@ -352,8 +405,9 @@ TEST_F(CacheFilterTest, UnsuccessfulValidation) { // Create filter for request 2. CacheFilterSharedPtr filter = makeFilter(simple_cache_); - // Make request require validation - request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, "no-cache"); + // Make request require validation. + request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, + Http::CustomHeaders::get().CacheControlValues.NoCache); // Decoding the request should find a cached response that requires validation. // As far as decoding the request is concerned, this is the same as a cache miss with the @@ -362,7 +416,7 @@ TEST_F(CacheFilterTest, UnsuccessfulValidation) { // Make sure validation conditional headers are added. const Http::TestRequestHeaderMapImpl injected_headers = { - {"if-none-match", etag}, {"if-modified-since", last_modified_date}}; + {"if-none-match", etag_}, {"if-modified-since", response_last_modified_}}; EXPECT_THAT(request_headers_, IsSupersetOfHeaders(injected_headers)); // Encode new response. @@ -548,7 +602,7 @@ TEST_F(CacheFilterTest, NotSatisfiableRange) { // Send two identical GET requests with bodies. The CacheFilter will just pass everything through. TEST_F(CacheFilterTest, GetRequestWithBodyAndTrailers) { request_headers_.setHost("GetRequestWithBodyAndTrailers"); - const std::string body = "abc"; + const std::string body = std::string(3, 'a'); Buffer::OwnedImpl request_buffer(body); Http::TestRequestTrailerMapImpl request_trailers; @@ -608,12 +662,228 @@ TEST_F(CacheFilterTest, FilterDeletedBeforePostedCallbackExecuted) { } } -// A new type alias for a different type of tests that use the exact same class +// A new type alias for a different type of tests that use the exact same class. +// In these tests, realistically the data in request 1 should be encoded in several chunks too, +// however, the only purpose of request 1 is to put the response in the cache, so it shouldn't +// matter. +// Cases where the body size is less than the buffer_limit_ are not exercised as they are +// already tested in the above tests. +using CacheChunkSizeTest = CacheFilterTest; + +// Test that a body with size exactly equal to the buffer limit will be encoded in 1 chunk. +TEST_F(CacheChunkSizeTest, EqualBufferLimit) { + request_headers_.setHost("EqualBufferLimit"); + const std::string body = std::string(getBufferLimit(), 'a'); + + { + // Create filter for request 1. + CacheFilterSharedPtr filter = makeFilter(simple_cache_); + + testDecodeRequestMiss(filter); + + // Encode response. + Buffer::OwnedImpl buffer(body); + response_headers_.setContentLength(body.size()); + EXPECT_EQ(filter->encodeHeaders(response_headers_, false), Http::FilterHeadersStatus::Continue); + EXPECT_EQ(filter->encodeData(buffer, true), Http::FilterDataStatus::Continue); + + filter->onDestroy(); + } + waitBeforeSecondRequest(); + { + // Create filter for request 2 + CacheFilterSharedPtr filter = makeFilter(simple_cache_); + + // The body should be encoded in a single chunk. + testDecodeRequestHitWithBody(filter, getBufferLimit()); + + filter->onDestroy(); + } +} + +// Test that a body with size greater than and divisible by buffer limit will be encoded as the +// correct number of chunks. +TEST_F(CacheChunkSizeTest, DivisibleByBufferLimit) { + request_headers_.setHost("DivisibleByBufferLimit"); + const uint64_t body_size = getBufferLimit() * 3; + const std::string body = std::string(body_size, 'a'); + + { + // Create filter for request 1. + CacheFilterSharedPtr filter = makeFilter(simple_cache_); + + testDecodeRequestMiss(filter); + + // Encode response. + Buffer::OwnedImpl buffer(body); + response_headers_.setContentLength(body.size()); + EXPECT_EQ(filter->encodeHeaders(response_headers_, false), Http::FilterHeadersStatus::Continue); + EXPECT_EQ(filter->encodeData(buffer, true), Http::FilterDataStatus::Continue); + + filter->onDestroy(); + } + waitBeforeSecondRequest(); + { + // Create filter for request 2 + CacheFilterSharedPtr filter = makeFilter(simple_cache_); + + // The body should be encoded in 3 chunks. + testDecodeRequestHitWithBody(filter, body_size); + + filter->onDestroy(); + } +} + +// Test that a body with size greater than but not divisible by buffer limit will be encoded as the +// correct number of chunks. +TEST_F(CacheChunkSizeTest, NotDivisbleByBufferLimit) { + request_headers_.setHost("NotDivisbleByBufferLimit"); + const uint64_t body_size = getBufferLimit() * 4.5; + const std::string body = std::string(body_size, 'a'); + + { + // Create filter for request 1. + CacheFilterSharedPtr filter = makeFilter(simple_cache_); + + testDecodeRequestMiss(filter); + + // Encode response. + Buffer::OwnedImpl buffer(body); + response_headers_.setContentLength(body.size()); + EXPECT_EQ(filter->encodeHeaders(response_headers_, false), Http::FilterHeadersStatus::Continue); + EXPECT_EQ(filter->encodeData(buffer, true), Http::FilterDataStatus::Continue); + + filter->onDestroy(); + } + waitBeforeSecondRequest(); + { + // Create filter for request 2 + CacheFilterSharedPtr filter = makeFilter(simple_cache_); + + // The body should be encoded in 5 chunks. + testDecodeRequestHitWithBody(filter, body_size); + + filter->onDestroy(); + } +} + +// Test that a body with size exactly equal to the buffer limit will be encoded in 1 chunk, in the +// case where validation takes place. +TEST_F(CacheChunkSizeTest, EqualBufferLimitWithValidation) { + request_headers_.setHost("EqualBufferLimitWithValidation"); + const std::string body = std::string(getBufferLimit(), 'a'); + + { + // Create filter for request 1. + CacheFilterSharedPtr filter = makeFilter(simple_cache_); + + testDecodeRequestMiss(filter); + + // Encode response. + // Add Etag & Last-Modified headers to the response for validation. + response_headers_.setReferenceKey(Http::CustomHeaders::get().Etag, etag_); + response_headers_.setReferenceKey(Http::CustomHeaders::get().LastModified, + response_last_modified_); + + Buffer::OwnedImpl buffer(body); + response_headers_.setContentLength(body.size()); + EXPECT_EQ(filter->encodeHeaders(response_headers_, false), Http::FilterHeadersStatus::Continue); + EXPECT_EQ(filter->encodeData(buffer, true), Http::FilterDataStatus::Continue); + + filter->onDestroy(); + } + waitBeforeSecondRequest(); + { + // Create filter for request 2 + CacheFilterSharedPtr filter = makeFilter(simple_cache_); + + testSuccessfulValidation(filter, body); + + filter->onDestroy(); + } +} + +// Test that a body with size greater than and divisible by buffer limit will be encoded as the +// correct number of chunks, in the case where validation takes place. +TEST_F(CacheChunkSizeTest, DivisibleByBufferLimitWithValidation) { + request_headers_.setHost("DivisibleByBufferLimitWithValidation"); + + setBufferLimit(5); + const std::string body = "1234567890abcde"; + + { + // Create filter for request 1. + CacheFilterSharedPtr filter = makeFilter(simple_cache_); + + testDecodeRequestMiss(filter); + + // Encode response. + // Add Etag & Last-Modified headers to the response for validation. + response_headers_.setReferenceKey(Http::CustomHeaders::get().Etag, etag_); + response_headers_.setReferenceKey(Http::CustomHeaders::get().LastModified, + response_last_modified_); + + Buffer::OwnedImpl buffer(body); + response_headers_.setContentLength(body.size()); + EXPECT_EQ(filter->encodeHeaders(response_headers_, false), Http::FilterHeadersStatus::Continue); + EXPECT_EQ(filter->encodeData(buffer, true), Http::FilterDataStatus::Continue); + + filter->onDestroy(); + } + waitBeforeSecondRequest(); + { + // Create filter for request 2 + CacheFilterSharedPtr filter = makeFilter(simple_cache_); + + testSuccessfulValidation(filter, body); + + filter->onDestroy(); + } +} + +// Test that a body with size greater than but not divisible by buffer limit will be encoded as the +// correct number of chunks, in the case where validation takes place. +TEST_F(CacheChunkSizeTest, NotDivisbleByBufferLimitWithValidation) { + request_headers_.setHost("NotDivisbleByBufferLimitWithValidation"); + setBufferLimit(5); + + const std::string body = "1234567890abcdefg"; + + { + // Create filter for request 1. + CacheFilterSharedPtr filter = makeFilter(simple_cache_); + + testDecodeRequestMiss(filter); + + // Encode response. + // Add Etag & Last-Modified headers to the response for validation. + response_headers_.setReferenceKey(Http::CustomHeaders::get().Etag, etag_); + response_headers_.setReferenceKey(Http::CustomHeaders::get().LastModified, + response_last_modified_); + + Buffer::OwnedImpl buffer(body); + response_headers_.setContentLength(body.size()); + EXPECT_EQ(filter->encodeHeaders(response_headers_, false), Http::FilterHeadersStatus::Continue); + EXPECT_EQ(filter->encodeData(buffer, true), Http::FilterDataStatus::Continue); + + filter->onDestroy(); + } + waitBeforeSecondRequest(); + { + // Create filter for request 2 + CacheFilterSharedPtr filter = makeFilter(simple_cache_); + + testSuccessfulValidation(filter, body); + + filter->onDestroy(); + } +} + +// A new type alias for a different type of tests that use the exact same class. using ValidationHeadersTest = CacheFilterTest; TEST_F(ValidationHeadersTest, EtagAndLastModified) { request_headers_.setHost("EtagAndLastModified"); - const std::string etag = "abc123"; // Make request 1 to insert the response into cache { @@ -621,9 +891,9 @@ TEST_F(ValidationHeadersTest, EtagAndLastModified) { testDecodeRequestMiss(filter); // Add validation headers to the response - response_headers_.setReferenceKey(Http::CustomHeaders::get().Etag, etag); + response_headers_.setReferenceKey(Http::CustomHeaders::get().Etag, etag_); response_headers_.setReferenceKey(Http::CustomHeaders::get().LastModified, - formatter_.now(time_source_)); + response_last_modified_); filter->encodeHeaders(response_headers_, true); } @@ -632,19 +902,19 @@ TEST_F(ValidationHeadersTest, EtagAndLastModified) { CacheFilterSharedPtr filter = makeFilter(simple_cache_); // Make sure the request requires validation - request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, "no-cache"); + request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, + Http::CustomHeaders::get().CacheControlValues.NoCache); testDecodeRequestMiss(filter); // Make sure validation conditional headers are added const Http::TestRequestHeaderMapImpl injected_headers = { - {"if-none-match", "abc123"}, {"if-modified-since", formatter_.now(time_source_)}}; + {"if-none-match", etag_}, {"if-modified-since", response_last_modified_}}; EXPECT_THAT(request_headers_, IsSupersetOfHeaders(injected_headers)); } } TEST_F(ValidationHeadersTest, EtagOnly) { request_headers_.setHost("EtagOnly"); - const std::string etag = "abc123"; // Make request 1 to insert the response into cache { @@ -652,7 +922,7 @@ TEST_F(ValidationHeadersTest, EtagOnly) { testDecodeRequestMiss(filter); // Add validation headers to the response - response_headers_.setReferenceKey(Http::CustomHeaders::get().Etag, etag); + response_headers_.setReferenceKey(Http::CustomHeaders::get().Etag, etag_); filter->encodeHeaders(response_headers_, true); } @@ -661,13 +931,14 @@ TEST_F(ValidationHeadersTest, EtagOnly) { CacheFilterSharedPtr filter = makeFilter(simple_cache_); // Make sure the request requires validation - request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, "no-cache"); + request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, + Http::CustomHeaders::get().CacheControlValues.NoCache); testDecodeRequestMiss(filter); // Make sure validation conditional headers are added // If-Modified-Since falls back to date - const Http::TestRequestHeaderMapImpl injected_headers = { - {"if-none-match", "abc123"}, {"if-modified-since", formatter_.now(time_source_)}}; + const Http::TestRequestHeaderMapImpl injected_headers = {{"if-none-match", etag_}, + {"if-modified-since", response_date_}}; EXPECT_THAT(request_headers_, IsSupersetOfHeaders(injected_headers)); } } @@ -682,7 +953,7 @@ TEST_F(ValidationHeadersTest, LastModifiedOnly) { // Add validation headers to the response response_headers_.setReferenceKey(Http::CustomHeaders::get().LastModified, - formatter_.now(time_source_)); + response_last_modified_); filter->encodeHeaders(response_headers_, true); } @@ -691,12 +962,13 @@ TEST_F(ValidationHeadersTest, LastModifiedOnly) { CacheFilterSharedPtr filter = makeFilter(simple_cache_); // Make sure the request requires validation - request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, "no-cache"); + request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, + Http::CustomHeaders::get().CacheControlValues.NoCache); testDecodeRequestMiss(filter); // Make sure validation conditional headers are added const Http::TestRequestHeaderMapImpl injected_headers = { - {"if-modified-since", formatter_.now(time_source_)}}; + {"if-modified-since", response_last_modified_}}; EXPECT_THAT(request_headers_, IsSupersetOfHeaders(injected_headers)); } } @@ -715,13 +987,13 @@ TEST_F(ValidationHeadersTest, NoEtagOrLastModified) { CacheFilterSharedPtr filter = makeFilter(simple_cache_); // Make sure the request requires validation - request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, "no-cache"); + request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, + Http::CustomHeaders::get().CacheControlValues.NoCache); testDecodeRequestMiss(filter); // Make sure validation conditional headers are added // If-Modified-Since falls back to date - const Http::TestRequestHeaderMapImpl injected_headers = { - {"if-modified-since", formatter_.now(time_source_)}}; + const Http::TestRequestHeaderMapImpl injected_headers = {{"if-modified-since", response_date_}}; EXPECT_THAT(request_headers_, IsSupersetOfHeaders(injected_headers)); } } @@ -743,14 +1015,108 @@ TEST_F(ValidationHeadersTest, InvalidLastModified) { CacheFilterSharedPtr filter = makeFilter(simple_cache_); // Make sure the request requires validation - request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, "no-cache"); + request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, + Http::CustomHeaders::get().CacheControlValues.NoCache); testDecodeRequestMiss(filter); // Make sure validation conditional headers are added // If-Modified-Since falls back to date + const Http::TestRequestHeaderMapImpl injected_headers = {{"if-modified-since", response_date_}}; + EXPECT_THAT(request_headers_, IsSupersetOfHeaders(injected_headers)); + } +} + +TEST_F(CacheChunkSizeTest, HandleDownstreamWatermarkCallbacks) { + request_headers_.setHost("DownstreamPressureHandling"); + const int chunks_count = 3; + const uint64_t body_size = getBufferLimit() * chunks_count; + const std::string body = std::string(body_size, 'a'); + { + CacheFilterSharedPtr filter = makeFilter(simple_cache_); + + testDecodeRequestMiss(filter); + + // Add Etag & Last-Modified headers to the response for validation. + response_headers_.setReferenceKey(Http::CustomHeaders::get().Etag, etag_); + response_headers_.setReferenceKey(Http::CustomHeaders::get().LastModified, + response_last_modified_); + + Buffer::OwnedImpl buffer(body); + response_headers_.setContentLength(body.size()); + EXPECT_EQ(filter->encodeHeaders(response_headers_, false), Http::FilterHeadersStatus::Continue); + EXPECT_EQ(filter->encodeData(buffer, true), Http::FilterDataStatus::Continue); + + filter->onDestroy(); + } + waitBeforeSecondRequest(); + { + CacheFilterSharedPtr filter = makeFilter(simple_cache_); + // Set require validation. + request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, + Http::CustomHeaders::get().CacheControlValues.NoCache); + + // Cached response requiring validation is treated as a cache miss. + testDecodeRequestMiss(filter); + + // Verify validation conditional headers are added. const Http::TestRequestHeaderMapImpl injected_headers = { - {"if-modified-since", formatter_.now(time_source_)}}; + {"if-none-match", etag_}, {"if-modified-since", response_last_modified_}}; EXPECT_THAT(request_headers_, IsSupersetOfHeaders(injected_headers)); + + // Advance time so that the cached date is updated. + time_source_.advanceTimeWait(std::chrono::seconds(10)); + const std::string not_modified_date = formatter_.now(time_source_); + Http::TestResponseHeaderMapImpl not_modified_response_headers = {{":status", "304"}, + {"date", not_modified_date}}; + + // The filter should continue headers encoding without ending the stream as data will be + // injected. + EXPECT_EQ(filter->encodeHeaders(not_modified_response_headers, true), + Http::FilterHeadersStatus::ContinueAndDontEndStream); + + // Verify the cached response headers with the updated date. + Http::TestResponseHeaderMapImpl updated_response_headers = response_headers_; + updated_response_headers.setDate(not_modified_date); + EXPECT_THAT(not_modified_response_headers, IsSupersetOfHeaders(updated_response_headers)); + + // Downstream backs up multiple times, increase watermarks. + filter->onAboveWriteBufferHighWatermark(); + filter->onAboveWriteBufferHighWatermark(); + + // The first cache lookup callback is already posted to the dispatcher before the watermark + // increases. Run the event loop to invoke the callback. No additional callbacks will be + // invoked due to watermark being greater than zero. + EXPECT_CALL(encoder_callbacks_, + injectEncodedDataToFilterChain( + testing::Property(&Buffer::Instance::toString, + testing::Eq(std::string(getBufferLimit(), 'a'))), + false)) + .Times(1); + dispatcher_->run(Event::Dispatcher::RunType::Block); + + // Lower the watermark, but still above 0. + filter->onBelowWriteBufferLowWatermark(); + EXPECT_CALL(encoder_callbacks_, + injectEncodedDataToFilterChain( + testing::Property(&Buffer::Instance::toString, + testing::Eq(std::string(getBufferLimit(), 'a'))), + _)) + .Times(0); + dispatcher_->run(Event::Dispatcher::RunType::Block); + + // Further lower the watermark, resume processing. + filter->onBelowWriteBufferLowWatermark(); + EXPECT_CALL(encoder_callbacks_, + injectEncodedDataToFilterChain( + testing::Property(&Buffer::Instance::toString, + testing::Eq(std::string(getBufferLimit(), 'a'))), + _)) + .Times(2); + dispatcher_->run(Event::Dispatcher::RunType::Block); + + ::testing::Mock::VerifyAndClearExpectations(&encoder_callbacks_); + + filter->onDestroy(); } } diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt index 5eaaceff978fc..d35991bd7e5e8 100644 --- a/tools/spelling/spelling_dictionary.txt +++ b/tools/spelling/spelling_dictionary.txt @@ -578,6 +578,7 @@ dgst dir dirname djb +dont downcalls downcasted downcased