Skip to content

Commit a6e13e8

Browse files
committed
http: check existence of codec when calling watermark callbacks
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
1 parent 07d91f9 commit a6e13e8

File tree

3 files changed

+23
-2
lines changed

3 files changed

+23
-2
lines changed

changelogs/current.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ minor_behavior_changes:
5858
5959
bug_fixes:
6060
# *Changes expected to improve the state of the world and are unlikely to have negative effects*
61+
- area: http
62+
change: |
63+
Fixed crash if a downstream watermark is hit by network filter writes before the HTTP codec is created.
6164
- area: tls
6265
change: |
6366
Fix on-demand TLS selector to enforce session resumption settings.

source/common/http/conn_manager_impl.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,10 +109,14 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
109109
void onEvent(Network::ConnectionEvent event) override;
110110
// Pass connection watermark events on to all the streams associated with that connection.
111111
void onAboveWriteBufferHighWatermark() override {
112-
codec_->onUnderlyingConnectionAboveWriteBufferHighWatermark();
112+
if (codec_) {
113+
codec_->onUnderlyingConnectionAboveWriteBufferHighWatermark();
114+
}
113115
}
114116
void onBelowWriteBufferLowWatermark() override {
115-
codec_->onUnderlyingConnectionBelowWriteBufferLowWatermark();
117+
if (codec_) {
118+
codec_->onUnderlyingConnectionBelowWriteBufferLowWatermark();
119+
}
116120
}
117121

118122
TimeSource& timeSource() { return time_source_; }

test/common/http/conn_manager_impl_test_2.cc

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1630,6 +1630,20 @@ TEST_F(HttpConnectionManagerImplTest, UnderlyingConnectionWatermarksUnwoundWithL
16301630
doRemoteClose();
16311631
}
16321632

1633+
// Can happen if a network filter writes large payloads to downstream before the HTTP
1634+
// filter gets data.
1635+
TEST_F(HttpConnectionManagerImplTest, UnderlyingConnectionWatermarkLimitsNoCodec) {
1636+
// Not used in the test.
1637+
delete codec_;
1638+
1639+
server_transformation_ = HttpConnectionManagerProto::PASS_THROUGH;
1640+
setup();
1641+
1642+
// No-ops since no codec setup yet. Verify no crash.
1643+
conn_manager_->onAboveWriteBufferHighWatermark();
1644+
conn_manager_->onBelowWriteBufferLowWatermark();
1645+
}
1646+
16331647
TEST_F(HttpConnectionManagerImplTest, AlterFilterWatermarkLimits) {
16341648
initial_buffer_limit_ = 100;
16351649
setup();

0 commit comments

Comments
 (0)