fix NPE In refreshCachedRoute#6359
Conversation
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
f5e2349 to
37adccd
Compare
| // encodeHeaders()/encodeData(). | ||
| EXPECT_CALL(*idle_timer, enableTimer(_)).Times(2); | ||
| EXPECT_CALL(*idle_timer, disableTimer()); | ||
| // Simulate and idle timeout so that the filter chain gets created |
There was a problem hiding this comment.
nit: end the comment with a period for consistency.
| Buffer::OwnedImpl fake_input; | ||
| conn_manager_->onData(fake_input, false); | ||
|
|
||
| // This should not be called as we don't have request headers |
There was a problem hiding this comment.
nit: end the comment with a period for consistency.
| EXPECT_CALL(*encoder_filters_[0], encodeComplete()); | ||
| EXPECT_CALL(*encoder_filters_[0], onDestroy()); | ||
|
|
||
| // 408 direct response after timeout. |
| EXPECT_CALL(*route_config_provider_.route_config_, route(_, _)).Times(0); | ||
|
|
||
| // Under heavy load it is possible that stream timeout will be reached before any headers were | ||
| // received. Envoy will creates a local reply that will go through the encoder filter chain. we |
| EXPECT_CALL(*route_config_provider_.route_config_, route(_, _)).Times(0); | ||
|
|
||
| // Under heavy load it is possible that stream timeout will be reached before any headers were | ||
| // received. Envoy will creates a local reply that will go through the encoder filter chain. we |
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
9958f26 to
704f43c
Compare
|
/retest |
|
🙀 failed invoking rebuild of |
mattklein123
left a comment
There was a problem hiding this comment.
Thank you so much for the quick fix. One small request to improve the test.
/wait
| // Under heavy load it is possible that stream timeout will be reached before any headers were | ||
| // received. Envoy will create a local reply that will go through the encoder filter chain. We | ||
| // want to make sure that encoder filters get a null route object. | ||
| auto route = encoder_filters_[0]->callbacks_->route(); |
There was a problem hiding this comment.
Instead of doing this here, can you potentially make this test a little more realistic by installing a mocker encoder filter and actually calling route() from within that filter's callbacks similar to what your filter does? You can see examples below in this file of how to setup a mock filter, it should be pretty simple.
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
|
/retest - compile_time_options stuck for 15 hours |
|
🐴 hold your horses - no failures detected, yet. |
|
thanks a lot guys |
Fix refreshCachedRoute not check if request headers are null
Risk Level: Low
Testing: Added unit test
Docs Changes: N/A
Release Notes: N/A
Fixes #6348