You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2020/04/06 18:31:02 UTC
[trafficserver] branch 8.1.x updated (822d92e -> 1a2924e)
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a change to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git.
from 822d92e Fixed the compatibility with previous cache verison
new 5dea427 Cleanup: do not re-define _proxy_ssn variable in local
new 492108f Fix crash in MIMEScanner::append with std::string. See #5587.
new 992914b Cleanup: VIO
new 23b97f0 Dechunk chunked contents on HttpTunnel if client protocol is HTTP/2 or HTTP/3
new 8af30a8 Log H2 priority information
new 8754223 Add 100-continue expectation support on H2 connection
new 830264c Address performance issue by removing use of read_avail
new 273aea8 Set END_STREAM flag when write_vio ntodo is 0
new 28bb79c Remove trailing white space from json formatter
new 2d9b2f1 Perf: Optimize sending HTTP/2 frame
new a65bc93 Add virtual destructors for Http2TxFrame, Http2FrequencyCounter
new 6c3362d Fix PUSH_PROMISE frame payload length
new b37bfde Fix heap-use-after-free on Http2Stream::destroy()
new 3c213fc Track scheduled events to (read|write)_vio.cont from Http2Stream
new b5ed51e Avoid unnecesarry copy on POST request over HTTP/2
new 327f95c Signal VC_EVENT_READ_COMPLETE when ATS received END_STREAM flag
new 71519ba Fix crash when H2 client does not set End-of-data bit
new 5234297 Update inactive_timeout_at in Http2Stream::signal_read_event()
new 15523bc Remove unnecesary HttpSM handler call with VC_EVENT_ERROR
new b537c7e Another option to fix potential HTTP/2 vio stall
new b647301 Adjust consume logic in data frame read
new 7b59b7d Remove update to unused variable
new e5ab03b Cleanup trailing whitespaces
new a3b6c77 Fix test_libhttp2 build issue of 8.1.x on Debian/Ubuntu
new 1a2924e Fix HTTP/2 AuTest for 8.1.x
The 25 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.gitignore | 1 +
doc/admin-guide/logging/formatting.en.rst | 6 +
iocore/eventsystem/I_IOBuffer.h | 4 +-
iocore/eventsystem/I_VIO.h | 26 +-
iocore/eventsystem/P_VIO.h | 34 +-
proxy/ProxyClientTransaction.cc | 12 +
proxy/ProxyClientTransaction.h | 2 +
proxy/hdrs/HTTP.h | 23 +-
proxy/http/HttpSM.cc | 13 +-
proxy/http/HttpSM.h | 13 +
proxy/http/HttpTransact.cc | 17 +-
proxy/http2/HTTP2.cc | 18 --
proxy/http2/HTTP2.h | 12 +-
proxy/http2/Http2ClientSession.cc | 27 +-
proxy/http2/Http2ClientSession.h | 91 +-----
proxy/http2/Http2ConnectionState.cc | 231 ++++++--------
proxy/http2/Http2Frame.cc | 253 +++++++++++++++
proxy/http2/Http2Frame.h | 256 +++++++++++++++
proxy/http2/Http2FrequencyCounter.h | 1 +
proxy/http2/Http2Stream.cc | 343 ++++++++++-----------
proxy/http2/Http2Stream.h | 44 +--
proxy/http2/Makefile.am | 27 +-
.../http2/unit_tests/main.cc | 59 ++--
proxy/http2/unit_tests/test_Http2Frame.cc | 64 ++++
proxy/logging/Log.cc | 10 +
proxy/logging/LogAccess.cc | 32 ++
proxy/logging/LogAccess.h | 20 +-
...{priority_0_stderr.gold => http2_9_stderr.gold} | 7 +-
.../sni-1.gold => h2/gold/http2_9_stdout.gold} | 0
tests/gold_tests/h2/gold/httpbin_0_stdout.gold | 2 +-
tests/gold_tests/h2/gold/httpbin_3_stderr.gold | 9 +
tests/gold_tests/h2/gold/httpbin_3_stdout.gold | 7 +
tests/gold_tests/h2/gold/httpbin_access.gold | 1 +
tests/gold_tests/h2/gold/nghttp_0_stdout.gold | 17 +
tests/gold_tests/h2/http2.test.py | 13 +
tests/gold_tests/h2/httpbin.test.py | 18 +-
tests/gold_tests/h2/nghttp.test.py | 86 ++++++
37 files changed, 1232 insertions(+), 567 deletions(-)
create mode 100644 proxy/http2/Http2Frame.cc
create mode 100644 proxy/http2/Http2Frame.h
copy src/tscore/unit_tests/test_scoped_resource.cc => proxy/http2/unit_tests/main.cc (53%)
create mode 100644 proxy/http2/unit_tests/test_Http2Frame.cc
copy tests/gold_tests/h2/gold/{priority_0_stderr.gold => http2_9_stderr.gold} (52%)
copy tests/gold_tests/{tls_hooks/gold/sni-1.gold => h2/gold/http2_9_stdout.gold} (100%)
create mode 100644 tests/gold_tests/h2/gold/httpbin_3_stderr.gold
create mode 100644 tests/gold_tests/h2/gold/httpbin_3_stdout.gold
create mode 100644 tests/gold_tests/h2/gold/nghttp_0_stdout.gold
create mode 100644 tests/gold_tests/h2/nghttp.test.py
[trafficserver] 25/25: Fix HTTP/2 AuTest for 8.1.x
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 1a2924e3d9640f29a331d459dd7f052257260deb
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Wed Apr 1 10:20:07 2020 +0900
Fix HTTP/2 AuTest for 8.1.x
---
tests/gold_tests/h2/nghttp.test.py | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tests/gold_tests/h2/nghttp.test.py b/tests/gold_tests/h2/nghttp.test.py
index 72cc51f..33b4c2a 100644
--- a/tests/gold_tests/h2/nghttp.test.py
+++ b/tests/gold_tests/h2/nghttp.test.py
@@ -46,7 +46,8 @@ microserver.addResponse("sessionlog.json",
# ----
# Setup ATS
# ----
-ts = Test.MakeATSProcess("ts", select_ports=True, enable_tls=True)
+ts = Test.MakeATSProcess("ts", select_ports=False)
+ts.Variables.ssl_port = 4443
# add ssl materials like key, certificates for the server
ts.addSSLfile("ssl/server.pem")
@@ -65,6 +66,7 @@ ts.Disk.records_config.update({
'proxy.config.diags.debug.tags': 'http',
'proxy.config.ssl.server.cert.path': '{0}'.format(ts.Variables.SSLDir),
'proxy.config.ssl.server.private_key.path': '{0}'.format(ts.Variables.SSLDir),
+ 'proxy.config.http.server_ports': '{0} {1}:ssl'.format(ts.Variables.port, ts.Variables.ssl_port),
'proxy.config.http.cache.http': 0,
'proxy.config.http2.active_timeout_in': 3,
})
[trafficserver] 07/25: Address performance issue by removing use of
read_avail
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 830264cf46d822920b5192295111a13fada54eec
Author: Susan Hinrichs <sh...@oath.com>
AuthorDate: Fri Feb 1 23:05:47 2019 +0000
Address performance issue by removing use of read_avail
(cherry picked from commit fc4975d97bfe652442b62cb3e1a614abd92952b5)
---
proxy/http2/Http2ConnectionState.cc | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc
index 869e0ee..7588b75 100644
--- a/proxy/http2/Http2ConnectionState.cc
+++ b/proxy/http2/Http2ConnectionState.cc
@@ -1446,7 +1446,6 @@ Http2ConnectionState::send_a_data_frame(Http2Stream *stream, size_t &payload_len
const ssize_t window_size = std::min(this->client_rwnd(), stream->client_rwnd());
const size_t buf_len = BUFFER_SIZE_FOR_INDEX(buffer_size_index[HTTP2_FRAME_TYPE_DATA]);
const size_t write_available_size = std::min(buf_len, static_cast<size_t>(window_size));
- size_t read_available_size = 0;
payload_length = 0;
uint8_t flags = 0x00;
@@ -1455,23 +1454,21 @@ Http2ConnectionState::send_a_data_frame(Http2Stream *stream, size_t &payload_len
SCOPED_MUTEX_LOCK(stream_lock, stream->mutex, this_ethread());
- if (current_reader) {
- read_available_size = static_cast<size_t>(current_reader->read_avail());
- } else {
+ if (!current_reader) {
Http2StreamDebug(this->ua_session, stream->get_id(), "couldn't get data reader");
return Http2SendDataFrameResult::ERROR;
}
// Select appropriate payload length
- if (read_available_size > 0) {
+ if (current_reader->is_read_avail_more_than(0)) {
// We only need to check for window size when there is a payload
if (window_size <= 0) {
Http2StreamDebug(this->ua_session, stream->get_id(), "No window");
return Http2SendDataFrameResult::NO_WINDOW;
}
// Copy into the payload buffer. Seems like we should be able to skip this copy step
- payload_length = std::min(read_available_size, write_available_size);
- current_reader->memcpy(payload_buffer, static_cast<int64_t>(payload_length));
+ payload_length = write_available_size;
+ payload_length = current_reader->read(payload_buffer, static_cast<int64_t>(write_available_size));
} else {
payload_length = 0;
}
@@ -1484,7 +1481,7 @@ Http2ConnectionState::send_a_data_frame(Http2Stream *stream, size_t &payload_len
return Http2SendDataFrameResult::NO_PAYLOAD;
}
- if (stream->is_body_done() && read_available_size <= write_available_size) {
+ if (stream->is_body_done() && !current_reader->is_read_avail_more_than(0)) {
flags |= HTTP2_FLAGS_DATA_END_STREAM;
}
@@ -1502,7 +1499,6 @@ Http2ConnectionState::send_a_data_frame(Http2Stream *stream, size_t &payload_len
data.finalize(payload_length);
stream->update_sent_count(payload_length);
- current_reader->consume(payload_length);
// xmit event
SCOPED_MUTEX_LOCK(lock, this->ua_session->mutex, this_ethread());
[trafficserver] 19/25: Remove unnecesary HttpSM handler call with
VC_EVENT_ERROR
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 15523bcdd6d68f63dffc0dc7df6b27e7fa4c1833
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Fri Jan 31 16:25:39 2020 +0900
Remove unnecesary HttpSM handler call with VC_EVENT_ERROR
(cherry picked from commit 240bb69f83d0111d96352af3104a4bdff88fd2e8)
Conflicts:
proxy/http2/Http2Stream.cc
---
proxy/http2/Http2Stream.cc | 4 ----
1 file changed, 4 deletions(-)
diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc
index ed160c9..e3fa693 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -463,7 +463,6 @@ Http2Stream::initiating_close()
// This should result in do_io_close or release being called. That will schedule the final
// kill yourself signal
- // Send the SM the EOS signal if there are no active VIO's to signal
// We are sending signals rather than calling the handlers directly to avoid the case where
// the HttpTunnel handler causes the HttpSM to be deleted on the stack.
bool sent_write_complete = false;
@@ -490,9 +489,6 @@ Http2Stream::initiating_close()
Http2StreamDebug("send EOS to read cont");
read_event = send_tracked_event(read_event, VC_EVENT_EOS, &read_vio);
}
- } else if (current_reader) {
- SCOPED_MUTEX_LOCK(lock, current_reader->mutex, this_ethread());
- current_reader->handleEvent(VC_EVENT_ERROR);
} else if (!sent_write_complete) {
// Transaction is already gone or not started. Kill yourself
do_io_close();
[trafficserver] 12/25: Fix PUSH_PROMISE frame payload length
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 6c3362d5c106e29ab31b2643ecc4511b4f5f3d46
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Thu Mar 26 15:09:55 2020 +0900
Fix PUSH_PROMISE frame payload length
(cherry picked from commit 582df40711b07cf01b25c65c7fddf27288e0a8bd)
Conflicts:
.gitignore
proxy/http2/HTTP2.h
---
.gitignore | 1 +
proxy/http2/HTTP2.h | 5 +--
proxy/http2/Http2Frame.h | 5 ++-
proxy/http2/Makefile.am | 24 ++++++++++--
proxy/http2/unit_tests/main.cc | 55 ++++++++++++++++++++++++++
proxy/http2/unit_tests/test_Http2Frame.cc | 64 +++++++++++++++++++++++++++++++
6 files changed, 146 insertions(+), 8 deletions(-)
diff --git a/.gitignore b/.gitignore
index 767cb1e..1e8da8b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -100,6 +100,7 @@ proxy/hdrs/test_mime
proxy/hdrs/test_Huffmancode
proxy/hdrs/test_XPACK
proxy/http/test_ForwardedConfig
+proxy/http2/test_libhttp2
proxy/http2/test_Http2DependencyTree
proxy/http2/test_Http2FrequencyCounter
proxy/http2/test_HPACK
diff --git a/proxy/http2/HTTP2.h b/proxy/http2/HTTP2.h
index 02b78b5..6b193e7 100644
--- a/proxy/http2/HTTP2.h
+++ b/proxy/http2/HTTP2.h
@@ -305,9 +305,8 @@ struct Http2RstStream {
// [RFC 7540] 6.6 PUSH_PROMISE Format
struct Http2PushPromise {
- Http2PushPromise() : pad_length(0), promised_streamid(0) {}
- uint8_t pad_length;
- Http2StreamId promised_streamid;
+ uint8_t pad_length = 0;
+ Http2StreamId promised_streamid = 0;
};
static inline bool
diff --git a/proxy/http2/Http2Frame.h b/proxy/http2/Http2Frame.h
index dd68b8c..b0bcd35 100644
--- a/proxy/http2/Http2Frame.h
+++ b/proxy/http2/Http2Frame.h
@@ -169,7 +169,10 @@ class Http2PushPromiseFrame : public Http2TxFrame
{
public:
Http2PushPromiseFrame(Http2StreamId stream_id, uint8_t flags, Http2PushPromise p, uint8_t *h, uint32_t l)
- : Http2TxFrame({l, HTTP2_FRAME_TYPE_PUSH_PROMISE, flags, stream_id}), _params(p), _hdr_block(h), _hdr_block_len(l)
+ : Http2TxFrame({l + static_cast<uint32_t>(sizeof(Http2StreamId)), HTTP2_FRAME_TYPE_PUSH_PROMISE, flags, stream_id}),
+ _params(p),
+ _hdr_block(h),
+ _hdr_block_len(l)
{
}
diff --git a/proxy/http2/Makefile.am b/proxy/http2/Makefile.am
index c6b494b..86cd765 100644
--- a/proxy/http2/Makefile.am
+++ b/proxy/http2/Makefile.am
@@ -60,14 +60,30 @@ libhttp2_a_SOURCES += \
endif
check_PROGRAMS = \
+ test_libhttp2 \
test_Http2DependencyTree \
test_Http2FrequencyCounter \
test_HPACK
-TESTS = \
- test_Http2DependencyTree \
- test_Http2FrequencyCounter \
- test_HPACK
+TESTS = $(check_PROGRAMS)
+
+test_libhttp2_LDADD = \
+ libhttp2.a \
+ $(top_builddir)/proxy/hdrs/libhdrs.a \
+ $(top_builddir)/src/tscore/libtscore.la \
+ $(top_builddir)/src/tscpp/util/libtscpputil.la \
+ $(top_builddir)/iocore/eventsystem/libinkevent.a \
+ $(top_builddir)/lib/records/librecords_p.a \
+ $(top_builddir)/mgmt/libmgmt_p.la \
+ $(top_builddir)/proxy/shared/libUglyLogStubs.a \
+ @HWLOC_LIBS@
+
+test_libhttp2_CPPFLAGS = $(AM_CPPFLAGS)\
+ -I$(abs_top_srcdir)/tests/include
+
+test_libhttp2_SOURCES = \
+ unit_tests/test_Http2Frame.cc \
+ unit_tests/main.cc
test_Http2DependencyTree_LDADD = \
$(top_builddir)/src/tscore/libtscore.la \
diff --git a/proxy/http2/unit_tests/main.cc b/proxy/http2/unit_tests/main.cc
new file mode 100644
index 0000000..4df7036
--- /dev/null
+++ b/proxy/http2/unit_tests/main.cc
@@ -0,0 +1,55 @@
+/** @file
+
+ The main file for test_libhttp2
+
+ @section license License
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+#define CATCH_CONFIG_MAIN
+#include "catch.hpp"
+
+#include "tscore/I_Layout.h"
+
+#include "I_EventSystem.h"
+#include "RecordsConfig.h"
+
+#include "diags.i"
+
+#define TEST_THREADS 1
+
+struct EventProcessorListener : Catch::TestEventListenerBase {
+ using TestEventListenerBase::TestEventListenerBase;
+
+ void
+ testRunStarting(Catch::TestRunInfo const &testRunInfo) override
+ {
+ Layout::create();
+ init_diags("", nullptr);
+ RecProcessInit(RECM_STAND_ALONE);
+ LibRecordsConfigInit();
+
+ ink_event_system_init(EVENT_SYSTEM_MODULE_VERSION);
+ eventProcessor.start(TEST_THREADS);
+
+ EThread *main_thread = new EThread;
+ main_thread->set_specific();
+ }
+};
+
+CATCH_REGISTER_LISTENER(EventProcessorListener);
diff --git a/proxy/http2/unit_tests/test_Http2Frame.cc b/proxy/http2/unit_tests/test_Http2Frame.cc
new file mode 100644
index 0000000..d1e695e
--- /dev/null
+++ b/proxy/http2/unit_tests/test_Http2Frame.cc
@@ -0,0 +1,64 @@
+/** @file
+
+ Unit tests for Http2Frame
+
+ @section license License
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+#include "catch.hpp"
+
+#include "Http2Frame.h"
+
+TEST_CASE("Http2Frame", "[http2][Http2Frame]")
+{
+ MIOBuffer *miob = new_MIOBuffer();
+ IOBufferReader *miob_r = miob->alloc_reader();
+
+ SECTION("PUSH_PROMISE")
+ {
+ Http2StreamId id = 1;
+ uint8_t flags = HTTP2_FLAGS_PUSH_PROMISE_END_HEADERS;
+ Http2PushPromise pp{0, 2};
+ uint8_t hdr_block[] = {0xbe, 0xef, 0xbe, 0xef, 0xbe, 0xef, 0xbe, 0xef, 0xbe, 0xef};
+ uint8_t hdr_block_len = sizeof(hdr_block);
+
+ Http2PushPromiseFrame frame(id, flags, pp, hdr_block, hdr_block_len);
+ uint64_t written = frame.write_to(miob);
+
+ CHECK(written == HTTP2_FRAME_HEADER_LEN + sizeof(Http2StreamId) + hdr_block_len);
+ CHECK(written == miob_r->read_avail());
+
+ uint8_t buf[32] = {0};
+ uint64_t read = miob_r->read(buf, written);
+ CHECK(read == written);
+
+ uint8_t expected[] = {
+ 0x00, 0x00, 0x0e, ///< Length
+ 0x05, ///< Type
+ 0x04, ///< Flags
+ 0x00, 0x00, 0x00, 0x01, ///< Stream Identifier (31)
+ 0x00, 0x00, 0x00, 0x02, ///< Promised Stream ID
+ 0xbe, 0xef, 0xbe, 0xef, 0xbe, 0xef, 0xbe, 0xef, 0xbe, 0xef ///< Header Block Fragment
+ };
+
+ CHECK(memcmp(buf, expected, written) == 0);
+ }
+
+ free_MIOBuffer(miob);
+}
[trafficserver] 10/25: Perf: Optimize sending HTTP/2 frame
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 2d9b2f1efc44bcbc343c778852742e7e64bda1f9
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Mon Jan 20 13:57:53 2020 +0900
Perf: Optimize sending HTTP/2 frame
Prior to this change, HTTP/2 was almost 30% slower than HTTP/1.1 (over TLS) on downloading a huge file (over 1GB).
Improvements:
- Avoid unnecessary IOBufferBlock allocation for all type of frame
- Avoid unnecessary copy on sending DATA frame
- Adjust IOBufferBlock size of Http2ClientSession::write_buffer
Cleanups:
- Decouple receiving & sending HTTP/2 Frame
- Remove unnecessary SCOPED_MUTEX_LOCK
(cherry picked from commit 48bcbe692ae7172479e8d71a91d1c779577186c9)
Conflicts:
proxy/http2/Http2ClientSession.cc
proxy/http2/Http2ClientSession.h
proxy/http2/Http2ConnectionState.cc
proxy/http2/Http2Stream.cc
---
proxy/http2/HTTP2.cc | 18 ---
proxy/http2/HTTP2.h | 7 +-
proxy/http2/Http2ClientSession.cc | 27 ++--
proxy/http2/Http2ClientSession.h | 91 +------------
proxy/http2/Http2ConnectionState.cc | 175 ++++++++-----------------
proxy/http2/Http2Frame.cc | 253 ++++++++++++++++++++++++++++++++++++
proxy/http2/Http2Frame.h | 252 +++++++++++++++++++++++++++++++++++
proxy/http2/Http2Stream.cc | 4 +-
proxy/http2/Makefile.am | 2 +
9 files changed, 585 insertions(+), 244 deletions(-)
diff --git a/proxy/http2/HTTP2.cc b/proxy/http2/HTTP2.cc
index 5be5108..7b40df7 100644
--- a/proxy/http2/HTTP2.cc
+++ b/proxy/http2/HTTP2.cc
@@ -242,24 +242,6 @@ http2_write_frame_header(const Http2FrameHeader &hdr, IOVec iov)
}
bool
-http2_write_data(const uint8_t *src, size_t length, const IOVec &iov)
-{
- byte_pointer ptr(iov.iov_base);
- write_and_advance(ptr, src, length);
-
- return true;
-}
-
-bool
-http2_write_headers(const uint8_t *src, size_t length, const IOVec &iov)
-{
- byte_pointer ptr(iov.iov_base);
- write_and_advance(ptr, src, length);
-
- return true;
-}
-
-bool
http2_write_rst_stream(uint32_t error_code, IOVec iov)
{
byte_pointer ptr(iov.iov_base);
diff --git a/proxy/http2/HTTP2.h b/proxy/http2/HTTP2.h
index 660ecbb..02b78b5 100644
--- a/proxy/http2/HTTP2.h
+++ b/proxy/http2/HTTP2.h
@@ -33,6 +33,9 @@ class HTTPHdr;
typedef unsigned Http2StreamId;
+constexpr Http2StreamId HTTP2_CONNECTION_CONTROL_STRTEAM = 0;
+constexpr uint8_t HTTP2_FRAME_NO_FLAG = 0;
+
// [RFC 7540] 6.9.2. Initial Flow Control Window Size
// the flow control window can be come negative so we need to track it with a signed type.
typedef int32_t Http2WindowSize;
@@ -323,10 +326,6 @@ bool http2_parse_frame_header(IOVec, Http2FrameHeader &);
bool http2_write_frame_header(const Http2FrameHeader &, IOVec);
-bool http2_write_data(const uint8_t *, size_t, const IOVec &);
-
-bool http2_write_headers(const uint8_t *, size_t, const IOVec &);
-
bool http2_write_rst_stream(uint32_t, IOVec);
bool http2_write_settings(const Http2SettingsParameter &, const IOVec &);
diff --git a/proxy/http2/Http2ClientSession.cc b/proxy/http2/Http2ClientSession.cc
index 422be4f..b014e0e 100644
--- a/proxy/http2/Http2ClientSession.cc
+++ b/proxy/http2/Http2ClientSession.cc
@@ -216,7 +216,8 @@ Http2ClientSession::new_connection(NetVConnection *new_vc, MIOBuffer *iobuf, IOB
this->h2_pushed_urls = ink_hash_table_create(InkHashTableKeyType_String);
this->h2_pushed_urls_size = 0;
- this->write_buffer = new_MIOBuffer(HTTP2_HEADER_BUFFER_SIZE_INDEX);
+ // Set write buffer size to max size of TLS record (16KB)
+ this->write_buffer = new_MIOBuffer(BUFFER_SIZE_INDEX_16K);
this->sm_writer = this->write_buffer->alloc_reader();
do_api_callout(TS_HTTP_SSN_START_HOOK);
@@ -325,6 +326,19 @@ Http2ClientSession::set_half_close_local_flag(bool flag)
half_close_local = flag;
}
+int64_t
+Http2ClientSession::xmit(const Http2TxFrame &frame)
+{
+ int64_t len = frame.write_to(this->write_buffer);
+
+ if (len > 0) {
+ total_write_len += len;
+ write_reenable();
+ }
+
+ return len;
+}
+
int
Http2ClientSession::main_event_handler(int event, void *edata)
{
@@ -349,16 +363,6 @@ Http2ClientSession::main_event_handler(int event, void *edata)
break;
}
- case HTTP2_SESSION_EVENT_XMIT: {
- Http2Frame *frame = (Http2Frame *)edata;
- total_write_len += frame->size();
- write_vio->nbytes = total_write_len;
- frame->xmit(this->write_buffer);
- write_reenable();
- retval = 0;
- break;
- }
-
case HTTP2_SESSION_EVENT_REENABLE:
// VIO will be reenableed in this handler
retval = (this->*session_handler)(VC_EVENT_READ_READY, static_cast<VIO *>(e->cookie));
@@ -384,6 +388,7 @@ Http2ClientSession::main_event_handler(int event, void *edata)
retval = 0;
break;
+ case HTTP2_SESSION_EVENT_XMIT:
default:
Http2SsnDebug("unexpected event=%d edata=%p", event, edata);
ink_release_assert(0);
diff --git a/proxy/http2/Http2ClientSession.h b/proxy/http2/Http2ClientSession.h
index d2b44cc..c1e3bb3 100644
--- a/proxy/http2/Http2ClientSession.h
+++ b/proxy/http2/Http2ClientSession.h
@@ -27,6 +27,7 @@
#include "Plugin.h"
#include "ProxyClientSession.h"
#include "Http2ConnectionState.h"
+#include "Http2Frame.h"
#include <string_view>
#include "tscore/ink_inet.h"
#include "tscore/History.h"
@@ -77,95 +78,6 @@ struct Http2UpgradeContext {
Http2ConnectionSettings client_settings;
};
-class Http2Frame
-{
-public:
- Http2Frame(const Http2FrameHeader &h, IOBufferReader *r)
- {
- this->hdr = h;
- this->ioreader = r;
- }
-
- Http2Frame(Http2FrameType type, Http2StreamId streamid, uint8_t flags)
- {
- this->hdr = {0, (uint8_t)type, flags, streamid};
- this->ioreader = nullptr;
- }
-
- IOBufferReader *
- reader() const
- {
- return ioreader;
- }
-
- const Http2FrameHeader &
- header() const
- {
- return this->hdr;
- }
-
- // Allocate an IOBufferBlock for payload of this frame.
- void
- alloc(int index)
- {
- this->ioblock = new_IOBufferBlock();
- this->ioblock->alloc(index);
- }
-
- // Return the writeable buffer space for frame payload
- IOVec
- write()
- {
- return make_iovec(this->ioblock->end(), this->ioblock->write_avail());
- }
-
- // Once the frame has been serialized, update the payload length of frame header.
- void
- finalize(size_t nbytes)
- {
- if (this->ioblock) {
- ink_assert((int64_t)nbytes <= this->ioblock->write_avail());
- this->ioblock->fill(nbytes);
-
- this->hdr.length = this->ioblock->size();
- }
- }
-
- void
- xmit(MIOBuffer *iobuffer)
- {
- // Write frame header
- uint8_t buf[HTTP2_FRAME_HEADER_LEN];
- http2_write_frame_header(hdr, make_iovec(buf));
- iobuffer->write(buf, sizeof(buf));
-
- // Write frame payload
- // It could be empty (e.g. SETTINGS frame with ACK flag)
- if (ioblock && ioblock->read_avail() > 0) {
- iobuffer->append_block(this->ioblock.get());
- }
- }
-
- int64_t
- size()
- {
- if (ioblock) {
- return HTTP2_FRAME_HEADER_LEN + ioblock->size();
- } else {
- return HTTP2_FRAME_HEADER_LEN;
- }
- }
-
- // noncopyable
- Http2Frame(Http2Frame &) = delete;
- Http2Frame &operator=(const Http2Frame &) = delete;
-
-private:
- Http2FrameHeader hdr; // frame header
- Ptr<IOBufferBlock> ioblock; // frame payload
- IOBufferReader *ioreader;
-};
-
class Http2ClientSession : public ProxyClientSession
{
public:
@@ -194,6 +106,7 @@ public:
// more methods
void write_reenable();
+ int64_t xmit(const Http2TxFrame &frame);
////////////////////
// Accessors
diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc
index 3e4056b..1eb4fb1 100644
--- a/proxy/http2/Http2ConnectionState.cc
+++ b/proxy/http2/Http2ConnectionState.cc
@@ -25,6 +25,7 @@
#include "Http2ConnectionState.h"
#include "Http2ClientSession.h"
#include "Http2Stream.h"
+#include "Http2Frame.h"
#include "Http2DebugNames.h"
#include "HttpDebugNames.h"
#include <sstream>
@@ -49,12 +50,12 @@ static const int buffer_size_index[HTTP2_FRAME_TYPE_MAX] = {
BUFFER_SIZE_INDEX_16K, // HTTP2_FRAME_TYPE_DATA
BUFFER_SIZE_INDEX_16K, // HTTP2_FRAME_TYPE_HEADERS
-1, // HTTP2_FRAME_TYPE_PRIORITY
- BUFFER_SIZE_INDEX_128, // HTTP2_FRAME_TYPE_RST_STREAM
- BUFFER_SIZE_INDEX_128, // HTTP2_FRAME_TYPE_SETTINGS
+ -1, // HTTP2_FRAME_TYPE_RST_STREAM
+ -1, // HTTP2_FRAME_TYPE_SETTINGS
BUFFER_SIZE_INDEX_16K, // HTTP2_FRAME_TYPE_PUSH_PROMISE
- BUFFER_SIZE_INDEX_128, // HTTP2_FRAME_TYPE_PING
- BUFFER_SIZE_INDEX_128, // HTTP2_FRAME_TYPE_GOAWAY
- BUFFER_SIZE_INDEX_128, // HTTP2_FRAME_TYPE_WINDOW_UPDATE
+ -1, // HTTP2_FRAME_TYPE_PING
+ -1, // HTTP2_FRAME_TYPE_GOAWAY
+ -1, // HTTP2_FRAME_TYPE_WINDOW_UPDATE
BUFFER_SIZE_INDEX_16K, // HTTP2_FRAME_TYPE_CONTINUATION
};
@@ -628,8 +629,8 @@ rcv_settings_frame(Http2ConnectionState &cstate, const Http2Frame &frame)
// [RFC 7540] 6.5. Once all values have been applied, the recipient MUST
// immediately emit a SETTINGS frame with the ACK flag set.
- Http2Frame ackFrame(HTTP2_FRAME_TYPE_SETTINGS, 0, HTTP2_FLAGS_SETTINGS_ACK);
- cstate.ua_session->handleEvent(HTTP2_SESSION_EVENT_XMIT, &ackFrame);
+ Http2SettingsFrame ack_frame(0, HTTP2_FLAGS_SETTINGS_ACK);
+ cstate.ua_session->xmit(ack_frame);
return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_NONE);
}
@@ -1448,27 +1449,29 @@ Http2ConnectionState::send_a_data_frame(Http2Stream *stream, size_t &payload_len
const size_t write_available_size = std::min(buf_len, static_cast<size_t>(window_size));
payload_length = 0;
- uint8_t flags = 0x00;
- uint8_t payload_buffer[buf_len];
- IOBufferReader *current_reader = stream->response_get_data_reader();
+ uint8_t flags = 0x00;
+ IOBufferReader *resp_reader = stream->response_get_data_reader();
SCOPED_MUTEX_LOCK(stream_lock, stream->mutex, this_ethread());
- if (!current_reader) {
+ if (!resp_reader) {
Http2StreamDebug(this->ua_session, stream->get_id(), "couldn't get data reader");
return Http2SendDataFrameResult::ERROR;
}
// Select appropriate payload length
- if (current_reader->is_read_avail_more_than(0)) {
+ if (resp_reader->is_read_avail_more_than(0)) {
// We only need to check for window size when there is a payload
if (window_size <= 0) {
Http2StreamDebug(this->ua_session, stream->get_id(), "No window");
return Http2SendDataFrameResult::NO_WINDOW;
}
- // Copy into the payload buffer. Seems like we should be able to skip this copy step
- payload_length = write_available_size;
- payload_length = current_reader->read(payload_buffer, static_cast<int64_t>(write_available_size));
+
+ if (resp_reader->is_read_avail_more_than(write_available_size)) {
+ payload_length = write_available_size;
+ } else {
+ payload_length = resp_reader->read_avail();
+ }
} else {
payload_length = 0;
}
@@ -1481,7 +1484,7 @@ Http2ConnectionState::send_a_data_frame(Http2Stream *stream, size_t &payload_len
return Http2SendDataFrameResult::NO_PAYLOAD;
}
- if (stream->is_write_vio_done() && !current_reader->is_read_avail_more_than(0)) {
+ if (stream->is_write_vio_done() && !resp_reader->is_read_avail_more_than(0)) {
flags |= HTTP2_FLAGS_DATA_END_STREAM;
}
@@ -1493,22 +1496,16 @@ Http2ConnectionState::send_a_data_frame(Http2Stream *stream, size_t &payload_len
Http2StreamDebug(ua_session, stream->get_id(), "Send a DATA frame - client window con: %5zd stream: %5zd payload: %5zd",
_client_rwnd, stream->client_rwnd(), payload_length);
- Http2Frame data(HTTP2_FRAME_TYPE_DATA, stream->get_id(), flags);
- data.alloc(buffer_size_index[HTTP2_FRAME_TYPE_DATA]);
- http2_write_data(payload_buffer, payload_length, data.write());
- data.finalize(payload_length);
+ Http2DataFrame data(stream->get_id(), flags, resp_reader, payload_length);
+ this->ua_session->xmit(data);
stream->update_sent_count(payload_length);
- // xmit event
- SCOPED_MUTEX_LOCK(lock, this->ua_session->mutex, this_ethread());
- this->ua_session->handleEvent(HTTP2_SESSION_EVENT_XMIT, &data);
-
if (flags & HTTP2_FLAGS_DATA_END_STREAM) {
Http2StreamDebug(ua_session, stream->get_id(), "END_STREAM");
stream->send_end_stream = true;
// Setting to the same state shouldn't be erroneous
- stream->change_state(data.header().type, data.header().flags);
+ stream->change_state(HTTP2_FRAME_TYPE_DATA, flags);
return Http2SendDataFrameResult::DONE;
}
@@ -1553,10 +1550,8 @@ Http2ConnectionState::send_headers_frame(Http2Stream *stream)
uint32_t buf_len = 0;
uint32_t header_blocks_size = 0;
int payload_length = 0;
- uint64_t sent = 0;
uint8_t flags = 0x00;
-
- HTTPHdr *resp_header = &stream->response_header;
+ HTTPHdr *resp_header = &stream->response_header;
Http2StreamDebug(ua_session, stream->get_id(), "Send HEADERS frame");
@@ -1593,10 +1588,6 @@ Http2ConnectionState::send_headers_frame(Http2Stream *stream)
} else {
payload_length = BUFFER_SIZE_FOR_INDEX(buffer_size_index[HTTP2_FRAME_TYPE_HEADERS]);
}
- Http2Frame headers(HTTP2_FRAME_TYPE_HEADERS, stream->get_id(), flags);
- headers.alloc(buffer_size_index[HTTP2_FRAME_TYPE_HEADERS]);
- http2_write_headers(buf, payload_length, headers.write());
- headers.finalize(payload_length);
// Change stream state
if (!stream->change_state(HTTP2_FRAME_TYPE_HEADERS, flags)) {
@@ -1611,10 +1602,9 @@ Http2ConnectionState::send_headers_frame(Http2Stream *stream)
return;
}
- // xmit event
- SCOPED_MUTEX_LOCK(lock, this->ua_session->mutex, this_ethread());
- this->ua_session->handleEvent(HTTP2_SESSION_EVENT_XMIT, &headers);
- sent += payload_length;
+ Http2HeadersFrame headers(stream->get_id(), flags, buf, payload_length);
+ this->ua_session->xmit(headers);
+ uint64_t sent = payload_length;
// Send CONTINUATION frames
flags = 0;
@@ -1625,14 +1615,10 @@ Http2ConnectionState::send_headers_frame(Http2Stream *stream)
if (sent + payload_length == header_blocks_size) {
flags |= HTTP2_FLAGS_CONTINUATION_END_HEADERS;
}
- Http2Frame headers(HTTP2_FRAME_TYPE_CONTINUATION, stream->get_id(), flags);
- headers.alloc(buffer_size_index[HTTP2_FRAME_TYPE_CONTINUATION]);
- http2_write_headers(buf + sent, payload_length, headers.write());
- headers.finalize(payload_length);
- stream->change_state(headers.header().type, headers.header().flags);
- // xmit event
- SCOPED_MUTEX_LOCK(lock, this->ua_session->mutex, this_ethread());
- this->ua_session->handleEvent(HTTP2_SESSION_EVENT_XMIT, &headers);
+ stream->change_state(HTTP2_FRAME_TYPE_CONTINUATION, flags);
+
+ Http2ContinuationFrame continuation_frame(stream->get_id(), flags, buf + sent, payload_length);
+ this->ua_session->xmit(continuation_frame);
sent += payload_length;
}
@@ -1648,7 +1634,6 @@ Http2ConnectionState::send_push_promise_frame(Http2Stream *stream, URL &url, con
uint32_t buf_len = 0;
uint32_t header_blocks_size = 0;
int payload_length = 0;
- uint64_t sent = 0;
uint8_t flags = 0x00;
if (client_settings.get(HTTP2_SETTINGS_ENABLE_PUSH) == 0) {
@@ -1702,16 +1687,13 @@ Http2ConnectionState::send_push_promise_frame(Http2Stream *stream, URL &url, con
payload_length =
BUFFER_SIZE_FOR_INDEX(buffer_size_index[HTTP2_FRAME_TYPE_PUSH_PROMISE]) - sizeof(push_promise.promised_streamid);
}
- Http2Frame headers(HTTP2_FRAME_TYPE_PUSH_PROMISE, stream->get_id(), flags);
- headers.alloc(buffer_size_index[HTTP2_FRAME_TYPE_PUSH_PROMISE]);
+
Http2StreamId id = this->get_latest_stream_id_out() + 2;
push_promise.promised_streamid = id;
- http2_write_push_promise(push_promise, buf, payload_length, headers.write());
- headers.finalize(sizeof(push_promise.promised_streamid) + payload_length);
- // xmit event
- SCOPED_MUTEX_LOCK(lock, this->ua_session->mutex, this_ethread());
- this->ua_session->handleEvent(HTTP2_SESSION_EVENT_XMIT, &headers);
- sent += payload_length;
+
+ Http2PushPromiseFrame push_promise_frame(stream->get_id(), flags, push_promise, buf, payload_length);
+ this->ua_session->xmit(push_promise_frame);
+ uint64_t sent = payload_length;
// Send CONTINUATION frames
flags = 0;
@@ -1722,13 +1704,9 @@ Http2ConnectionState::send_push_promise_frame(Http2Stream *stream, URL &url, con
if (sent + payload_length == header_blocks_size) {
flags |= HTTP2_FLAGS_CONTINUATION_END_HEADERS;
}
- Http2Frame headers(HTTP2_FRAME_TYPE_CONTINUATION, stream->get_id(), flags);
- headers.alloc(buffer_size_index[HTTP2_FRAME_TYPE_CONTINUATION]);
- http2_write_headers(buf + sent, payload_length, headers.write());
- headers.finalize(payload_length);
- // xmit event
- SCOPED_MUTEX_LOCK(lock, this->ua_session->mutex, this_ethread());
- this->ua_session->handleEvent(HTTP2_SESSION_EVENT_XMIT, &headers);
+
+ Http2ContinuationFrame continuation(stream->get_id(), flags, buf + sent, payload_length);
+ this->ua_session->xmit(continuation);
sent += payload_length;
}
ats_free(buf);
@@ -1772,12 +1750,6 @@ Http2ConnectionState::send_rst_stream_frame(Http2StreamId id, Http2ErrorCode ec)
++stream_error_count;
}
- Http2Frame rst_stream(HTTP2_FRAME_TYPE_RST_STREAM, id, 0);
-
- rst_stream.alloc(buffer_size_index[HTTP2_FRAME_TYPE_RST_STREAM]);
- http2_write_rst_stream(static_cast<uint32_t>(ec), rst_stream.write());
- rst_stream.finalize(HTTP2_RST_STREAM_LEN);
-
// change state to closed
Http2Stream *stream = find_stream(id);
if (stream != nullptr) {
@@ -1793,9 +1765,8 @@ Http2ConnectionState::send_rst_stream_frame(Http2StreamId id, Http2ErrorCode ec)
}
}
- // xmit event
- SCOPED_MUTEX_LOCK(lock, this->ua_session->mutex, this_ethread());
- this->ua_session->handleEvent(HTTP2_SESSION_EVENT_XMIT, &rst_stream);
+ Http2RstStreamFrame rst_stream(id, static_cast<uint32_t>(ec));
+ this->ua_session->xmit(rst_stream);
}
void
@@ -1805,11 +1776,8 @@ Http2ConnectionState::send_settings_frame(const Http2ConnectionSettings &new_set
Http2StreamDebug(ua_session, stream_id, "Send SETTINGS frame");
- Http2Frame settings(HTTP2_FRAME_TYPE_SETTINGS, stream_id, 0);
- settings.alloc(buffer_size_index[HTTP2_FRAME_TYPE_SETTINGS]);
-
- IOVec iov = settings.write();
- uint32_t settings_length = 0;
+ Http2SettingsParameter params[HTTP2_SETTINGS_MAX];
+ size_t params_size = 0;
for (int i = HTTP2_SETTINGS_HEADER_TABLE_SIZE; i < HTTP2_SETTINGS_MAX; ++i) {
Http2SettingsIdentifier id = static_cast<Http2SettingsIdentifier>(i);
@@ -1817,32 +1785,17 @@ Http2ConnectionState::send_settings_frame(const Http2ConnectionSettings &new_set
// Send only difference
if (settings_value != server_settings.get(id)) {
- const Http2SettingsParameter param = {static_cast<uint16_t>(id), settings_value};
+ Http2StreamDebug(ua_session, stream_id, " %s : %u", Http2DebugNames::get_settings_param_name(id), settings_value);
- // Write settings to send buffer
- if (!http2_write_settings(param, iov)) {
- this->send_goaway_frame(this->latest_streamid_in, Http2ErrorCode::HTTP2_ERROR_INTERNAL_ERROR);
- this->ua_session->set_half_close_local_flag(true);
- if (fini_event == nullptr) {
- fini_event = this_ethread()->schedule_imm_local((Continuation *)this, HTTP2_SESSION_EVENT_FINI);
- }
-
- return;
- }
- iov.iov_base = reinterpret_cast<uint8_t *>(iov.iov_base) + HTTP2_SETTINGS_PARAMETER_LEN;
- iov.iov_len -= HTTP2_SETTINGS_PARAMETER_LEN;
- settings_length += HTTP2_SETTINGS_PARAMETER_LEN;
+ params[params_size++] = {static_cast<uint16_t>(id), settings_value};
// Update current settings
server_settings.set(id, new_settings.get(id));
-
- Http2StreamDebug(ua_session, stream_id, " %s : %u", Http2DebugNames::get_settings_param_name(param.id), param.value);
}
}
- settings.finalize(settings_length);
- SCOPED_MUTEX_LOCK(lock, this->ua_session->mutex, this_ethread());
- this->ua_session->handleEvent(HTTP2_SESSION_EVENT_XMIT, &settings);
+ Http2SettingsFrame settings(stream_id, HTTP2_FRAME_NO_FLAG, params, params_size);
+ this->ua_session->xmit(settings);
}
void
@@ -1850,15 +1803,8 @@ Http2ConnectionState::send_ping_frame(Http2StreamId id, uint8_t flag, const uint
{
Http2StreamDebug(ua_session, id, "Send PING frame");
- Http2Frame ping(HTTP2_FRAME_TYPE_PING, id, flag);
-
- ping.alloc(buffer_size_index[HTTP2_FRAME_TYPE_PING]);
- http2_write_ping(opaque_data, ping.write());
- ping.finalize(HTTP2_PING_LEN);
-
- // xmit event
- SCOPED_MUTEX_LOCK(lock, this->ua_session->mutex, this_ethread());
- this->ua_session->handleEvent(HTTP2_SESSION_EVENT_XMIT, &ping);
+ Http2PingFrame ping(id, flag, opaque_data);
+ this->ua_session->xmit(ping);
}
// As for gracefull shutdown, TS should process outstanding stream as long as possible.
@@ -1874,21 +1820,14 @@ Http2ConnectionState::send_goaway_frame(Http2StreamId id, Http2ErrorCode ec)
HTTP2_INCREMENT_THREAD_DYN_STAT(HTTP2_STAT_CONNECTION_ERRORS_COUNT, this_ethread());
}
- Http2Frame frame(HTTP2_FRAME_TYPE_GOAWAY, 0, 0);
- Http2Goaway goaway;
+ this->tx_error_code = {ProxyErrorClass::SSN, static_cast<uint32_t>(ec)};
+ Http2Goaway goaway;
goaway.last_streamid = id;
goaway.error_code = ec;
- frame.alloc(buffer_size_index[HTTP2_FRAME_TYPE_GOAWAY]);
- http2_write_goaway(goaway, frame.write());
- frame.finalize(HTTP2_GOAWAY_LEN);
-
- this->tx_error_code = {ProxyErrorClass::SSN, static_cast<uint32_t>(ec)};
-
- // xmit event
- SCOPED_MUTEX_LOCK(lock, this->ua_session->mutex, this_ethread());
- this->ua_session->handleEvent(HTTP2_SESSION_EVENT_XMIT, &frame);
+ Http2GoawayFrame frame(goaway);
+ this->ua_session->xmit(frame);
}
void
@@ -1897,14 +1836,8 @@ Http2ConnectionState::send_window_update_frame(Http2StreamId id, uint32_t size)
Http2StreamDebug(ua_session, id, "Send WINDOW_UPDATE frame");
// Create WINDOW_UPDATE frame
- Http2Frame window_update(HTTP2_FRAME_TYPE_WINDOW_UPDATE, id, 0x0);
- window_update.alloc(buffer_size_index[HTTP2_FRAME_TYPE_WINDOW_UPDATE]);
- http2_write_window_update(static_cast<uint32_t>(size), window_update.write());
- window_update.finalize(sizeof(uint32_t));
-
- // xmit event
- SCOPED_MUTEX_LOCK(lock, this->ua_session->mutex, this_ethread());
- this->ua_session->handleEvent(HTTP2_SESSION_EVENT_XMIT, &window_update);
+ Http2WindowUpdateFrame window_update(id, size);
+ this->ua_session->xmit(window_update);
}
void
diff --git a/proxy/http2/Http2Frame.cc b/proxy/http2/Http2Frame.cc
new file mode 100644
index 0000000..a731f09
--- /dev/null
+++ b/proxy/http2/Http2Frame.cc
@@ -0,0 +1,253 @@
+/** @file
+
+ Http2Frame
+
+ @section license License
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+ */
+
+#include "Http2Frame.h"
+
+//
+// Http2Frame
+//
+IOBufferReader *
+Http2Frame::reader() const
+{
+ return this->_ioreader;
+}
+
+const Http2FrameHeader &
+Http2Frame::header() const
+{
+ return this->_hdr;
+}
+
+bool
+Http2Frame::is_from_early_data() const
+{
+ return this->_from_early_data;
+}
+
+//
+// DATA Frame
+//
+int64_t
+Http2DataFrame::write_to(MIOBuffer *iobuffer) const
+{
+ // Write frame header
+ uint8_t buf[HTTP2_FRAME_HEADER_LEN];
+ http2_write_frame_header(this->_hdr, make_iovec(buf));
+ int64_t len = iobuffer->write(buf, sizeof(buf));
+
+ // Write frame payload
+ if (this->_reader && this->_payload_len > 0) {
+ int64_t written = 0;
+ // Fill current IOBufferBlock as much as possible to reduce SSL_write() calls
+ while (written < this->_payload_len) {
+ int64_t read_len = std::min(this->_payload_len - written, this->_reader->block_read_avail());
+ written += iobuffer->write(this->_reader->start(), read_len);
+ this->_reader->consume(read_len);
+ }
+ len += written;
+ }
+
+ return len;
+}
+
+//
+// HEADERS Frame
+//
+int64_t
+Http2HeadersFrame::write_to(MIOBuffer *iobuffer) const
+{
+ // Validation
+ if (this->_hdr_block_len > Http2::max_frame_size) {
+ return -1;
+ }
+
+ // Write frame header
+ uint8_t buf[HTTP2_FRAME_HEADER_LEN];
+ http2_write_frame_header(this->_hdr, make_iovec(buf));
+ int64_t len = iobuffer->write(buf, sizeof(buf));
+
+ // Write frame payload
+ if (this->_hdr_block && this->_hdr_block_len > 0) {
+ len += iobuffer->write(this->_hdr_block, this->_hdr_block_len);
+ }
+
+ return len;
+}
+
+//
+// PRIORITY Frame
+//
+int64_t
+Http2PriorityFrame::write_to(MIOBuffer *iobuffer) const
+{
+ ink_abort("not supported yet");
+
+ return 0;
+}
+
+//
+// RST_STREM Frame
+//
+int64_t
+Http2RstStreamFrame::write_to(MIOBuffer *iobuffer) const
+{
+ // Write frame header
+ uint8_t buf[HTTP2_FRAME_HEADER_LEN];
+ http2_write_frame_header(this->_hdr, make_iovec(buf));
+ int64_t len = iobuffer->write(buf, sizeof(buf));
+
+ // Write frame payload
+ uint8_t payload[HTTP2_RST_STREAM_LEN];
+ http2_write_rst_stream(this->_error_code, make_iovec(payload));
+ len += iobuffer->write(payload, sizeof(payload));
+
+ return len;
+}
+
+//
+// SETTINGS Frame
+//
+int64_t
+Http2SettingsFrame::write_to(MIOBuffer *iobuffer) const
+{
+ // Write frame header
+ uint8_t buf[HTTP2_FRAME_HEADER_LEN];
+ http2_write_frame_header(this->_hdr, make_iovec(buf));
+ int64_t len = iobuffer->write(buf, sizeof(buf));
+
+ // Write frame payload
+ for (uint32_t i = 0; i < this->_psize; ++i) {
+ Http2SettingsParameter *p = this->_params + i;
+
+ uint8_t p_buf[HTTP2_SETTINGS_PARAMETER_LEN];
+ http2_write_settings(*p, make_iovec(p_buf));
+ len += iobuffer->write(p_buf, sizeof(p_buf));
+ }
+
+ return len;
+}
+
+//
+// PUSH_PROMISE Frame
+//
+int64_t
+Http2PushPromiseFrame::write_to(MIOBuffer *iobuffer) const
+{
+ // Validation
+ if (this->_hdr_block_len > Http2::max_frame_size) {
+ return -1;
+ }
+
+ // Write frame header
+ uint8_t buf[HTTP2_FRAME_HEADER_LEN];
+ http2_write_frame_header(this->_hdr, make_iovec(buf));
+ int64_t len = iobuffer->write(buf, sizeof(buf));
+
+ // Write frame payload
+ uint8_t p_buf[HTTP2_MAX_FRAME_SIZE];
+ http2_write_push_promise(this->_params, this->_hdr_block, this->_hdr_block_len, make_iovec(p_buf));
+ len += iobuffer->write(p_buf, sizeof(Http2StreamId) + this->_hdr_block_len);
+
+ return len;
+}
+
+//
+// PING Frame
+//
+int64_t
+Http2PingFrame::write_to(MIOBuffer *iobuffer) const
+{
+ // Write frame header
+ uint8_t buf[HTTP2_FRAME_HEADER_LEN];
+ http2_write_frame_header(this->_hdr, make_iovec(buf));
+ int64_t len = iobuffer->write(buf, sizeof(buf));
+
+ // Write frame payload
+ uint8_t payload[HTTP2_PING_LEN] = {0};
+ http2_write_ping(this->_opaque_data, make_iovec(payload));
+ len += iobuffer->write(payload, sizeof(payload));
+
+ return len;
+}
+
+//
+// GOAWAY Frame
+//
+int64_t
+Http2GoawayFrame::write_to(MIOBuffer *iobuffer) const
+{
+ // Write frame header
+ uint8_t buf[HTTP2_FRAME_HEADER_LEN];
+ http2_write_frame_header(this->_hdr, make_iovec(buf));
+ int64_t len = iobuffer->write(buf, sizeof(buf));
+
+ // Write frame payload
+ uint8_t payload[HTTP2_GOAWAY_LEN];
+ http2_write_goaway(this->_params, make_iovec(payload));
+ len += iobuffer->write(payload, sizeof(payload));
+
+ return len;
+}
+
+//
+// WINDOW_UPDATE Frame
+//
+int64_t
+Http2WindowUpdateFrame::write_to(MIOBuffer *iobuffer) const
+{
+ // Write frame header
+ uint8_t buf[HTTP2_FRAME_HEADER_LEN];
+ http2_write_frame_header(this->_hdr, make_iovec(buf));
+ int64_t len = iobuffer->write(buf, sizeof(buf));
+
+ // Write frame payload
+ uint8_t payload[HTTP2_WINDOW_UPDATE_LEN];
+ http2_write_window_update(this->_window, make_iovec(payload));
+ len += iobuffer->write(payload, sizeof(payload));
+
+ return len;
+}
+
+//
+// CONTINUATION Frame
+//
+int64_t
+Http2ContinuationFrame::write_to(MIOBuffer *iobuffer) const
+{
+ // Validation
+ if (this->_hdr_block_len > Http2::max_frame_size) {
+ return -1;
+ }
+
+ // Write frame header
+ uint8_t buf[HTTP2_FRAME_HEADER_LEN];
+ http2_write_frame_header(this->_hdr, make_iovec(buf));
+ int64_t len = iobuffer->write(buf, sizeof(buf));
+
+ // Write frame payload
+ if (this->_hdr_block && this->_hdr_block_len > 0) {
+ len += iobuffer->write(this->_hdr_block, this->_hdr_block_len);
+ }
+
+ return len;
+}
diff --git a/proxy/http2/Http2Frame.h b/proxy/http2/Http2Frame.h
new file mode 100644
index 0000000..1715dd5
--- /dev/null
+++ b/proxy/http2/Http2Frame.h
@@ -0,0 +1,252 @@
+/** @file
+
+ Http2Frame
+
+ @section license License
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+ */
+
+#pragma once
+
+#include "P_Net.h"
+
+#include "HTTP2.h"
+
+/**
+ Incoming HTTP/2 Frame
+ */
+class Http2Frame
+{
+public:
+ Http2Frame(const Http2FrameHeader &h, IOBufferReader *r, bool e = false) : _hdr(h), _ioreader(r), _from_early_data(e) {}
+
+ // Accessor
+ IOBufferReader *reader() const;
+ const Http2FrameHeader &header() const;
+ bool is_from_early_data() const;
+
+private:
+ Http2FrameHeader _hdr;
+ IOBufferReader *_ioreader = nullptr;
+ bool _from_early_data = false;
+};
+
+/**
+ Outgoing HTTP/2 Frame
+ */
+class Http2TxFrame
+{
+public:
+ Http2TxFrame(const Http2FrameHeader &h) : _hdr(h) {}
+
+ // Don't allocate on heap
+ void *operator new(std::size_t) = delete;
+ void *operator new[](std::size_t) = delete;
+
+ virtual int64_t write_to(MIOBuffer *iobuffer) const = 0;
+
+protected:
+ Http2FrameHeader _hdr;
+};
+
+/**
+ DATA Frame
+ */
+class Http2DataFrame : public Http2TxFrame
+{
+public:
+ Http2DataFrame(Http2StreamId stream_id, uint8_t flags, IOBufferReader *r, uint32_t l)
+ : Http2TxFrame({l, HTTP2_FRAME_TYPE_DATA, flags, stream_id}), _reader(r), _payload_len(l)
+ {
+ }
+
+ int64_t write_to(MIOBuffer *iobuffer) const override;
+
+private:
+ IOBufferReader *_reader = nullptr;
+ uint32_t _payload_len = 0;
+};
+
+/**
+ HEADERS Frame
+
+ TODO: support priority info & padding using Http2HeadersParameter
+ */
+class Http2HeadersFrame : public Http2TxFrame
+{
+public:
+ Http2HeadersFrame(Http2StreamId stream_id, uint8_t flags, uint8_t *h, uint32_t l)
+ : Http2TxFrame({l, HTTP2_FRAME_TYPE_HEADERS, flags, stream_id}), _hdr_block(h), _hdr_block_len(l)
+ {
+ }
+
+ int64_t write_to(MIOBuffer *iobuffer) const override;
+
+private:
+ uint8_t *_hdr_block = nullptr;
+ uint32_t _hdr_block_len = 0;
+};
+
+/**
+ PRIORITY Frame
+
+ TODO: implement xmit function
+ */
+class Http2PriorityFrame : public Http2TxFrame
+{
+public:
+ Http2PriorityFrame(Http2StreamId stream_id, uint8_t flags, Http2Priority p)
+ : Http2TxFrame({HTTP2_PRIORITY_LEN, HTTP2_FRAME_TYPE_PRIORITY, flags, stream_id}), _params(p)
+ {
+ }
+
+ int64_t write_to(MIOBuffer *iobuffer) const override;
+
+private:
+ Http2Priority _params;
+};
+
+/**
+ RST_STREAM Frame
+ */
+class Http2RstStreamFrame : public Http2TxFrame
+{
+public:
+ Http2RstStreamFrame(Http2StreamId stream_id, uint32_t e)
+ : Http2TxFrame({HTTP2_RST_STREAM_LEN, HTTP2_FRAME_TYPE_RST_STREAM, HTTP2_FRAME_NO_FLAG, stream_id}), _error_code(e)
+ {
+ }
+
+ int64_t write_to(MIOBuffer *iobuffer) const override;
+
+private:
+ uint32_t _error_code;
+};
+
+/**
+ SETTINGS Frame
+ */
+class Http2SettingsFrame : public Http2TxFrame
+{
+public:
+ Http2SettingsFrame(Http2StreamId stream_id, uint8_t flags) : Http2TxFrame({0, HTTP2_FRAME_TYPE_SETTINGS, flags, stream_id}) {}
+ Http2SettingsFrame(Http2StreamId stream_id, uint8_t flags, Http2SettingsParameter *p, uint32_t s)
+ : Http2TxFrame({static_cast<uint32_t>(HTTP2_SETTINGS_PARAMETER_LEN) * s, HTTP2_FRAME_TYPE_SETTINGS, flags, stream_id}),
+ _params(p),
+ _psize(s)
+ {
+ }
+
+ int64_t write_to(MIOBuffer *iobuffer) const override;
+
+private:
+ Http2SettingsParameter *_params = nullptr;
+ uint32_t _psize = 0;
+};
+
+/**
+ PUSH_PROMISE Frame
+
+ TODO: support padding
+ */
+class Http2PushPromiseFrame : public Http2TxFrame
+{
+public:
+ Http2PushPromiseFrame(Http2StreamId stream_id, uint8_t flags, Http2PushPromise p, uint8_t *h, uint32_t l)
+ : Http2TxFrame({l, HTTP2_FRAME_TYPE_PUSH_PROMISE, flags, stream_id}), _params(p), _hdr_block(h), _hdr_block_len(l)
+ {
+ }
+
+ int64_t write_to(MIOBuffer *iobuffer) const override;
+
+private:
+ Http2PushPromise _params;
+ uint8_t *_hdr_block = nullptr;
+ uint32_t _hdr_block_len = 0;
+};
+
+/**
+ PING Frame
+ */
+class Http2PingFrame : public Http2TxFrame
+{
+public:
+ Http2PingFrame(Http2StreamId stream_id, uint8_t flags, const uint8_t *data)
+ : Http2TxFrame({HTTP2_PING_LEN, HTTP2_FRAME_TYPE_PING, flags, stream_id}), _opaque_data(data)
+ {
+ }
+
+ int64_t write_to(MIOBuffer *iobuffer) const override;
+
+private:
+ const uint8_t *_opaque_data;
+};
+
+/**
+ GOAWAY Frame
+
+ TODO: support Additional Debug Data
+ */
+class Http2GoawayFrame : public Http2TxFrame
+{
+public:
+ Http2GoawayFrame(Http2Goaway p)
+ : Http2TxFrame({HTTP2_GOAWAY_LEN, HTTP2_FRAME_TYPE_GOAWAY, HTTP2_FRAME_NO_FLAG, HTTP2_CONNECTION_CONTROL_STRTEAM}), _params(p)
+ {
+ }
+
+ int64_t write_to(MIOBuffer *iobuffer) const override;
+
+private:
+ Http2Goaway _params;
+};
+
+/**
+ WINDOW_UPDATE Frame
+ */
+class Http2WindowUpdateFrame : public Http2TxFrame
+{
+public:
+ Http2WindowUpdateFrame(Http2StreamId stream_id, uint32_t w)
+ : Http2TxFrame({HTTP2_WINDOW_UPDATE_LEN, HTTP2_FRAME_TYPE_WINDOW_UPDATE, HTTP2_FRAME_NO_FLAG, stream_id}), _window(w)
+ {
+ }
+
+ int64_t write_to(MIOBuffer *iobuffer) const override;
+
+private:
+ uint32_t _window = 0;
+};
+
+/**
+ CONTINUATION Frame
+ */
+class Http2ContinuationFrame : public Http2TxFrame
+{
+public:
+ Http2ContinuationFrame(Http2StreamId stream_id, uint8_t flags, uint8_t *h, uint32_t l)
+ : Http2TxFrame({l, HTTP2_FRAME_TYPE_CONTINUATION, flags, stream_id}), _hdr_block(h), _hdr_block_len(l)
+ {
+ }
+
+ int64_t write_to(MIOBuffer *iobuffer) const override;
+
+private:
+ uint8_t *_hdr_block = nullptr;
+ uint32_t _hdr_block_len = 0;
+};
diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc
index 1a2c242..5f9d6fe 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -370,7 +370,9 @@ Http2Stream::do_io_close(int /* flags */)
if (parent && this->is_client_state_writeable()) {
// Make sure any trailing end of stream frames are sent
// Wee will be removed at send_data_frames or closing connection phase
- static_cast<Http2ClientSession *>(parent)->connection_state.send_data_frames(this);
+ Http2ClientSession *h2_proxy_ssn = static_cast<Http2ClientSession *>(parent);
+ SCOPED_MUTEX_LOCK(lock, h2_proxy_ssn->connection_state.mutex, this_ethread());
+ h2_proxy_ssn->connection_state.send_data_frames(this);
}
clear_timers();
diff --git a/proxy/http2/Makefile.am b/proxy/http2/Makefile.am
index c0ae3ab..c6b494b 100644
--- a/proxy/http2/Makefile.am
+++ b/proxy/http2/Makefile.am
@@ -38,6 +38,8 @@ libhttp2_a_SOURCES = \
HPACK.h \
HTTP2.cc \
HTTP2.h \
+ Http2Frame.cc \
+ Http2Frame.h \
Http2ClientSession.cc \
Http2ClientSession.h \
Http2ConnectionState.cc \
[trafficserver] 21/25: Adjust consume logic in data frame read
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit b64730175ff2f96c121e3ee1d91ceb64e9f6f8b7
Author: Susan Hinrichs <sh...@verizonmedia.com>
AuthorDate: Mon Feb 24 17:39:14 2020 +0000
Adjust consume logic in data frame read
(cherry picked from commit 4094cdad2db9b92ecd4af972f95e253d30281ad3)
---
proxy/http2/Http2ConnectionState.cc | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc
index 1293619..fa5437d 100644
--- a/proxy/http2/Http2ConnectionState.cc
+++ b/proxy/http2/Http2ConnectionState.cc
@@ -181,13 +181,18 @@ rcv_data_frame(Http2ConnectionState &cstate, const Http2Frame &frame)
myreader->consume(HTTP2_DATA_PADLEN_LEN);
}
- while (nbytes < unpadded_length) {
+ if (nbytes < unpadded_length) {
size_t read_len = BUFFER_SIZE_FOR_INDEX(buffer_size_index[HTTP2_FRAME_TYPE_DATA]);
if (nbytes + read_len > unpadded_length) {
read_len -= nbytes + read_len - unpadded_length;
}
- nbytes += writer->write(myreader, read_len);
- myreader->consume(nbytes);
+ unsigned int num_written = writer->write(myreader, read_len);
+ if (num_written != read_len) {
+ myreader->writer()->dealloc_reader(myreader);
+ return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_STREAM, Http2ErrorCode::HTTP2_ERROR_INTERNAL_ERROR);
+ }
+ nbytes += num_written;
+ myreader->consume(num_written);
}
myreader->writer()->dealloc_reader(myreader);
[trafficserver] 01/25: Cleanup: do not re-define _proxy_ssn
variable in local
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 5dea4271f2a69908a9d2581d709b95cb9a0229cd
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Wed Oct 9 15:37:33 2019 -0700
Cleanup: do not re-define _proxy_ssn variable in local
(cherry picked from commit 0ee48a0ccb064226215385bd359d3d6907d97595)
Conflicts:
proxy/http2/Http2Stream.cc
---
proxy/http2/Http2Stream.cc | 38 ++++++++++++++++++--------------------
1 file changed, 18 insertions(+), 20 deletions(-)
diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc
index 076ca48..8cd332e 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -411,10 +411,9 @@ Http2Stream::terminate_if_possible()
{
if (terminate_stream && reentrancy_count == 0) {
REMEMBER(NO_EVENT, this->reentrancy_count);
-
- Http2ClientSession *h2_parent = static_cast<Http2ClientSession *>(parent);
- SCOPED_MUTEX_LOCK(lock, h2_parent->connection_state.mutex, this_ethread());
- h2_parent->connection_state.delete_stream(this);
+ Http2ClientSession *h2_proxy_ssn = static_cast<Http2ClientSession *>(parent);
+ SCOPED_MUTEX_LOCK(lock, h2_proxy_ssn->connection_state.mutex, this_ethread());
+ h2_proxy_ssn->connection_state.delete_stream(this);
destroy();
}
}
@@ -576,8 +575,7 @@ Http2Stream::update_write_request(IOBufferReader *buf_reader, int64_t write_len,
return;
}
ink_release_assert(this->_thread == this_ethread());
-
- Http2ClientSession *parent = static_cast<Http2ClientSession *>(this->get_parent());
+ Http2ClientSession *h2_proxy_ssn = static_cast<Http2ClientSession *>(this->get_parent());
SCOPED_MUTEX_LOCK(lock, write_vio.mutex, this_ethread());
@@ -633,17 +631,17 @@ Http2Stream::update_write_request(IOBufferReader *buf_reader, int64_t write_len,
int len;
const char *value = field->value_get(&len);
if (memcmp(HTTP_VALUE_CLOSE, value, HTTP_LEN_CLOSE) == 0) {
- SCOPED_MUTEX_LOCK(lock, parent->connection_state.mutex, this_ethread());
- if (parent->connection_state.get_shutdown_state() == HTTP2_SHUTDOWN_NONE) {
- parent->connection_state.set_shutdown_state(HTTP2_SHUTDOWN_NOT_INITIATED, Http2ErrorCode::HTTP2_ERROR_NO_ERROR);
+ SCOPED_MUTEX_LOCK(lock, h2_proxy_ssn->connection_state.mutex, this_ethread());
+ if (h2_proxy_ssn->connection_state.get_shutdown_state() == HTTP2_SHUTDOWN_NONE) {
+ h2_proxy_ssn->connection_state.set_shutdown_state(HTTP2_SHUTDOWN_NOT_INITIATED, Http2ErrorCode::HTTP2_ERROR_NO_ERROR);
}
}
}
{
- SCOPED_MUTEX_LOCK(lock, parent->connection_state.mutex, this_ethread());
+ SCOPED_MUTEX_LOCK(lock, h2_proxy_ssn->connection_state.mutex, this_ethread());
// Send the response header back
- parent->connection_state.send_headers_frame(this);
+ h2_proxy_ssn->connection_state.send_headers_frame(this);
}
// See if the response is chunked. Set up the dechunking logic if it is
@@ -700,25 +698,25 @@ Http2Stream::signal_write_event(bool call_update)
void
Http2Stream::push_promise(URL &url, const MIMEField *accept_encoding)
{
- Http2ClientSession *parent = static_cast<Http2ClientSession *>(this->get_parent());
- SCOPED_MUTEX_LOCK(lock, parent->connection_state.mutex, this_ethread());
- parent->connection_state.send_push_promise_frame(this, url, accept_encoding);
+ Http2ClientSession *h2_proxy_ssn = static_cast<Http2ClientSession *>(this->get_parent());
+ SCOPED_MUTEX_LOCK(lock, h2_proxy_ssn->connection_state.mutex, this_ethread());
+ h2_proxy_ssn->connection_state.send_push_promise_frame(this, url, accept_encoding);
}
void
Http2Stream::send_response_body(bool call_update)
{
- Http2ClientSession *parent = static_cast<Http2ClientSession *>(this->get_parent());
- inactive_timeout_at = Thread::get_hrtime() + inactive_timeout;
+ Http2ClientSession *h2_proxy_ssn = static_cast<Http2ClientSession *>(this->get_parent());
+ inactive_timeout_at = Thread::get_hrtime() + inactive_timeout;
if (Http2::stream_priority_enabled) {
- SCOPED_MUTEX_LOCK(lock, parent->connection_state.mutex, this_ethread());
- parent->connection_state.schedule_stream(this);
+ SCOPED_MUTEX_LOCK(lock, h2_proxy_ssn->connection_state.mutex, this_ethread());
+ h2_proxy_ssn->connection_state.schedule_stream(this);
// signal_write_event() will be called from `Http2ConnectionState::send_data_frames_depends_on_priority()`
// when write_vio is consumed
} else {
- SCOPED_MUTEX_LOCK(lock, parent->connection_state.mutex, this_ethread());
- parent->connection_state.send_data_frames(this);
+ SCOPED_MUTEX_LOCK(lock, h2_proxy_ssn->connection_state.mutex, this_ethread());
+ h2_proxy_ssn->connection_state.send_data_frames(this);
this->signal_write_event(call_update);
// XXX The call to signal_write_event can destroy/free the Http2Stream.
// Don't modify the Http2Stream after calling this method.
[trafficserver] 11/25: Add virtual destructors for Http2TxFrame,
Http2FrequencyCounter
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit a65bc9381dc6f3c5628c989dff6f0ff82b76c765
Author: Alan M. Carroll <am...@waka.solidwallofcode.com>
AuthorDate: Mon Jan 27 11:03:33 2020 -0600
Add virtual destructors for Http2TxFrame, Http2FrequencyCounter
(cherry picked from commit 11aeedfc0e08391555bfa880469b5b371639530f)
---
proxy/http2/Http2Frame.h | 1 +
proxy/http2/Http2FrequencyCounter.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/proxy/http2/Http2Frame.h b/proxy/http2/Http2Frame.h
index 1715dd5..dd68b8c 100644
--- a/proxy/http2/Http2Frame.h
+++ b/proxy/http2/Http2Frame.h
@@ -53,6 +53,7 @@ class Http2TxFrame
{
public:
Http2TxFrame(const Http2FrameHeader &h) : _hdr(h) {}
+ virtual ~Http2TxFrame() {}
// Don't allocate on heap
void *operator new(std::size_t) = delete;
diff --git a/proxy/http2/Http2FrequencyCounter.h b/proxy/http2/Http2FrequencyCounter.h
index 0b6de17..bcd3dbb 100644
--- a/proxy/http2/Http2FrequencyCounter.h
+++ b/proxy/http2/Http2FrequencyCounter.h
@@ -31,6 +31,7 @@ class Http2FrequencyCounter
public:
void increment(uint16_t amount = 1);
uint32_t get_count();
+ virtual ~Http2FrequencyCounter() {}
protected:
uint16_t _count[2] = {0};
[trafficserver] 16/25: Signal VC_EVENT_READ_COMPLETE when ATS
received END_STREAM flag
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 327f95c3cf3c7f8b7acea35970a47153272b45f5
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Fri Feb 7 21:47:54 2020 +0900
Signal VC_EVENT_READ_COMPLETE when ATS received END_STREAM flag
(cherry picked from commit 2868ce196d537e351a34aec981d8b820131b6559)
---
proxy/http2/Http2ConnectionState.cc | 17 ++++--
tests/gold_tests/h2/gold/nghttp_0_stdout.gold | 17 ++++++
tests/gold_tests/h2/nghttp.test.py | 84 +++++++++++++++++++++++++++
3 files changed, 114 insertions(+), 4 deletions(-)
diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc
index 5ca6549..1293619 100644
--- a/proxy/http2/Http2ConnectionState.cc
+++ b/proxy/http2/Http2ConnectionState.cc
@@ -139,11 +139,17 @@ rcv_data_frame(Http2ConnectionState &cstate, const Http2Frame &frame)
return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR,
"recv data bad payload length");
}
- }
- // If Data length is 0, do nothing.
- if (payload_length == 0) {
- return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_NONE);
+ // Pure END_STREAM
+ if (payload_length == 0) {
+ stream->signal_read_event(VC_EVENT_READ_COMPLETE);
+ return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_NONE);
+ }
+ } else {
+ // If payload length is 0 without END_STREAM flag, do nothing
+ if (payload_length == 0) {
+ return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_NONE);
+ }
}
// Check whether Window Size is acceptable
@@ -373,6 +379,9 @@ rcv_headers_frame(Http2ConnectionState &cstate, const Http2Frame &frame)
stream->new_transaction();
// Send request header to SM
stream->send_request(cstate);
+ } else {
+ // Signal VC_EVENT_READ_COMPLETE becasue received trailing header fields with END_STREAM flag
+ stream->signal_read_event(VC_EVENT_READ_COMPLETE);
}
} else {
// NOTE: Expect CONTINUATION Frame. Do NOT change state of stream or decode
diff --git a/tests/gold_tests/h2/gold/nghttp_0_stdout.gold b/tests/gold_tests/h2/gold/nghttp_0_stdout.gold
new file mode 100644
index 0000000..1487943
--- /dev/null
+++ b/tests/gold_tests/h2/gold/nghttp_0_stdout.gold
@@ -0,0 +1,17 @@
+``
+[``] send HEADERS frame <length=``, flags=0x04, stream_id=1>
+``
+``trailer: foo
+``
+[``] send DATA frame <length=``, flags=0x00, stream_id=1>
+``
+[``] send HEADERS frame <length=8, flags=0x05, stream_id=1>
+``; END_STREAM | END_HEADERS
+``
+``foo: bar
+``
+[``] recv (stream_id=1) :status: 200
+``
+[``] recv RST_STREAM frame <length=4, flags=0x00, stream_id=1>
+``(error_code=NO_ERROR(0x00))
+``
diff --git a/tests/gold_tests/h2/nghttp.test.py b/tests/gold_tests/h2/nghttp.test.py
new file mode 100644
index 0000000..72cc51f
--- /dev/null
+++ b/tests/gold_tests/h2/nghttp.test.py
@@ -0,0 +1,84 @@
+'''
+'''
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import os
+Test.Summary = '''
+Test with nghttp
+'''
+# need Curl
+Test.SkipUnless(
+ Condition.HasProgram("nghttp", "Nghttp need to be installed on system for this test to work"),
+)
+Test.ContinueOnFail = True
+
+# ----
+# Setup Origin Server
+# ----
+microserver = Test.MakeOriginServer("microserver")
+
+# 128KB
+post_body = "0123456789abcdef" * 8192
+post_body_file = open(os.path.join(Test.RunDirectory, "post_body"), "w")
+post_body_file.write(post_body)
+post_body_file.close()
+
+# For Test Case 0
+microserver.addResponse("sessionlog.json",
+ {"headers": "POST /post HTTP/1.1\r\nHost: www.example.com\r\nTrailer: foo\r\n\r\n",
+ "timestamp": "1469733493.993", "body": post_body},
+ {"headers": "HTTP/1.1 200 OK\r\nServer: microserver\r\nConnection: close\r\n\r\n", "timestamp": "1469733493.993", "body": ""})
+
+# ----
+# Setup ATS
+# ----
+ts = Test.MakeATSProcess("ts", select_ports=True, enable_tls=True)
+
+# add ssl materials like key, certificates for the server
+ts.addSSLfile("ssl/server.pem")
+ts.addSSLfile("ssl/server.key")
+
+ts.Disk.remap_config.AddLine(
+ 'map /post http://127.0.0.1:{0}/post'.format(microserver.Variables.Port)
+)
+
+ts.Disk.ssl_multicert_config.AddLine(
+ 'dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key'
+)
+
+ts.Disk.records_config.update({
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.debug.tags': 'http',
+ 'proxy.config.ssl.server.cert.path': '{0}'.format(ts.Variables.SSLDir),
+ 'proxy.config.ssl.server.private_key.path': '{0}'.format(ts.Variables.SSLDir),
+ 'proxy.config.http.cache.http': 0,
+ 'proxy.config.http2.active_timeout_in': 3,
+})
+
+# ----
+# Test Cases
+# ----
+
+# Test Case 0: Trailer
+tr = Test.AddTestRun()
+tr.Processes.Default.Command = "nghttp -v --no-dep 'https://127.0.0.1:{0}/post' --trailer 'foo: bar' -d 'post_body'".format(
+ ts.Variables.ssl_port)
+tr.Processes.Default.ReturnCode = 0
+tr.Processes.Default.StartBefore(microserver)
+tr.Processes.Default.StartBefore(Test.Processes.ts)
+tr.Processes.Default.Streams.stdout = "gold/nghttp_0_stdout.gold"
+tr.StillRunningAfter = microserver
[trafficserver] 18/25: Update inactive_timeout_at in
Http2Stream::signal_read_event()
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 523429733ba8cc7944cd89808032e0037683c1d8
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Thu Jan 30 16:24:17 2020 +0900
Update inactive_timeout_at in Http2Stream::signal_read_event()
(cherry picked from commit 7d3781092e7aadef386dc8693a735230a15aea12)
---
proxy/http2/Http2Stream.cc | 1 +
1 file changed, 1 insertion(+)
diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc
index b14b6e7..ed160c9 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -671,6 +671,7 @@ Http2Stream::signal_read_event(int event)
MUTEX_TRY_LOCK(lock, read_vio.cont->mutex, this_ethread());
if (lock.is_locked()) {
+ inactive_timeout_at = Thread::get_hrtime() + inactive_timeout;
this->read_vio.cont->handleEvent(event, &this->read_vio);
} else {
if (this->_read_vio_event) {
[trafficserver] 24/25: Fix test_libhttp2 build issue of 8.1.x on
Debian/Ubuntu
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit a3b6c77d1911bee05e9da2560f0ee5bd5a7a239c
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Tue Mar 31 15:44:57 2020 +0900
Fix test_libhttp2 build issue of 8.1.x on Debian/Ubuntu
We don't see this issue on master, because we got rid of TCL.
---
proxy/http2/Makefile.am | 1 +
1 file changed, 1 insertion(+)
diff --git a/proxy/http2/Makefile.am b/proxy/http2/Makefile.am
index 86cd765..beb8692 100644
--- a/proxy/http2/Makefile.am
+++ b/proxy/http2/Makefile.am
@@ -76,6 +76,7 @@ test_libhttp2_LDADD = \
$(top_builddir)/lib/records/librecords_p.a \
$(top_builddir)/mgmt/libmgmt_p.la \
$(top_builddir)/proxy/shared/libUglyLogStubs.a \
+ @LIBTCL@ \
@HWLOC_LIBS@
test_libhttp2_CPPFLAGS = $(AM_CPPFLAGS)\
[trafficserver] 15/25: Avoid unnecesarry copy on POST request over
HTTP/2
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit b5ed51edb7340a5a13798f7d7d4ebb951ec8045a
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Thu Sep 26 15:46:05 2019 +0900
Avoid unnecesarry copy on POST request over HTTP/2
(cherry picked from commit c55001bec19d4db63f58b484186b942fbec5ae2a)
Conflicts:
proxy/http2/Http2Stream.cc
---
proxy/http2/Http2ConnectionState.cc | 19 ++++--
proxy/http2/Http2Stream.cc | 113 +++++++++++++++++++-----------------
proxy/http2/Http2Stream.h | 14 ++++-
3 files changed, 86 insertions(+), 60 deletions(-)
diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc
index 1eb4fb1..5ca6549 100644
--- a/proxy/http2/Http2ConnectionState.cc
+++ b/proxy/http2/Http2ConnectionState.cc
@@ -161,6 +161,11 @@ rcv_data_frame(Http2ConnectionState &cstate, const Http2Frame &frame)
stream->decrement_server_rwnd(payload_length);
const uint32_t unpadded_length = payload_length - pad_length;
+ MIOBuffer *writer = stream->read_vio_writer();
+ if (writer == nullptr) {
+ return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_STREAM, Http2ErrorCode::HTTP2_ERROR_INTERNAL_ERROR);
+ }
+
// If we call write() multiple times, we must keep the same reader, so we can
// update its offset via consume. Otherwise, we will read the same data on the
// second time through
@@ -169,18 +174,24 @@ rcv_data_frame(Http2ConnectionState &cstate, const Http2Frame &frame)
if (frame.header().flags & HTTP2_FLAGS_DATA_PADDED) {
myreader->consume(HTTP2_DATA_PADLEN_LEN);
}
- while (nbytes < payload_length - pad_length) {
+
+ while (nbytes < unpadded_length) {
size_t read_len = BUFFER_SIZE_FOR_INDEX(buffer_size_index[HTTP2_FRAME_TYPE_DATA]);
if (nbytes + read_len > unpadded_length) {
read_len -= nbytes + read_len - unpadded_length;
}
- nbytes += stream->request_buffer.write(myreader, read_len);
+ nbytes += writer->write(myreader, read_len);
myreader->consume(nbytes);
- // If there is an outstanding read, update the buffer
- stream->update_read_request(INT64_MAX, true);
}
myreader->writer()->dealloc_reader(myreader);
+ if (frame.header().flags & HTTP2_FLAGS_DATA_END_STREAM) {
+ // TODO: set total written size to read_vio.nbytes
+ stream->signal_read_event(VC_EVENT_READ_COMPLETE);
+ } else {
+ stream->signal_read_event(VC_EVENT_READ_READY);
+ }
+
uint32_t initial_rwnd = cstate.server_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE);
uint32_t min_rwnd = std::min(initial_rwnd, cstate.server_settings.get(HTTP2_SETTINGS_MAX_FRAME_SIZE));
// Connection level WINDOW UPDATE
diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc
index 74d910f..b14b6e7 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -55,8 +55,7 @@ Http2Stream::init(Http2StreamId sid, ssize_t initial_rwnd)
HTTP2_INCREMENT_THREAD_DYN_STAT(HTTP2_STAT_CURRENT_CLIENT_STREAM_COUNT, _thread);
HTTP2_INCREMENT_THREAD_DYN_STAT(HTTP2_STAT_TOTAL_CLIENT_STREAM_COUNT, _thread);
- sm_reader = request_reader = request_buffer.alloc_reader();
- http_parser_init(&http_parser);
+ sm_reader = this->_request_buffer.alloc_reader();
// FIXME: Are you sure? every "stream" needs request_header?
_req_header.create(HTTP_TYPE_REQUEST);
response_header.create(HTTP_TYPE_RESPONSE);
@@ -196,25 +195,33 @@ Http2Stream::send_request(Http2ConnectionState &cstate)
int bufindex;
int dumpoffset = 0;
int done, tmp;
- IOBufferBlock *block;
do {
- bufindex = 0;
- tmp = dumpoffset;
- block = request_buffer.get_current_block();
+ bufindex = 0;
+ tmp = dumpoffset;
+ IOBufferBlock *block = this->_request_buffer.get_current_block();
if (!block) {
- request_buffer.add_block();
- block = request_buffer.get_current_block();
+ this->_request_buffer.add_block();
+ block = this->_request_buffer.get_current_block();
}
done = _req_header.print(block->start(), block->write_avail(), &bufindex, &tmp);
dumpoffset += bufindex;
- request_buffer.fill(bufindex);
+ this->_request_buffer.fill(bufindex);
if (!done) {
- request_buffer.add_block();
+ this->_request_buffer.add_block();
}
} while (!done);
- // Is there a read_vio request waiting?
- this->update_read_request(INT64_MAX, true);
+ if (bufindex == 0) {
+ // No data to signal read event
+ return;
+ }
+
+ if (this->recv_end_stream) {
+ this->read_vio.nbytes = bufindex;
+ this->signal_read_event(VC_EVENT_READ_COMPLETE);
+ } else {
+ this->signal_read_event(VC_EVENT_READ_READY);
+ }
}
bool
@@ -335,9 +342,7 @@ Http2Stream::do_io_read(Continuation *c, int64_t nbytes, MIOBuffer *buf)
read_vio.vc_server = this;
read_vio.op = VIO::READ;
- // Is there already data in the request_buffer? If so, copy it over and then
- // schedule a READ_READY or READ_COMPLETE event after we return.
- update_read_request(nbytes, false, true);
+ // TODO: re-enable read_vio
return &read_vio;
}
@@ -529,44 +534,26 @@ Http2Stream::update_read_request(int64_t read_len, bool call_update, bool check_
ink_release_assert(this->_thread == this_ethread());
SCOPED_MUTEX_LOCK(lock, read_vio.mutex, this_ethread());
- if (read_vio.nbytes > 0 && read_vio.ndone <= read_vio.nbytes) {
- // If this vio has a different buffer, we must copy
- ink_release_assert(this_ethread() == this->_thread);
- if (read_vio.buffer.writer() != (&request_buffer)) {
- int64_t num_to_read = read_vio.nbytes - read_vio.ndone;
- if (num_to_read > read_len) {
- num_to_read = read_len;
- }
- if (num_to_read > 0) {
- int bytes_added = read_vio.buffer.writer()->write(request_reader, num_to_read);
- if (bytes_added > 0 || (check_eos && recv_end_stream)) {
- request_reader->consume(bytes_added);
- read_vio.ndone += bytes_added;
- int send_event = (read_vio.nbytes == read_vio.ndone || recv_end_stream) ? VC_EVENT_READ_COMPLETE : VC_EVENT_READ_READY;
- if (call_update) { // Safe to call vio handler directly
- inactive_timeout_at = Thread::get_hrtime() + inactive_timeout;
- if (read_vio.cont && this->current_reader) {
- read_vio.cont->handleEvent(send_event, &read_vio);
- }
- } else { // Called from do_io_read. Still setting things up. Send event to handle this after the dust settles
- read_event = send_tracked_event(read_event, send_event, &read_vio);
- }
- }
- }
- } else {
- // Try to be smart and only signal if there was additional data
- int send_event = (read_vio.nbytes == read_vio.ndone) ? VC_EVENT_READ_COMPLETE : VC_EVENT_READ_READY;
- if (request_reader->read_avail() > 0 || send_event == VC_EVENT_READ_COMPLETE) {
- if (call_update) { // Safe to call vio handler directly
- inactive_timeout_at = Thread::get_hrtime() + inactive_timeout;
- if (read_vio.cont && this->current_reader) {
- read_vio.cont->handleEvent(send_event, &read_vio);
- }
- } else { // Called from do_io_read. Still setting things up. Send event
- // to handle this after the dust settles
- read_event = send_tracked_event(read_event, send_event, &read_vio);
- }
+ if (read_vio.nbytes == 0) {
+ return;
+ }
+
+ // Try to be smart and only signal if there was additional data
+ int send_event = VC_EVENT_READ_READY;
+ if (read_vio.ntodo() == 0 || (this->recv_end_stream && this->read_vio.nbytes != INT64_MAX)) {
+ send_event = VC_EVENT_READ_COMPLETE;
+ }
+
+ int64_t read_avail = this->read_vio.buffer.writer()->max_read_avail();
+ if (read_avail > 0 || send_event == VC_EVENT_READ_COMPLETE) {
+ if (call_update) { // Safe to call vio handler directly
+ inactive_timeout_at = Thread::get_hrtime() + inactive_timeout;
+ if (read_vio.cont && this->current_reader) {
+ read_vio.cont->handleEvent(send_event, &read_vio);
}
+ } else { // Called from do_io_read. Still setting things up. Send event
+ // to handle this after the dust settles
+ read_event = send_tracked_event(read_event, send_event, &read_vio);
}
}
}
@@ -676,6 +663,24 @@ Http2Stream::update_write_request(IOBufferReader *buf_reader, int64_t write_len,
}
void
+Http2Stream::signal_read_event(int event)
+{
+ if (this->read_vio.cont == nullptr || this->read_vio.cont->mutex == nullptr || this->read_vio.op == VIO::NONE) {
+ return;
+ }
+
+ MUTEX_TRY_LOCK(lock, read_vio.cont->mutex, this_ethread());
+ if (lock.is_locked()) {
+ this->read_vio.cont->handleEvent(event, &this->read_vio);
+ } else {
+ if (this->_read_vio_event) {
+ this->_read_vio_event->cancel();
+ }
+ this->_read_vio_event = this_ethread()->schedule_imm(read_vio.cont, event, &read_vio);
+ }
+}
+
+void
Http2Stream::signal_write_event(bool call_update)
{
if (this->write_vio.cont == nullptr || this->write_vio.op == VIO::NONE) {
@@ -755,7 +760,7 @@ Http2Stream::destroy()
// Safe to initiate SSN_CLOSE if this is the last stream
if (parent) {
- cid = _proxy_ssn->connection_id();
+ cid = parent->connection_id();
Http2ClientSession *h2_proxy_ssn = static_cast<Http2ClientSession *>(parent);
SCOPED_MUTEX_LOCK(lock, h2_proxy_ssn->connection_state.mutex, this_ethread());
@@ -803,7 +808,7 @@ Http2Stream::destroy()
response_header.destroy();
// Drop references to all buffer data
- request_buffer.clear();
+ this->_request_buffer.clear();
// Free the mutexes in the VIO
read_vio.mutex.clear();
diff --git a/proxy/http2/Http2Stream.h b/proxy/http2/Http2Stream.h
index 261f44c..461f228 100644
--- a/proxy/http2/Http2Stream.h
+++ b/proxy/http2/Http2Stream.h
@@ -73,7 +73,10 @@ public:
void terminate_if_possible();
void update_read_request(int64_t read_len, bool send_update, bool check_eos = false);
void update_write_request(IOBufferReader *buf_reader, int64_t write_len, bool send_update);
+
+ void signal_read_event(int event);
void signal_write_event(bool call_update);
+
void restart_sending();
void push_promise(URL &url, const MIMEField *accept_encoding);
@@ -119,6 +122,7 @@ public:
void update_initial_rwnd(Http2WindowSize new_size);
bool has_trailing_header() const;
void set_request_headers(HTTPHdr &h2_headers);
+ MIOBuffer *read_vio_writer() const;
//////////////////
// Variables
@@ -137,8 +141,6 @@ public:
HTTPHdr response_header;
IOBufferReader *response_reader = nullptr;
- IOBufferReader *request_reader = nullptr;
- MIOBuffer request_buffer = CLIENT_CONNECTION_FIRST_READ_BUFFER_SIZE_INDEX;
Http2DependencyTree::Node *priority_node = nullptr;
private:
@@ -160,6 +162,8 @@ private:
int64_t _http_sm_id = -1;
HTTPHdr _req_header;
+ MIOBuffer _request_buffer = CLIENT_CONNECTION_FIRST_READ_BUFFER_SIZE_INDEX;
+ int64_t read_vio_nbytes;
VIO read_vio;
VIO write_vio;
@@ -323,3 +327,9 @@ Http2Stream::is_first_transaction() const
{
return is_first_transaction_flag;
}
+
+inline MIOBuffer *
+Http2Stream::read_vio_writer() const
+{
+ return this->read_vio.get_writer();
+}
[trafficserver] 20/25: Another option to fix potential HTTP/2 vio
stall
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit b537c7e300e5ac2345f7139fdad628ad40e07dc4
Author: Susan Hinrichs <sh...@verizonmedia.com>
AuthorDate: Mon Feb 10 23:27:55 2020 +0000
Another option to fix potential HTTP/2 vio stall
(cherry picked from commit 87e79069f07daf17f54958de1d1f9d4198ccc266)
Conflicts:
proxy/http2/Http2Stream.cc
proxy/http2/Http2Stream.h
---
proxy/http2/Http2Stream.cc | 73 +++++++++++++++++++++-------------------------
proxy/http2/Http2Stream.h | 4 ++-
2 files changed, 36 insertions(+), 41 deletions(-)
diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc
index e3fa693..5107a3d 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -76,7 +76,13 @@ Http2Stream::main_event_handler(int event, void *edata)
Event *e = static_cast<Event *>(edata);
reentrancy_count++;
- if (e == cross_thread_event) {
+ if (e == _read_vio_event) {
+ this->signal_read_event(e->callback_event);
+ return 0;
+ } else if (e == _write_vio_event) {
+ this->signal_write_event(e->callback_event);
+ return 0;
+ } else if (e == cross_thread_event) {
cross_thread_event = nullptr;
} else if (e == active_event) {
event = VC_EVENT_ACTIVE_TIMEOUT;
@@ -98,41 +104,17 @@ Http2Stream::main_event_handler(int event, void *edata)
case VC_EVENT_ACTIVE_TIMEOUT:
case VC_EVENT_INACTIVITY_TIMEOUT:
if (current_reader && read_vio.ntodo() > 0) {
- MUTEX_TRY_LOCK(lock, read_vio.mutex, this_ethread());
- if (lock.is_locked()) {
- read_vio.cont->handleEvent(event, &read_vio);
- } else {
- if (this->_read_vio_event) {
- this->_read_vio_event->cancel();
- }
- this->_read_vio_event = this_ethread()->schedule_imm(read_vio.cont, event, &read_vio);
- }
+ this->signal_read_event(event);
} else if (current_reader && write_vio.ntodo() > 0) {
- MUTEX_TRY_LOCK(lock, write_vio.mutex, this_ethread());
- if (lock.is_locked()) {
- write_vio.cont->handleEvent(event, &write_vio);
- } else {
- if (this->_write_vio_event) {
- this->_write_vio_event->cancel();
- }
- this->_write_vio_event = this_ethread()->schedule_imm(write_vio.cont, event, &write_vio);
- }
+ this->signal_write_event(event);
}
break;
case VC_EVENT_WRITE_READY:
case VC_EVENT_WRITE_COMPLETE:
inactive_timeout_at = Thread::get_hrtime() + inactive_timeout;
if (e->cookie == &write_vio) {
- if (write_vio.mutex) {
- MUTEX_TRY_LOCK(lock, write_vio.mutex, this_ethread());
- if (lock.is_locked() && write_vio.cont && this->current_reader) {
- write_vio.cont->handleEvent(event, &write_vio);
- } else {
- if (this->_write_vio_event) {
- this->_write_vio_event->cancel();
- }
- this->_write_vio_event = this_ethread()->schedule_imm(write_vio.cont, event, &write_vio);
- }
+ if (write_vio.mutex && write_vio.cont && this->current_reader) {
+ this->signal_write_event(event);
}
} else {
update_write_request(write_vio.get_reader(), INT64_MAX, true);
@@ -142,16 +124,8 @@ Http2Stream::main_event_handler(int event, void *edata)
case VC_EVENT_READ_READY:
inactive_timeout_at = Thread::get_hrtime() + inactive_timeout;
if (e->cookie == &read_vio) {
- if (read_vio.mutex) {
- MUTEX_TRY_LOCK(lock, read_vio.mutex, this_ethread());
- if (lock.is_locked() && read_vio.cont && this->current_reader) {
- read_vio.cont->handleEvent(event, &read_vio);
- } else {
- if (this->_read_vio_event) {
- this->_read_vio_event->cancel();
- }
- this->_read_vio_event = this_ethread()->schedule_imm(read_vio.cont, event, &read_vio);
- }
+ if (read_vio.mutex && read_vio.cont && this->current_reader) {
+ signal_read_event(event);
}
} else {
this->update_read_request(INT64_MAX, true);
@@ -673,7 +647,26 @@ Http2Stream::signal_read_event(int event)
if (this->_read_vio_event) {
this->_read_vio_event->cancel();
}
- this->_read_vio_event = this_ethread()->schedule_imm(read_vio.cont, event, &read_vio);
+ this->_read_vio_event = this_ethread()->schedule_in(this, retry_delay, event, &read_vio);
+ }
+}
+
+void
+Http2Stream::signal_write_event(int event)
+{
+ if (this->write_vio.cont == nullptr || this->write_vio.cont->mutex == nullptr || this->write_vio.op == VIO::NONE) {
+ return;
+ }
+
+ MUTEX_TRY_LOCK(lock, write_vio.cont->mutex, this_ethread());
+ if (lock.is_locked()) {
+ inactive_timeout_at = Thread::get_hrtime() + inactive_timeout;
+ this->write_vio.cont->handleEvent(event, &this->write_vio);
+ } else {
+ if (this->_write_vio_event) {
+ this->_write_vio_event->cancel();
+ }
+ this->_write_vio_event = this_ethread()->schedule_in(this, retry_delay, event, &write_vio);
}
}
diff --git a/proxy/http2/Http2Stream.h b/proxy/http2/Http2Stream.h
index 461f228..fc52cf3 100644
--- a/proxy/http2/Http2Stream.h
+++ b/proxy/http2/Http2Stream.h
@@ -49,7 +49,8 @@ enum class Http2StreamMilestone {
class Http2Stream : public ProxyClientTransaction
{
public:
- using super = ProxyClientTransaction; ///< Parent type.
+ const int retry_delay = HRTIME_MSECONDS(10);
+ using super = ProxyClientTransaction; ///< Parent type.
Http2Stream(Http2StreamId sid = 0, ssize_t initial_rwnd = Http2::initial_window_size);
@@ -75,6 +76,7 @@ public:
void update_write_request(IOBufferReader *buf_reader, int64_t write_len, bool send_update);
void signal_read_event(int event);
+ void signal_write_event(int event);
void signal_write_event(bool call_update);
void restart_sending();
[trafficserver] 17/25: Fix crash when H2 client does not set
End-of-data bit
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 71519ba534fb5bd5823f2d4a6b7ba56d371775a0
Author: Susan Hinrichs <sh...@verizonmedia.com>
AuthorDate: Tue Feb 11 22:23:46 2020 +0000
Fix crash when H2 client does not set End-of-data bit
(cherry picked from commit 7d4f66b0b25f6aaccc2f9c7d2bde58ce2b47bd15)
---
proxy/http/HttpSM.cc | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index b137470..e431c40 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -2696,6 +2696,8 @@ HttpSM::tunnel_handler_post(int event, void *data)
handle_post_failure();
break;
case HTTP_SM_POST_UA_FAIL:
+ // Client side failed. Shutdown and go home. No need to communicate back to UA
+ terminate_sm = true;
break;
case HTTP_SM_POST_SUCCESS:
// It's time to start reading the response
@@ -3441,9 +3443,7 @@ HttpSM::tunnel_handler_post_ua(int event, HttpTunnelProducer *p)
hsm_release_assert(ua_entry->in_tunnel == true);
if (p->consumer_list.head && p->consumer_list.head->vc_type == HT_TRANSFORM) {
hsm_release_assert(post_transform_info.entry->in_tunnel == true);
- } else if (server_entry != nullptr) {
- hsm_release_assert(server_entry->in_tunnel == true);
- }
+ } // server side may have completed before the user agent side, so it may no longer be in tunnel
break;
case VC_EVENT_READ_COMPLETE:
@@ -3580,7 +3580,8 @@ HttpSM::tunnel_handler_post_server(int event, HttpTunnelConsumer *c)
case VC_EVENT_WRITE_COMPLETE:
// Completed successfully
- c->write_success = true;
+ c->write_success = true;
+ server_entry->in_tunnel = false;
break;
default:
ink_release_assert(0);
[trafficserver] 04/25: Dechunk chunked contents on HttpTunnel if
client protocol is HTTP/2 or HTTP/3
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 23b97f02bf1a5eeee26d1e7264b9a302baae7efa
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Mon Sep 2 14:31:56 2019 +0900
Dechunk chunked contents on HttpTunnel if client protocol is HTTP/2 or HTTP/3
(cherry picked from commit 3ac74b5157cf6705b94d5eeb910953e25a6cdfc0)
Conflicts:
proxy/http/HttpTransact.cc
proxy/http2/Http2Stream.cc
proxy/http2/Http2Stream.h
---
proxy/http/HttpTransact.cc | 17 ++++++-----
proxy/http2/Http2Stream.cc | 76 +++-------------------------------------------
proxy/http2/Http2Stream.h | 14 +--------
3 files changed, 15 insertions(+), 92 deletions(-)
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index 72680ae..cebef72 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -6833,10 +6833,10 @@ HttpTransact::handle_response_keep_alive_headers(State *s, HTTPVersion ver, HTTP
// to the client to keep the connection alive.
// Insert a Transfer-Encoding header in the response if necessary.
- // check that the client is HTTP 1.1 and the conf allows chunking or the client
- // protocol unchunks before returning to the user agent (i.e. is http/2)
- if (s->client_info.http_version == HTTPVersion(1, 1) && s->txn_conf->chunking_enabled == 1 &&
- s->state_machine->ua_txn->is_chunked_encoding_supported() &&
+ // check that the client protocol is HTTP/1.1 and the conf allows chunking or
+ // the client protocol doesn't support chunked transfer coding (i.e. HTTP/1.0, HTTP/2)
+ if (s->state_machine->ua_txn && s->state_machine->ua_txn->is_chunked_encoding_supported() &&
+ s->client_info.http_version == HTTPVersion(1, 1) && s->txn_conf->chunking_enabled == 1 &&
// if we're not sending a body, don't set a chunked header regardless of server response
!is_response_body_precluded(s->hdr_info.client_response.status_get(), s->method) &&
// we do not need chunked encoding for internal error messages
@@ -6868,12 +6868,13 @@ HttpTransact::handle_response_keep_alive_headers(State *s, HTTPVersion ver, HTTP
}
// Close the connection if client_info is not keep-alive.
- // Otherwise, if we cannot trust the content length and the client process chunked encoding, we will close the connection
- // unless we are going to use chunked encoding or the client issued
- // a PUSH request
+ // Otherwise, if we cannot trust the content length, we will close the connection
+ // unless we are going to use chunked encoding on HTTP/1.1 or the client issued a PUSH request
if (s->client_info.keep_alive != HTTP_KEEPALIVE) {
ka_action = KA_DISABLED;
- } else if (s->hdr_info.trust_response_cl == false && s->state_machine->ua_txn->is_chunked_encoding_supported() &&
+ } else if (s->state_machine->client_protocol && (strncmp(s->state_machine->client_protocol, "http/2", 6) == 0)) {
+ ka_action = KA_CONNECTION;
+ } else if (s->hdr_info.trust_response_cl == false &&
!(s->client_info.receive_chunked_response == true ||
(s->method == HTTP_WKSIDX_PUSH && s->client_info.keep_alive == HTTP_KEEPALIVE))) {
ka_action = KA_CLOSE;
diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc
index 8c9af05..2294dd5 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -567,7 +567,7 @@ void
Http2Stream::update_write_request(IOBufferReader *buf_reader, int64_t write_len, bool call_update)
{
if (!this->is_client_state_writeable() || closed || parent == nullptr || write_vio.mutex == nullptr ||
- (buf_reader == nullptr && write_len == 0)) {
+ (buf_reader == nullptr && write_len == 0) || this->response_reader == nullptr) {
return;
}
@@ -580,22 +580,7 @@ Http2Stream::update_write_request(IOBufferReader *buf_reader, int64_t write_len,
SCOPED_MUTEX_LOCK(lock, write_vio.mutex, this_ethread());
- // if response is chunked, limit the dechunked_buffer size.
- bool is_done = false;
- if (this->chunked) {
- if (chunked_handler.dechunked_buffer && chunked_handler.dechunked_buffer->max_read_avail() > HTTP2_MAX_BUFFER_USAGE) {
- if (buffer_full_write_event == nullptr) {
- buffer_full_write_event = _thread->schedule_imm(this, VC_EVENT_WRITE_READY);
- }
- } else {
- this->response_process_data(is_done);
- }
- }
-
- if (this->response_get_data_reader() == nullptr) {
- return;
- }
- int64_t bytes_avail = this->response_get_data_reader()->read_avail();
+ int64_t bytes_avail = this->response_reader->read_avail();
if (write_vio.nbytes > 0 && write_vio.ntodo() > 0) {
int64_t num_to_write = write_vio.ntodo();
if (num_to_write > write_len) {
@@ -610,7 +595,7 @@ Http2Stream::update_write_request(IOBufferReader *buf_reader, int64_t write_len,
", reader.read_avail=%" PRId64,
write_vio.nbytes, write_vio.ndone, write_vio.get_writer()->write_avail(), bytes_avail);
- if (bytes_avail <= 0 && !is_done) {
+ if (bytes_avail <= 0) {
return;
}
@@ -645,14 +630,7 @@ Http2Stream::update_write_request(IOBufferReader *buf_reader, int64_t write_len,
h2_proxy_ssn->connection_state.send_headers_frame(this);
}
- // See if the response is chunked. Set up the dechunking logic if it is
- // Make sure to check if the chunk is complete and signal appropriately
- this->response_initialize_data_handling(is_done);
-
- // If there is additional data, send it along in a data frame. Or if this was header only
- // make sure to send the end of stream
- is_done |= (write_vio.ntodo() + this->response_header.length_get()) == bytes_avail;
- if (this->response_is_data_available() || is_done) {
+ if (this->response_reader->is_read_avail_more_than(0)) {
this->_milestones.mark(Http2StreamMilestone::START_TX_DATA_FRAMES);
this->send_response_body(call_update);
}
@@ -806,7 +784,6 @@ Http2Stream::destroy()
if (header_blocks) {
ats_free(header_blocks);
}
- chunked_handler.clear();
clear_timers();
clear_io_events();
http_parser_clear(&http_parser);
@@ -815,53 +792,10 @@ Http2Stream::destroy()
THREAD_FREE(this, http2StreamAllocator, this_ethread());
}
-void
-Http2Stream::response_initialize_data_handling(bool &is_done)
-{
- is_done = false;
- int chunked_index = response_header.value_get_index(TS_MIME_FIELD_TRANSFER_ENCODING, TS_MIME_LEN_TRANSFER_ENCODING,
- TS_HTTP_VALUE_CHUNKED, TS_HTTP_LEN_CHUNKED);
- // -1 means this value was not found for this field
- if (chunked_index >= 0) {
- Http2StreamDebug("Response is chunked");
- chunked = true;
- this->chunked_handler.init_by_action(this->response_reader, ChunkedHandler::ACTION_DECHUNK);
- this->chunked_handler.state = ChunkedHandler::CHUNK_READ_SIZE;
- this->chunked_handler.dechunked_reader = this->chunked_handler.dechunked_buffer->alloc_reader();
- this->response_reader->dealloc();
- this->response_reader = nullptr;
- // Get things going if there is already data waiting
- if (this->chunked_handler.chunked_reader->is_read_avail_more_than(0)) {
- response_process_data(is_done);
- }
- }
-}
-
-void
-Http2Stream::response_process_data(bool &done)
-{
- done = false;
- if (chunked) {
- do {
- if (chunked_handler.state == ChunkedHandler::CHUNK_FLOW_CONTROL) {
- chunked_handler.state = ChunkedHandler::CHUNK_READ_SIZE_START;
- }
- done = this->chunked_handler.process_chunked_content();
- } while (chunked_handler.state == ChunkedHandler::CHUNK_FLOW_CONTROL);
- }
-}
-
-bool
-Http2Stream::response_is_data_available() const
-{
- IOBufferReader *reader = this->response_get_data_reader();
- return reader ? reader->is_read_avail_more_than(0) : false;
-}
-
IOBufferReader *
Http2Stream::response_get_data_reader() const
{
- return (chunked) ? chunked_handler.dechunked_reader : response_reader;
+ return this->response_reader;
}
void
diff --git a/proxy/http2/Http2Stream.h b/proxy/http2/Http2Stream.h
index 16d8790..1e176e9 100644
--- a/proxy/http2/Http2Stream.h
+++ b/proxy/http2/Http2Stream.h
@@ -24,9 +24,8 @@
#pragma once
#include "HTTP2.h"
-#include "../ProxyClientTransaction.h"
+#include "ProxyClientTransaction.h"
#include "Http2DebugNames.h"
-#include "../http/HttpTunnel.h" // To get ChunkedHandler
#include "Http2DependencyTree.h"
#include "tscore/History.h"
#include "Milestones.h"
@@ -104,7 +103,6 @@ public:
bool is_client_state_writeable() const;
bool is_closed() const;
- bool response_is_chunked() const;
IOBufferReader *response_get_data_reader() const;
void mark_milestone(Http2StreamMilestone type);
@@ -142,8 +140,6 @@ public:
Http2DependencyTree::Node *priority_node = nullptr;
private:
- void response_initialize_data_handling(bool &is_done);
- void response_process_data(bool &is_done);
bool response_is_data_available() const;
Event *send_tracked_event(Event *event, int send_event, VIO *vio);
void send_response_body(bool call_update);
@@ -169,7 +165,6 @@ private:
Milestones<Http2StreamMilestone, static_cast<size_t>(Http2StreamMilestone::LAST_ENTRY)> _milestones;
bool trailing_header = false;
- bool chunked = false;
// A brief disucssion of similar flags and state variables: _state, closed, terminate_stream
//
@@ -203,7 +198,6 @@ private:
std::vector<size_t> _recent_rwnd_increment = {SIZE_MAX, SIZE_MAX, SIZE_MAX, SIZE_MAX, SIZE_MAX};
int _recent_rwnd_increment_index = 0;
- ChunkedHandler chunked_handler;
Event *cross_thread_event = nullptr;
Event *buffer_full_write_event = nullptr;
@@ -302,12 +296,6 @@ Http2Stream::ignore_keep_alive()
}
inline bool
-Http2Stream::response_is_chunked() const
-{
- return chunked;
-}
-
-inline bool
Http2Stream::allow_half_open() const
{
return false;
[trafficserver] 09/25: Remove trailing white space from json
formatter
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 28bb79c5c2314a2d7c5ea04857eab378ba79a77c
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Fri Jan 31 08:20:40 2020 +0900
Remove trailing white space from json formatter
(cherry picked from commit 9836b0bbb4c36e2c1c70a57a7cfe66276ac9c0ba)
---
tests/gold_tests/h2/httpbin.test.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/gold_tests/h2/httpbin.test.py b/tests/gold_tests/h2/httpbin.test.py
index 851eee2..388df74 100644
--- a/tests/gold_tests/h2/httpbin.test.py
+++ b/tests/gold_tests/h2/httpbin.test.py
@@ -80,7 +80,7 @@ Test.Disk.File(os.path.join(ts.Variables.LOGDIR, 'access.log'), exists=True, con
# TODO: when httpbin 0.8.0 or later is released, remove below json pretty print hack
json_printer = '''
-python -c "import sys,json; print(json.dumps(json.load(sys.stdin), indent=2))"
+python -c "import sys,json; print(json.dumps(json.load(sys.stdin), indent=2, separators=(',', ': ')))"
'''
# ----
[trafficserver] 23/25: Cleanup trailing whitespaces
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit e5ab03b91379b5412c6e59cbc2499fb4c42cdab0
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Tue Mar 31 15:04:56 2020 +0900
Cleanup trailing whitespaces
---
doc/admin-guide/logging/formatting.en.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/doc/admin-guide/logging/formatting.en.rst b/doc/admin-guide/logging/formatting.en.rst
index 6f253df..2c512cc 100644
--- a/doc/admin-guide/logging/formatting.en.rst
+++ b/doc/admin-guide/logging/formatting.en.rst
@@ -206,9 +206,9 @@ ctid Client Request Client Transaction ID, a non-negative number for a transact
which is different for all currently-active transactions on the
same client connection. For client HTTP/2 transactions, this
value is the stream ID for the transaction.
-ctpw Client Request Client Transaction Priority Weight, the priority weight for the
+ctpw Client Request Client Transaction Priority Weight, the priority weight for the
underlying HTTP/2 protocol.
-ctpd Client Request Client Transaction Priority Dependence, the transaction ID that
+ctpd Client Request Client Transaction Priority Dependence, the transaction ID that
the current transaction depends on for HTTP/2 priority logic.
===== ============== ==================================================================
[trafficserver] 14/25: Track scheduled events to
(read|write)_vio.cont from Http2Stream
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 3c213fc8e270d9f343977600e4c4182d7afa86ff
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Fri Sep 13 09:16:33 2019 +0900
Track scheduled events to (read|write)_vio.cont from Http2Stream
(cherry picked from commit 9bd2fd6f0264bfe2176895531f8348fea20f84fa)
---
proxy/http2/Http2Stream.cc | 30 ++++++++++++++++++++++++++----
proxy/http2/Http2Stream.h | 6 ++++--
2 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc
index 2e11782..74d910f 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -103,14 +103,20 @@ Http2Stream::main_event_handler(int event, void *edata)
if (lock.is_locked()) {
read_vio.cont->handleEvent(event, &read_vio);
} else {
- this_ethread()->schedule_imm(read_vio.cont, event, &read_vio);
+ if (this->_read_vio_event) {
+ this->_read_vio_event->cancel();
+ }
+ this->_read_vio_event = this_ethread()->schedule_imm(read_vio.cont, event, &read_vio);
}
} else if (current_reader && write_vio.ntodo() > 0) {
MUTEX_TRY_LOCK(lock, write_vio.mutex, this_ethread());
if (lock.is_locked()) {
write_vio.cont->handleEvent(event, &write_vio);
} else {
- this_ethread()->schedule_imm(write_vio.cont, event, &write_vio);
+ if (this->_write_vio_event) {
+ this->_write_vio_event->cancel();
+ }
+ this->_write_vio_event = this_ethread()->schedule_imm(write_vio.cont, event, &write_vio);
}
}
break;
@@ -123,7 +129,10 @@ Http2Stream::main_event_handler(int event, void *edata)
if (lock.is_locked() && write_vio.cont && this->current_reader) {
write_vio.cont->handleEvent(event, &write_vio);
} else {
- this_ethread()->schedule_imm(write_vio.cont, event, &write_vio);
+ if (this->_write_vio_event) {
+ this->_write_vio_event->cancel();
+ }
+ this->_write_vio_event = this_ethread()->schedule_imm(write_vio.cont, event, &write_vio);
}
}
} else {
@@ -139,7 +148,10 @@ Http2Stream::main_event_handler(int event, void *edata)
if (lock.is_locked() && read_vio.cont && this->current_reader) {
read_vio.cont->handleEvent(event, &read_vio);
} else {
- this_ethread()->schedule_imm(read_vio.cont, event, &read_vio);
+ if (this->_read_vio_event) {
+ this->_read_vio_event->cancel();
+ }
+ this->_read_vio_event = this_ethread()->schedule_imm(read_vio.cont, event, &read_vio);
}
}
} else {
@@ -885,6 +897,16 @@ Http2Stream::clear_io_events()
buffer_full_write_event->cancel();
buffer_full_write_event = nullptr;
}
+
+ if (this->_read_vio_event) {
+ this->_read_vio_event->cancel();
+ this->_read_vio_event = nullptr;
+ }
+
+ if (this->_write_vio_event) {
+ this->_write_vio_event->cancel();
+ this->_write_vio_event = nullptr;
+ }
}
void
diff --git a/proxy/http2/Http2Stream.h b/proxy/http2/Http2Stream.h
index 4630190..261f44c 100644
--- a/proxy/http2/Http2Stream.h
+++ b/proxy/http2/Http2Stream.h
@@ -211,8 +211,10 @@ private:
ink_hrtime inactive_timeout_at = 0;
Event *inactive_event = nullptr;
- Event *read_event = nullptr;
- Event *write_event = nullptr;
+ Event *read_event = nullptr;
+ Event *write_event = nullptr;
+ Event *_read_vio_event = nullptr;
+ Event *_write_vio_event = nullptr;
};
extern ClassAllocator<Http2Stream> http2StreamAllocator;
[trafficserver] 02/25: Fix crash in MIMEScanner::append with
std::string. See #5587.
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 492108fde157a4e820ec73eb50d1f613191b861f
Author: Alan M. Carroll <am...@apache.org>
AuthorDate: Thu Jun 6 18:22:00 2019 -0500
Fix crash in MIMEScanner::append with std::string. See #5587.
(cherry picked from commit b7b6b4bc9a649822376ddeaeaaaf251baa4d0e2d)
Conflicts:
proxy/http2/Http2Stream.h
---
proxy/http2/Http2Stream.cc | 2 ++
1 file changed, 2 insertions(+)
diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc
index 8cd332e..8c9af05 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -60,6 +60,7 @@ Http2Stream::init(Http2StreamId sid, ssize_t initial_rwnd)
// FIXME: Are you sure? every "stream" needs request_header?
_req_header.create(HTTP_TYPE_REQUEST);
response_header.create(HTTP_TYPE_RESPONSE);
+ http_parser_init(&http_parser);
}
int
@@ -808,6 +809,7 @@ Http2Stream::destroy()
chunked_handler.clear();
clear_timers();
clear_io_events();
+ http_parser_clear(&http_parser);
super::destroy();
THREAD_FREE(this, http2StreamAllocator, this_ethread());
[trafficserver] 22/25: Remove update to unused variable
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 7b59b7dec8be0d8a140c0569ec1681a9769a503e
Author: Susan Hinrichs <sh...@verizonmedia.com>
AuthorDate: Mon Mar 2 21:24:21 2020 +0000
Remove update to unused variable
(cherry picked from commit deea85a25793bcab3a2428b9d7f0993248121c1c)
---
proxy/http2/Http2ConnectionState.cc | 1 -
1 file changed, 1 deletion(-)
diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc
index fa5437d..1a3872f 100644
--- a/proxy/http2/Http2ConnectionState.cc
+++ b/proxy/http2/Http2ConnectionState.cc
@@ -191,7 +191,6 @@ rcv_data_frame(Http2ConnectionState &cstate, const Http2Frame &frame)
myreader->writer()->dealloc_reader(myreader);
return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_STREAM, Http2ErrorCode::HTTP2_ERROR_INTERNAL_ERROR);
}
- nbytes += num_written;
myreader->consume(num_written);
}
myreader->writer()->dealloc_reader(myreader);
[trafficserver] 05/25: Log H2 priority information
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 8af30a85f5d8012559b5dab0334f1ef0a4908bf8
Author: Susan Hinrichs <sh...@oath.com>
AuthorDate: Mon Sep 23 17:28:27 2019 +0000
Log H2 priority information
(cherry picked from commit 89cf5afacc6eab8f24eed5ece5eeb2ca36ac5359)
Conflicts:
proxy/ProxyTransaction.cc
proxy/ProxyTransaction.h
proxy/http/HttpSM.h
proxy/http2/Http2Stream.cc
proxy/http2/Http2Stream.h
---
doc/admin-guide/logging/formatting.en.rst | 6 ++++++
proxy/ProxyClientTransaction.cc | 12 ++++++++++++
proxy/ProxyClientTransaction.h | 2 ++
proxy/http/HttpSM.cc | 4 +++-
proxy/http/HttpSM.h | 13 +++++++++++++
proxy/http2/Http2Stream.cc | 16 ++++++++++++++++
proxy/http2/Http2Stream.h | 2 ++
proxy/logging/Log.cc | 10 ++++++++++
proxy/logging/LogAccess.cc | 32 +++++++++++++++++++++++++++++++
proxy/logging/LogAccess.h | 20 ++++++++++---------
10 files changed, 107 insertions(+), 10 deletions(-)
diff --git a/doc/admin-guide/logging/formatting.en.rst b/doc/admin-guide/logging/formatting.en.rst
index 3f15da6..6f253df 100644
--- a/doc/admin-guide/logging/formatting.en.rst
+++ b/doc/admin-guide/logging/formatting.en.rst
@@ -185,6 +185,8 @@ Connections and Transactions
.. _sstc:
.. _ccid:
.. _ctid:
+.. _ctpw:
+.. _ctpd:
The following log fields are used to list various details of connections and
transactions between |TS| proxies and origin servers.
@@ -204,6 +206,10 @@ ctid Client Request Client Transaction ID, a non-negative number for a transact
which is different for all currently-active transactions on the
same client connection. For client HTTP/2 transactions, this
value is the stream ID for the transaction.
+ctpw Client Request Client Transaction Priority Weight, the priority weight for the
+ underlying HTTP/2 protocol.
+ctpd Client Request Client Transaction Priority Dependence, the transaction ID that
+ the current transaction depends on for HTTP/2 priority logic.
===== ============== ==================================================================
.. _admin-logging-fields-content-type:
diff --git a/proxy/ProxyClientTransaction.cc b/proxy/ProxyClientTransaction.cc
index 17c1c93..dcbdf5f 100644
--- a/proxy/ProxyClientTransaction.cc
+++ b/proxy/ProxyClientTransaction.cc
@@ -115,3 +115,15 @@ ProxyClientTransaction::set_tx_error_code(ProxyError e)
this->current_reader->t_state.client_info.tx_error_code = e;
}
}
+
+int
+ProxyClientTransaction::get_transaction_priority_weight() const
+{
+ return 0;
+}
+
+int
+ProxyClientTransaction::get_transaction_priority_dependence() const
+{
+ return 0;
+}
diff --git a/proxy/ProxyClientTransaction.h b/proxy/ProxyClientTransaction.h
index 12fcf30..560c5c6 100644
--- a/proxy/ProxyClientTransaction.h
+++ b/proxy/ProxyClientTransaction.h
@@ -235,6 +235,8 @@ public:
return current_reader;
}
+ virtual int get_transaction_priority_weight() const;
+ virtual int get_transaction_priority_dependence() const;
virtual bool allow_half_open() const = 0;
virtual const char *
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index e23ab53..b137470 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -432,7 +432,9 @@ HttpSM::attach_client_session(ProxyClientTransaction *client_vc, IOBufferReader
// It seems to be possible that the ua_txn pointer will go stale before log entries for this HTTP transaction are
// generated. Therefore, collect information that may be needed for logging from the ua_txn object at this point.
//
- _client_transaction_id = ua_txn->get_transaction_id();
+ _client_transaction_id = ua_txn->get_transaction_id();
+ _client_transaction_priority_weight = ua_txn->get_transaction_priority_weight();
+ _client_transaction_priority_dependence = ua_txn->get_transaction_priority_dependence();
{
auto p = ua_txn->get_parent();
diff --git a/proxy/http/HttpSM.h b/proxy/http/HttpSM.h
index b49e203..ebfdeba 100644
--- a/proxy/http/HttpSM.h
+++ b/proxy/http/HttpSM.h
@@ -617,9 +617,22 @@ public:
return _client_transaction_id;
}
+ int
+ client_transaction_priority_weight() const
+ {
+ return _client_transaction_priority_weight;
+ }
+
+ int
+ client_transaction_priority_dependence() const
+ {
+ return _client_transaction_priority_dependence;
+ }
+
private:
PostDataBuffers _postbuf;
int _client_connection_id = -1, _client_transaction_id = -1;
+ int _client_transaction_priority_weight = -1, _client_transaction_priority_dependence = -1;
};
// Function to get the cache_sm object - YTS Team, yamsat
diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc
index 2294dd5..6b33d3d 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -949,3 +949,19 @@ Http2Stream::_switch_thread_if_not_on_right_thread(int event, void *edata)
}
return true;
}
+
+int
+Http2Stream::get_transaction_priority_weight() const
+{
+ return priority_node ? priority_node->weight : 0;
+}
+
+int
+Http2Stream::get_transaction_priority_dependence() const
+{
+ if (!priority_node) {
+ return -1;
+ } else {
+ return priority_node->parent ? priority_node->parent->id : 0;
+ }
+}
diff --git a/proxy/http2/Http2Stream.h b/proxy/http2/Http2Stream.h
index 1e176e9..e93ae14 100644
--- a/proxy/http2/Http2Stream.h
+++ b/proxy/http2/Http2Stream.h
@@ -94,6 +94,8 @@ public:
bool allow_half_open() const override;
bool is_first_transaction() const override;
int get_transaction_id() const override;
+ int get_transaction_priority_weight() const override;
+ int get_transaction_priority_dependence() const override;
bool ignore_keep_alive() override;
void clear_inactive_timer();
diff --git a/proxy/logging/Log.cc b/proxy/logging/Log.cc
index 63b3419..b48afe5 100644
--- a/proxy/logging/Log.cc
+++ b/proxy/logging/Log.cc
@@ -869,6 +869,16 @@ Log::init_fields()
global_field_list.add(field, false);
ink_hash_table_insert(field_symbol_hash, "ctid", field);
+ field = new LogField("client_transaction_priority_weight", "ctpw", LogField::sINT,
+ &LogAccess::marshal_client_http_transaction_priority_weight, &LogAccess::unmarshal_int_to_str);
+ global_field_list.add(field, false);
+ ink_hash_table_insert(field_symbol_hash, "ctpw", field);
+
+ field = new LogField("client_transaction_priority_dependence", "ctpd", LogField::sINT,
+ &LogAccess::marshal_client_http_transaction_priority_dependence, &LogAccess::unmarshal_int_to_str);
+ global_field_list.add(field, false);
+ ink_hash_table_insert(field_symbol_hash, "ctpd", field);
+
init_status |= FIELDS_INITIALIZED;
}
diff --git a/proxy/logging/LogAccess.cc b/proxy/logging/LogAccess.cc
index 7f8388a..c51a634 100644
--- a/proxy/logging/LogAccess.cc
+++ b/proxy/logging/LogAccess.cc
@@ -2475,6 +2475,38 @@ LogAccess::marshal_client_http_transaction_id(char *buf)
-------------------------------------------------------------------------*/
int
+LogAccess::marshal_client_http_transaction_priority_weight(char *buf)
+{
+ if (buf) {
+ int64_t id = 0;
+ if (m_http_sm) {
+ id = m_http_sm->client_transaction_priority_weight();
+ }
+ marshal_int(buf, id);
+ }
+ return INK_MIN_ALIGN;
+}
+
+/*-------------------------------------------------------------------------
+ -------------------------------------------------------------------------*/
+
+int
+LogAccess::marshal_client_http_transaction_priority_dependence(char *buf)
+{
+ if (buf) {
+ int64_t id = 0;
+ if (m_http_sm) {
+ id = m_http_sm->client_transaction_priority_dependence();
+ }
+ marshal_int(buf, id);
+ }
+ return INK_MIN_ALIGN;
+}
+
+/*-------------------------------------------------------------------------
+ -------------------------------------------------------------------------*/
+
+int
LogAccess::marshal_http_header_field(LogField::Container container, char *field, char *buf)
{
char *str = nullptr;
diff --git a/proxy/logging/LogAccess.h b/proxy/logging/LogAccess.h
index a4a721a..860c089 100644
--- a/proxy/logging/LogAccess.h
+++ b/proxy/logging/LogAccess.h
@@ -232,15 +232,17 @@ public:
// other fields
//
- inkcoreapi int marshal_transfer_time_ms(char *); // INT
- inkcoreapi int marshal_transfer_time_s(char *); // INT
- inkcoreapi int marshal_file_size(char *); // INT
- inkcoreapi int marshal_plugin_identity_id(char *); // INT
- inkcoreapi int marshal_plugin_identity_tag(char *); // STR
- inkcoreapi int marshal_process_uuid(char *); // STR
- inkcoreapi int marshal_client_http_connection_id(char *); // INT
- inkcoreapi int marshal_client_http_transaction_id(char *); // INT
- inkcoreapi int marshal_cache_lookup_url_canon(char *); // STR
+ inkcoreapi int marshal_transfer_time_ms(char *); // INT
+ inkcoreapi int marshal_transfer_time_s(char *); // INT
+ inkcoreapi int marshal_file_size(char *); // INT
+ inkcoreapi int marshal_plugin_identity_id(char *); // INT
+ inkcoreapi int marshal_plugin_identity_tag(char *); // STR
+ inkcoreapi int marshal_process_uuid(char *); // STR
+ inkcoreapi int marshal_client_http_connection_id(char *); // INT
+ inkcoreapi int marshal_client_http_transaction_id(char *); // INT
+ inkcoreapi int marshal_client_http_transaction_priority_weight(char *); // INT
+ inkcoreapi int marshal_client_http_transaction_priority_dependence(char *); // INT
+ inkcoreapi int marshal_cache_lookup_url_canon(char *); // STR
// named fields from within a http header
//
[trafficserver] 08/25: Set END_STREAM flag when write_vio ntodo is 0
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 273aea8d9be782d25103cbda63ba86e753825991
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Tue Nov 19 10:10:16 2019 +0900
Set END_STREAM flag when write_vio ntodo is 0
(cherry picked from commit 52538c4fe966b5a67f6701a919f7e07fade164a2)
Conflicts:
proxy/http2/Http2ConnectionState.cc
---
proxy/http2/Http2ConnectionState.cc | 12 +++++++-----
proxy/http2/Http2Stream.h | 4 ++--
tests/gold_tests/h2/gold/http2_9_stderr.gold | 10 ++++++++++
tests/gold_tests/h2/gold/http2_9_stdout.gold | 0
tests/gold_tests/h2/http2.test.py | 13 +++++++++++++
5 files changed, 32 insertions(+), 7 deletions(-)
diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc
index 7588b75..3e4056b 100644
--- a/proxy/http2/Http2ConnectionState.cc
+++ b/proxy/http2/Http2ConnectionState.cc
@@ -1415,7 +1415,7 @@ Http2ConnectionState::send_data_frames_depends_on_priority()
switch (result) {
case Http2SendDataFrameResult::NO_ERROR: {
// No response body to send
- if (len == 0 && !stream->is_body_done()) {
+ if (len == 0 && !stream->is_write_vio_done()) {
dependency_tree->deactivate(node, len);
} else {
dependency_tree->update(node, len);
@@ -1476,12 +1476,12 @@ Http2ConnectionState::send_a_data_frame(Http2Stream *stream, size_t &payload_len
// Are we at the end?
// If we return here, we never send the END_STREAM in the case of a early terminating OS.
// OK if there is no body yet. Otherwise continue on to send a DATA frame and delete the stream
- if (!stream->is_body_done() && payload_length == 0) {
+ if (!stream->is_write_vio_done() && payload_length == 0) {
Http2StreamDebug(this->ua_session, stream->get_id(), "No payload");
return Http2SendDataFrameResult::NO_PAYLOAD;
}
- if (stream->is_body_done() && !current_reader->is_read_avail_more_than(0)) {
+ if (stream->is_write_vio_done() && !current_reader->is_read_avail_more_than(0)) {
flags |= HTTP2_FLAGS_DATA_END_STREAM;
}
@@ -1505,7 +1505,7 @@ Http2ConnectionState::send_a_data_frame(Http2Stream *stream, size_t &payload_len
this->ua_session->handleEvent(HTTP2_SESSION_EVENT_XMIT, &data);
if (flags & HTTP2_FLAGS_DATA_END_STREAM) {
- Http2StreamDebug(ua_session, stream->get_id(), "End of DATA frame");
+ Http2StreamDebug(ua_session, stream->get_id(), "END_STREAM");
stream->send_end_stream = true;
// Setting to the same state shouldn't be erroneous
stream->change_state(data.header().type, data.header().flags);
@@ -1583,7 +1583,9 @@ Http2ConnectionState::send_headers_frame(Http2Stream *stream)
if (header_blocks_size <= static_cast<uint32_t>(BUFFER_SIZE_FOR_INDEX(buffer_size_index[HTTP2_FRAME_TYPE_HEADERS]))) {
payload_length = header_blocks_size;
flags |= HTTP2_FLAGS_HEADERS_END_HEADERS;
- if (h2_hdr.presence(MIME_PRESENCE_CONTENT_LENGTH) && h2_hdr.get_content_length() == 0) {
+ if ((h2_hdr.presence(MIME_PRESENCE_CONTENT_LENGTH) && h2_hdr.get_content_length() == 0) ||
+ (!resp_header->expect_final_response() && stream->is_write_vio_done())) {
+ Http2StreamDebug(ua_session, stream->get_id(), "END_STREAM");
flags |= HTTP2_FLAGS_HEADERS_END_STREAM;
stream->send_end_stream = true;
}
diff --git a/proxy/http2/Http2Stream.h b/proxy/http2/Http2Stream.h
index e93ae14..4630190 100644
--- a/proxy/http2/Http2Stream.h
+++ b/proxy/http2/Http2Stream.h
@@ -111,7 +111,7 @@ public:
void increment_data_length(uint64_t length);
bool payload_length_is_valid() const;
- bool is_body_done() const;
+ bool is_write_vio_done() const;
void update_sent_count(unsigned num_bytes);
Http2StreamId get_id() const;
Http2StreamState get_state() const;
@@ -227,7 +227,7 @@ Http2Stream::mark_milestone(Http2StreamMilestone type)
}
inline bool
-Http2Stream::is_body_done() const
+Http2Stream::is_write_vio_done() const
{
return this->write_vio.ntodo() == 0;
}
diff --git a/tests/gold_tests/h2/gold/http2_9_stderr.gold b/tests/gold_tests/h2/gold/http2_9_stderr.gold
new file mode 100644
index 0000000..ea2f129
--- /dev/null
+++ b/tests/gold_tests/h2/gold/http2_9_stderr.gold
@@ -0,0 +1,10 @@
+``
+> GET /status/204 HTTP/2
+> Host: ``
+> User-Agent: curl/``
+> Accept: */*
+``
+< HTTP/2 204``
+< server: ATS/``
+< date: ``
+``
diff --git a/tests/gold_tests/h2/gold/http2_9_stdout.gold b/tests/gold_tests/h2/gold/http2_9_stdout.gold
new file mode 100644
index 0000000..e69de29
diff --git a/tests/gold_tests/h2/http2.test.py b/tests/gold_tests/h2/http2.test.py
index 4a4fc29..fd139a3 100644
--- a/tests/gold_tests/h2/http2.test.py
+++ b/tests/gold_tests/h2/http2.test.py
@@ -70,6 +70,11 @@ server.addResponse("sessionlog.json",
{"headers": "GET /huge_resp_hdrs HTTP/1.1\r\nHost: www.example.com\r\n\r\n", "timestamp": "1469733493.993", "body": ""},
{"headers": "HTTP/1.1 200 OK\r\nServer: microserver\r\nConnection: close\r\nContent-Length: 6\r\n\r\n", "timestamp": "1469733493.993", "body": "200 OK"})
+# For Test Case 9 - /status/204
+server.addResponse("sessionlog.json",
+ {"headers": "GET /status/204 HTTP/1.1\r\nHost: www.example.com\r\n\r\n", "timestamp": "1469733493.993", "body": ""},
+ {"headers": "HTTP/1.1 204 No Content\r\nServer: microserver\r\nConnection: close\r\n\r\n", "timestamp": "1469733493.993", "body": ""})
+
# ----
# Setup ATS
# ----
@@ -181,3 +186,11 @@ tr.Processes.Default.ReturnCode = 0
tr.Processes.Default.Streams.stdout = "gold/http2_8_stdout.gold"
tr.Processes.Default.Streams.stderr = "gold/http2_8_stderr.gold"
tr.StillRunningAfter = server
+
+# Test Case 9: Header Only Response - e.g. 204
+tr = Test.AddTestRun()
+tr.Processes.Default.Command = 'curl -vs -k --http2 https://127.0.0.1:{0}/status/204'.format(ts.Variables.ssl_port)
+tr.Processes.Default.ReturnCode = 0
+tr.Processes.Default.Streams.stdout = "gold/http2_9_stdout.gold"
+tr.Processes.Default.Streams.stderr = "gold/http2_9_stderr.gold"
+tr.StillRunningAfter = server
[trafficserver] 03/25: Cleanup: VIO
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 992914b11adfd206c29804e210d5b7a84966a2f6
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Tue Sep 10 08:53:54 2019 +0900
Cleanup: VIO
- No logical changes
- Remove unnecessary include headers
- Fix format
- Make const functions
(cherry picked from commit dc0fb99bab960026ba842210ec16fab4f4701e65)
Conflicts:
iocore/eventsystem/I_VIO.h
---
iocore/eventsystem/I_IOBuffer.h | 4 ++--
iocore/eventsystem/I_VIO.h | 26 +++++++++-----------------
iocore/eventsystem/P_VIO.h | 34 ++++++++++------------------------
3 files changed, 21 insertions(+), 43 deletions(-)
diff --git a/iocore/eventsystem/I_IOBuffer.h b/iocore/eventsystem/I_IOBuffer.h
index ac652e3..852ccee 100644
--- a/iocore/eventsystem/I_IOBuffer.h
+++ b/iocore/eventsystem/I_IOBuffer.h
@@ -1184,13 +1184,13 @@ public:
*/
struct MIOBufferAccessor {
IOBufferReader *
- reader()
+ reader() const
{
return entry;
}
MIOBuffer *
- writer()
+ writer() const
{
return mbuf;
}
diff --git a/iocore/eventsystem/I_VIO.h b/iocore/eventsystem/I_VIO.h
index eacbc35..69f4890 100644
--- a/iocore/eventsystem/I_VIO.h
+++ b/iocore/eventsystem/I_VIO.h
@@ -25,17 +25,12 @@
#pragma once
#define I_VIO_h
-#include "tscore/ink_platform.h"
-#include "I_EventSystem.h"
#if !defined(I_IOBuffer_h)
#error "include I_IOBuffer.h"
----include I_IOBuffer.h
#endif
-#include "tscore/ink_apidefs.h"
- class Continuation;
+
+class Continuation;
class VConnection;
-class IOVConnection;
-class MIOBuffer;
class ProxyMutex;
/**
@@ -73,9 +68,12 @@ class ProxyMutex;
class VIO
{
public:
+ explicit VIO(int aop);
+ VIO();
~VIO() {}
+
/** Interface for the VConnection that owns this handle. */
- Continuation *get_continuation();
+ Continuation *get_continuation() const;
void set_continuation(Continuation *cont);
/**
@@ -102,8 +100,8 @@ public:
/////////////////////
void set_writer(MIOBuffer *writer);
void set_reader(IOBufferReader *reader);
- MIOBuffer *get_writer();
- IOBufferReader *get_reader();
+ MIOBuffer *get_writer() const;
+ IOBufferReader *get_reader() const;
/**
Reenable the IO operation.
@@ -140,10 +138,7 @@ public:
inkcoreapi void reenable_re();
void disable();
- bool is_disabled();
-
- VIO(int aop);
- VIO();
+ bool is_disabled() const;
enum {
NONE = 0,
@@ -160,7 +155,6 @@ public:
STAT,
};
-public:
/**
Continuation to callback.
@@ -225,5 +219,3 @@ public:
private:
bool _disabled = false;
};
-
-#include "I_VConnection.h"
diff --git a/iocore/eventsystem/P_VIO.h b/iocore/eventsystem/P_VIO.h
index 5c0d057..b09a190 100644
--- a/iocore/eventsystem/P_VIO.h
+++ b/iocore/eventsystem/P_VIO.h
@@ -27,44 +27,45 @@
TS_INLINE
VIO::VIO(int aop) : cont(nullptr), nbytes(0), ndone(0), op(aop), buffer(), vc_server(nullptr), mutex(nullptr) {}
-/////////////////////////////////////////////////////////////
-//
-// VIO::VIO()
-//
-/////////////////////////////////////////////////////////////
TS_INLINE
VIO::VIO() : cont(nullptr), nbytes(0), ndone(0), op(VIO::NONE), buffer(), vc_server(nullptr), mutex(nullptr) {}
TS_INLINE Continuation *
-VIO::get_continuation()
+VIO::get_continuation() const
{
return cont;
}
+
TS_INLINE void
VIO::set_writer(MIOBuffer *writer)
{
buffer.writer_for(writer);
}
+
TS_INLINE void
VIO::set_reader(IOBufferReader *reader)
{
buffer.reader_for(reader);
}
+
TS_INLINE MIOBuffer *
-VIO::get_writer()
+VIO::get_writer() const
{
return buffer.writer();
}
+
TS_INLINE IOBufferReader *
-VIO::get_reader()
+VIO::get_reader() const
{
return (buffer.reader());
}
+
TS_INLINE int64_t
VIO::ntodo() const
{
return nbytes - ndone;
}
+
TS_INLINE void
VIO::done()
{
@@ -75,11 +76,6 @@ VIO::done()
}
}
-/////////////////////////////////////////////////////////////
-//
-// VIO::set_continuation()
-//
-/////////////////////////////////////////////////////////////
TS_INLINE void
VIO::set_continuation(Continuation *acont)
{
@@ -96,11 +92,6 @@ VIO::set_continuation(Continuation *acont)
return;
}
-/////////////////////////////////////////////////////////////
-//
-// VIO::reenable()
-//
-/////////////////////////////////////////////////////////////
TS_INLINE void
VIO::reenable()
{
@@ -110,11 +101,6 @@ VIO::reenable()
}
}
-/////////////////////////////////////////////////////////////
-//
-// VIO::reenable_re()
-//
-/////////////////////////////////////////////////////////////
TS_INLINE void
VIO::reenable_re()
{
@@ -131,7 +117,7 @@ VIO::disable()
}
TS_INLINE bool
-VIO::is_disabled()
+VIO::is_disabled() const
{
return this->_disabled;
}
[trafficserver] 06/25: Add 100-continue expectation support on H2
connection
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 8754223ceeb6db022230398bd10dcf2ccc0f9546
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Wed Nov 20 14:05:54 2019 +0900
Add 100-continue expectation support on H2 connection
(cherry picked from commit c0fe598af5ad41a73c44d8f10fc5bae6baa555f7)
Conflicts:
tests/gold_tests/h2/gold/httpbin_0_stdout.gold
tests/gold_tests/h2/httpbin.test.py
---
proxy/hdrs/HTTP.h | 23 +++++++++++++++++++++--
proxy/http2/Http2Stream.cc | 11 +++++++++++
tests/gold_tests/h2/gold/httpbin_0_stdout.gold | 2 +-
tests/gold_tests/h2/gold/httpbin_3_stderr.gold | 9 +++++++++
tests/gold_tests/h2/gold/httpbin_3_stdout.gold | 7 +++++++
tests/gold_tests/h2/gold/httpbin_access.gold | 1 +
tests/gold_tests/h2/httpbin.test.py | 18 +++++++++++++++++-
7 files changed, 67 insertions(+), 4 deletions(-)
diff --git a/proxy/hdrs/HTTP.h b/proxy/hdrs/HTTP.h
index 7d91765..6867d37 100644
--- a/proxy/hdrs/HTTP.h
+++ b/proxy/hdrs/HTTP.h
@@ -618,7 +618,7 @@ public:
/// header internals, they must be able to do this.
void mark_target_dirty() const;
- HTTPStatus status_get();
+ HTTPStatus status_get() const;
void status_set(HTTPStatus status);
const char *reason_get(int *length);
@@ -635,6 +635,7 @@ public:
bool is_cache_control_set(const char *cc_directive_wks);
bool is_pragma_no_cache_set();
bool is_keep_alive_set() const;
+ bool expect_final_response() const;
HTTPKeepAlive keep_alive_get() const;
protected:
@@ -1004,6 +1005,24 @@ HTTPHdr::is_keep_alive_set() const
return this->keep_alive_get() == HTTP_KEEPALIVE;
}
+/**
+ Check the status code is informational and expecting final response
+ - e.g. "100 Continue", "103 Early Hints"
+
+ Please note that "101 Switching Protocol" is not included.
+ */
+inline bool
+HTTPHdr::expect_final_response() const
+{
+ switch (this->status_get()) {
+ case HTTP_STATUS_CONTINUE:
+ case HTTP_STATUS_EARLY_HINTS:
+ return true;
+ default:
+ return false;
+ }
+}
+
/*-------------------------------------------------------------------------
-------------------------------------------------------------------------*/
@@ -1154,7 +1173,7 @@ http_hdr_status_get(HTTPHdrImpl *hh)
-------------------------------------------------------------------------*/
inline HTTPStatus
-HTTPHdr::status_get()
+HTTPHdr::status_get() const
{
ink_assert(valid());
diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc
index 6b33d3d..1a2c242 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -630,6 +630,17 @@ Http2Stream::update_write_request(IOBufferReader *buf_reader, int64_t write_len,
h2_proxy_ssn->connection_state.send_headers_frame(this);
}
+ // Roll back states of response header to read final response
+ if (this->response_header.expect_final_response()) {
+ this->response_header_done = false;
+ response_header.destroy();
+ response_header.create(HTTP_TYPE_RESPONSE);
+ http_parser_clear(&http_parser);
+ http_parser_init(&http_parser);
+ }
+
+ this->signal_write_event(call_update);
+
if (this->response_reader->is_read_avail_more_than(0)) {
this->_milestones.mark(Http2StreamMilestone::START_TX_DATA_FRAMES);
this->send_response_body(call_update);
diff --git a/tests/gold_tests/h2/gold/httpbin_0_stdout.gold b/tests/gold_tests/h2/gold/httpbin_0_stdout.gold
index cb7b2d6..d8e6107 100644
--- a/tests/gold_tests/h2/gold/httpbin_0_stdout.gold
+++ b/tests/gold_tests/h2/gold/httpbin_0_stdout.gold
@@ -1,5 +1,5 @@
{
``
- "url":"http://``/get"
+ "url": "http://``/get"
``
}
diff --git a/tests/gold_tests/h2/gold/httpbin_3_stderr.gold b/tests/gold_tests/h2/gold/httpbin_3_stderr.gold
new file mode 100644
index 0000000..8109f9d
--- /dev/null
+++ b/tests/gold_tests/h2/gold/httpbin_3_stderr.gold
@@ -0,0 +1,9 @@
+``
+> POST /post HTTP/2
+``
+> Expect: 100-continue
+``
+< HTTP/2 100``
+``
+< HTTP/2 200``
+``
diff --git a/tests/gold_tests/h2/gold/httpbin_3_stdout.gold b/tests/gold_tests/h2/gold/httpbin_3_stdout.gold
new file mode 100644
index 0000000..6105808
--- /dev/null
+++ b/tests/gold_tests/h2/gold/httpbin_3_stdout.gold
@@ -0,0 +1,7 @@
+{
+ ``
+ "form": {
+ "key": "value"
+ },
+ ``
+}
diff --git a/tests/gold_tests/h2/gold/httpbin_access.gold b/tests/gold_tests/h2/gold/httpbin_access.gold
index 3f77947..d409c47 100644
--- a/tests/gold_tests/h2/gold/httpbin_access.gold
+++ b/tests/gold_tests/h2/gold/httpbin_access.gold
@@ -1,3 +1,4 @@
[``] GET http://127.0.0.1:``/get HTTP/1.1 http/2 `` `` TCP_MISS 200 ``
[``] GET http://127.0.0.1:``/bytes/0 HTTP/1.1 http/2 `` `` TCP_MISS 200 0
[``] GET http://127.0.0.1:``/stream-bytes/102400?seed=0 HTTP/1.1 http/2 `` `` TCP_MISS 200 102400
+``
diff --git a/tests/gold_tests/h2/httpbin.test.py b/tests/gold_tests/h2/httpbin.test.py
index f30eea4..851eee2 100644
--- a/tests/gold_tests/h2/httpbin.test.py
+++ b/tests/gold_tests/h2/httpbin.test.py
@@ -78,13 +78,19 @@ logs:
Test.Disk.File(os.path.join(ts.Variables.LOGDIR, 'access.log'), exists=True, content='gold/httpbin_access.gold')
+# TODO: when httpbin 0.8.0 or later is released, remove below json pretty print hack
+json_printer = '''
+python -c "import sys,json; print(json.dumps(json.load(sys.stdin), indent=2))"
+'''
+
# ----
# Test Cases
# ----
# Test Case 0: Basic request and resposne
test_run = Test.AddTestRun()
-test_run.Processes.Default.Command = 'curl -vs -k --http2 https://127.0.0.1:{0}/get'.format(ts.Variables.ssl_port)
+test_run.Processes.Default.Command = "curl -vs -k --http2 https://127.0.0.1:{0}/get | {1}".format(
+ ts.Variables.ssl_port, json_printer)
test_run.Processes.Default.ReturnCode = 0
test_run.Processes.Default.StartBefore(httpbin, ready=When.PortOpen(httpbin.Variables.Port))
test_run.Processes.Default.StartBefore(Test.Processes.ts, ready=When.PortOpen(ts.Variables.ssl_port))
@@ -109,6 +115,16 @@ test_run.Processes.Default.Streams.stdout = "gold/httpbin_2_stdout.gold"
test_run.Processes.Default.Streams.stderr = "gold/httpbin_2_stderr.gold"
test_run.StillRunningAfter = httpbin
+# Test Case 3: Expect 100-Continue
+test_run = Test.AddTestRun()
+test_run.Processes.Default.Command = "curl -vs -k --http2 https://127.0.0.1:{0}/post --data 'key=value' -H 'Expect: 100-continue' --expect100-timeout 1 --max-time 5 | {1}".format(
+ ts.Variables.ssl_port, json_printer)
+test_run.Processes.Default.ReturnCode = 0
+test_run.Processes.Default.Streams.stdout = "gold/httpbin_3_stdout.gold"
+test_run.Processes.Default.Streams.stderr = "gold/httpbin_3_stderr.gold"
+test_run.StillRunningAfter = httpbin
+
+
# Check Logging
test_run = Test.AddTestRun()
test_run.DelayStart = 10
[trafficserver] 13/25: Fix heap-use-after-free on
Http2Stream::destroy()
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 8.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit b37bfde7c2b2903358684ecc379453c33673dab4
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Mon Jan 27 13:36:01 2020 +0900
Fix heap-use-after-free on Http2Stream::destroy()
(cherry picked from commit ca6747102646080fe03046384488150ce93143d9)
Conflicts:
proxy/http2/Http2Stream.cc
---
proxy/http2/Http2Stream.cc | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc
index 5f9d6fe..2e11782 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -743,15 +743,18 @@ Http2Stream::destroy()
// Safe to initiate SSN_CLOSE if this is the last stream
if (parent) {
- Http2ClientSession *h2_parent = static_cast<Http2ClientSession *>(parent);
- SCOPED_MUTEX_LOCK(lock, h2_parent->connection_state.mutex, this_ethread());
+ cid = _proxy_ssn->connection_id();
+
+ Http2ClientSession *h2_proxy_ssn = static_cast<Http2ClientSession *>(parent);
+ SCOPED_MUTEX_LOCK(lock, h2_proxy_ssn->connection_state.mutex, this_ethread());
// Make sure the stream is removed from the stream list and priority tree
// In many cases, this has been called earlier, so this call is a no-op
- h2_parent->connection_state.delete_stream(this);
+ h2_proxy_ssn->connection_state.delete_stream(this);
// Update session's stream counts, so it accurately goes into keep-alive state
- h2_parent->connection_state.release_stream(this);
- cid = parent->connection_id();
+ h2_proxy_ssn->connection_state.release_stream(this);
+
+ // Do not access `_proxy_ssn` in below. It might be freed by `release_stream`.
}
// Clean up the write VIO in case of inactivity timeout