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 2018/03/06 18:16:18 UTC
[trafficserver] branch 7.1.x updated (f5d1ed6 -> 912c446)
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a change to branch 7.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git.
from f5d1ed6 Acquire lock before calling Http2ClientSession::free() from Http2ConnectionState
new c66cd89 Combine mutex of Http2ClientSession and Http2ConnectionState
new dd51074 Prevent potential dispatch of HttpTunnel events to a dead consumer.
new 2616e58 Respond with 400 code when Content-Length headers mismatch, remove duplicate copies of the Content-Length header with exactly same values, and remove Content-Length headers if Transfer-Encoding header exists.
new 912c446 Select the current cert when looking for issuer
The 4 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:
iocore/net/OCSPStapling.cc | 7 ++++
proxy/hdrs/HTTP.cc | 60 +++++++++++++++++++++++++++++++--
proxy/hdrs/HTTP.h | 1 +
proxy/http/HttpTunnel.h | 30 ++++++++++++++---
proxy/http2/Http2ClientSession.cc | 2 +-
tests/gold_tests/headers/syntax.test.py | 31 ++++++++++++++---
6 files changed, 118 insertions(+), 13 deletions(-)
--
To stop receiving notification emails like this one, please contact
zwoop@apache.org.
[trafficserver] 03/04: Respond with 400 code when Content-Length
headers mismatch,
remove duplicate copies of the Content-Length header with exactly same
values,
and remove Content-Length headers if Transfer-Encoding header exists.
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 7.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 2616e580de7d66b9098c464d503a049c7814e35a
Author: Bryan Call <bc...@apache.org>
AuthorDate: Mon Mar 5 16:21:18 2018 -0800
Respond with 400 code when Content-Length headers mismatch, remove
duplicate copies of the Content-Length header with exactly same values,
and remove Content-Length headers if Transfer-Encoding header exists.
(cherry picked from commit 44af6e5ba6d323354da0dd00f35aa7920d477805)
---
proxy/hdrs/HTTP.cc | 60 +++++++++++++++++++++++++++++++--
proxy/hdrs/HTTP.h | 1 +
tests/gold_tests/headers/syntax.test.py | 31 ++++++++++++++---
3 files changed, 85 insertions(+), 7 deletions(-)
diff --git a/proxy/hdrs/HTTP.cc b/proxy/hdrs/HTTP.cc
index c265e12..cb32ed7 100644
--- a/proxy/hdrs/HTTP.cc
+++ b/proxy/hdrs/HTTP.cc
@@ -967,8 +967,12 @@ http_parser_parse_req(HTTPParser *parser, HdrHeap *heap, HTTPHdrImpl *hh, const
}
ParseResult ret = mime_parser_parse(&parser->m_mime_parser, heap, hh->m_fields_impl, start, end, must_copy_strings, eof);
+ // If we're done with the main parse do some validation
if (ret == PARSE_RESULT_DONE) {
- ret = validate_hdr_host(hh); // if we're done with the main parse, check HOST.
+ ret = validate_hdr_host(hh); // check HOST header
+ }
+ if (ret == PARSE_RESULT_DONE) {
+ ret = validate_hdr_content_length(heap, hh);
}
return ret;
}
@@ -1118,8 +1122,12 @@ http_parser_parse_req(HTTPParser *parser, HdrHeap *heap, HTTPHdrImpl *hh, const
parser->m_parsing_http = false;
ParseResult ret = mime_parser_parse(&parser->m_mime_parser, heap, hh->m_fields_impl, start, end, must_copy_strings, eof);
+ // If we're done with the main parse do some validation
if (ret == PARSE_RESULT_DONE) {
- ret = validate_hdr_host(hh); // if we're done with the main parse, check HOST.
+ ret = validate_hdr_host(hh); // check HOST header
+ }
+ if (ret == PARSE_RESULT_DONE) {
+ ret = validate_hdr_content_length(heap, hh);
}
return ret;
}
@@ -1166,6 +1174,54 @@ validate_hdr_host(HTTPHdrImpl *hh)
return ret;
}
+ParseResult
+validate_hdr_content_length(HdrHeap *heap, HTTPHdrImpl *hh)
+{
+ MIMEField *content_length_field = mime_hdr_field_find(hh->m_fields_impl, MIME_FIELD_CONTENT_LENGTH, MIME_LEN_CONTENT_LENGTH);
+
+ if (content_length_field) {
+ // RFC 7230 section 3.3.3:
+ // If a message is received with both a Transfer-Encoding and a
+ // Content-Length header field, the Transfer-Encoding overrides
+ // the Content-Length
+ if (mime_hdr_field_find(hh->m_fields_impl, MIME_FIELD_TRANSFER_ENCODING, MIME_LEN_TRANSFER_ENCODING) != nullptr) {
+ // Delete all Content-Length headers
+ Debug("http", "Transfer-Encoding header and Content-Length headers the request, removing all Content-Length headers");
+ mime_hdr_field_delete(heap, hh->m_fields_impl, content_length_field);
+ return PARSE_RESULT_DONE;
+ }
+
+ // RFC 7230 section 3.3.3:
+ // If a message is received without Transfer-Encoding and with
+ // either multiple Content-Length header fields having differing
+ // field-values or a single Content-Length header field having an
+ // invalid value, then the message framing is invalid and the
+ // recipient MUST treat it as an unrecoverable error. If this is a
+ // request message, the server MUST respond with a 400 (Bad Request)
+ // status code and then close the connection
+ int content_length_len = 0;
+ const char *content_length_val = content_length_field->value_get(&content_length_len);
+
+ while (content_length_field->has_dups()) {
+ int content_length_len_2 = 0;
+ const char *content_length_val_2 = content_length_field->m_next_dup->value_get(&content_length_len_2);
+
+ if ((content_length_len != content_length_len_2) ||
+ (memcmp(content_length_val, content_length_val_2, content_length_len) != 0)) {
+ // Values are different, parse error
+ Debug("http", "Content-Length headers don't match, returning parse error");
+ return PARSE_RESULT_ERROR;
+ } else {
+ // Delete the duplicate since it has the same value
+ Debug("http", "Deleting duplicate Content-Length header");
+ mime_hdr_field_delete(heap, hh->m_fields_impl, content_length_field->m_next_dup, false);
+ }
+ }
+ }
+
+ return PARSE_RESULT_DONE;
+}
+
/*-------------------------------------------------------------------------
-------------------------------------------------------------------------*/
diff --git a/proxy/hdrs/HTTP.h b/proxy/hdrs/HTTP.h
index 3cd61a3..ff50e1b 100644
--- a/proxy/hdrs/HTTP.h
+++ b/proxy/hdrs/HTTP.h
@@ -445,6 +445,7 @@ void http_parser_clear(HTTPParser *parser);
ParseResult http_parser_parse_req(HTTPParser *parser, HdrHeap *heap, HTTPHdrImpl *hh, const char **start, const char *end,
bool must_copy_strings, bool eof, bool strict_uri_parsing);
ParseResult validate_hdr_host(HTTPHdrImpl *hh);
+ParseResult validate_hdr_content_length(HdrHeap *heap, HTTPHdrImpl *hh);
ParseResult http_parser_parse_resp(HTTPParser *parser, HdrHeap *heap, HTTPHdrImpl *hh, const char **start, const char *end,
bool must_copy_strings, bool eof);
diff --git a/tests/gold_tests/headers/syntax.test.py b/tests/gold_tests/headers/syntax.test.py
index e38b97e..2938304 100644
--- a/tests/gold_tests/headers/syntax.test.py
+++ b/tests/gold_tests/headers/syntax.test.py
@@ -64,21 +64,42 @@ tr = Test.AddTestRun()
tr.Processes.Default.StartBefore(server)
tr.Processes.Default.StartBefore(Test.Processes.ts)
tr.Processes.Default.StartBefore(Test.Processes.ts, ready=When.PortOpen(ts.Variables.ssl_port))
-tr.Processes.Default.Command = 'curl -s -D - -v --ipv4 --http1.1 -H " foo: bar" -H "Host: www.example.com" http://localhost:8080/'
+tr.Processes.Default.Command = 'curl -s -D - -v --ipv4 --http1.1 -H " foo: bar" -H "Host: www.example.com" http://localhost:8080/'
tr.Processes.Default.ReturnCode = 0
tr.Processes.Default.Streams.stdout = "syntax.200.gold"
tr.StillRunningAfter = ts
-# Test 2 - 400 Response
+# Test 2 - 400 Response - Single space after field name
tr = Test.AddTestRun()
-tr.Processes.Default.Command = 'curl -s -D - -v --ipv4 --http1.1 -H "foo : bar" -H "Host: www.example.com" http://localhost:8080/'
+tr.Processes.Default.Command = 'curl -s -D - -v --ipv4 --http1.1 -H "foo : bar" -H "Host: www.example.com" http://localhost:8080/'
tr.Processes.Default.ReturnCode = 0
tr.Processes.Default.Streams.stdout = "syntax.400.gold"
tr.StillRunningAfter = ts
-# Test 3 - 400 Response
+# Test 3 - 400 Response - Double space after field name
tr = Test.AddTestRun()
-tr.Processes.Default.Command = 'curl -s -D - -v --ipv4 --http1.1 -H "foo : bar" -H "Host: www.example.com" http://localhost:8080/'
+tr.Processes.Default.Command = 'curl -s -D - -v --ipv4 --http1.1 -H "foo : bar" -H "Host: www.example.com" http://localhost:8080/'
tr.Processes.Default.ReturnCode = 0
tr.Processes.Default.Streams.stdout = "syntax.400.gold"
tr.StillRunningAfter = ts
+
+# Test 4 - 400 Response - Three different Content-Length headers
+tr = Test.AddTestRun()
+tr.Processes.Default.Command = 'curl -s -D - -v --ipv4 --http1.1 -d "hello world" -H "Content-Length: 11" -H "Content-Length: 10" -H "Content-Length: 9" -H "Host: www.example.com" http://localhost:8080/'
+tr.Processes.Default.ReturnCode = 0
+tr.Processes.Default.Streams.stdout = "syntax.400.gold"
+tr.StillRunningAfter = ts
+
+# Test 4 - 200 Response - Three same Content-Length headers
+tr = Test.AddTestRun()
+tr.Processes.Default.Command = 'curl -s -D - -v --ipv4 --http1.1 -d "hello world" -H "Content-Length: 11" -H "Content-Length: 11" -H "Content-Length: 11" -H "Host: www.example.com" http://localhost:8080/'
+tr.Processes.Default.ReturnCode = 0
+tr.Processes.Default.Streams.stdout = "syntax.200.gold"
+tr.StillRunningAfter = ts
+
+# Test 4 - 200 Response - Three different Content-Length headers with a Transfer ecoding header
+tr = Test.AddTestRun()
+tr.Processes.Default.Command = 'curl -s -D - -v --ipv4 --http1.1 -d "hello world" -H "Transfer-Encoding: chunked" -H "Content-Length: 11" -H "Content-Length: 10" -H "Content-Length: 9" -H "Host: www.example.com" http://localhost:8080/'
+tr.Processes.Default.ReturnCode = 0
+tr.Processes.Default.Streams.stdout = "syntax.200.gold"
+tr.StillRunningAfter = ts
--
To stop receiving notification emails like this one, please contact
zwoop@apache.org.
[trafficserver] 01/04: Combine mutex of Http2ClientSession and
Http2ConnectionState
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 7.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit c66cd890243ba7d45094988eb7e53131ddd230d7
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Mon Mar 5 14:52:08 2018 +0900
Combine mutex of Http2ClientSession and Http2ConnectionState
(cherry picked from commit dca48a03511dfa6b72da65eeb1f0b82e96270e9b)
---
proxy/http2/Http2ClientSession.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/proxy/http2/Http2ClientSession.cc b/proxy/http2/Http2ClientSession.cc
index 792279e..d67759d 100644
--- a/proxy/http2/Http2ClientSession.cc
+++ b/proxy/http2/Http2ClientSession.cc
@@ -185,7 +185,7 @@ Http2ClientSession::new_connection(NetVConnection *new_vc, MIOBuffer *iobuf, IOB
this->mutex = new_vc->mutex;
this->in_destroy = false;
- this->connection_state.mutex = new_ProxyMutex();
+ this->connection_state.mutex = this->mutex;
DebugHttp2Ssn("session born, netvc %p", this->client_vc);
--
To stop receiving notification emails like this one, please contact
zwoop@apache.org.
[trafficserver] 02/04: Prevent potential dispatch of HttpTunnel
events to a dead consumer.
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 7.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit dd51074bc5a9c0a15712e2030935473b3d4b4885
Author: Alan M. Carroll <so...@yahoo-inc.com>
AuthorDate: Fri Jun 2 13:07:36 2017 -0500
Prevent potential dispatch of HttpTunnel events to a dead consumer.
(cherry picked from commit 6f0bc840b2cbfcbead88c4a411dc719c4f9d6f25)
---
proxy/http/HttpTunnel.h | 30 +++++++++++++++++++++++++-----
1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/proxy/http/HttpTunnel.h b/proxy/http/HttpTunnel.h
index 42c6fd4..54c83f0 100644
--- a/proxy/http/HttpTunnel.h
+++ b/proxy/http/HttpTunnel.h
@@ -446,12 +446,32 @@ HttpTunnel::get_producer(HttpTunnelType_t type)
inline HttpTunnelConsumer *
HttpTunnel::get_consumer(VConnection *vc)
{
- for (int i = 0; i < MAX_CONSUMERS; i++) {
- if (consumers[i].vc == vc) {
- return consumers + i;
+ /** Rare but persistent problem in which a @c INKVConnInternal is used by a consumer, released,
+ and then re-allocated for a different consumer. This causes two consumers to have the same VC
+ pointer resulting in this method returning the wrong consumer. Note this is a not a bad use of
+ the tunnel, but an unfortunate interaction with the FIFO free lists.
+
+ It's not correct to check for the consumer being alive - at a minimum `HTTP_TUNNEL_EVENT_DONE`
+ is dispatched against a consumer after the consumer is not alive. Instead if a non-alive
+ consumer matches it is stored as a candidate and returned if no other match is found. If a
+ live matching consumer is found, it is immediately returned. It is never valid to have the
+ same VC in more than one active consumer. This should avoid a performance impact because in
+ the usual case the consumer will be alive.
+
+ In the case of a deliberate dispatch of an event to a dead consumer that has a duplicate vc
+ address, this will select the last consumer which will be correct as the consumers are added
+ in order therefore the latter consumer will be the most recent / appropriate target.
+ */
+ HttpTunnelConsumer *zret = nullptr;
+ for (HttpTunnelConsumer &c : consumers) {
+ if (c.vc == vc) {
+ zret = &c;
+ if (c.alive) { // a match that's alive is always the best.
+ break;
+ }
}
}
- return NULL;
+ return zret;
}
inline HttpTunnelProducer *
@@ -470,7 +490,7 @@ HttpTunnel::get_consumer(VIO *vio)
{
if (vio) {
for (int i = 0; i < MAX_CONSUMERS; i++) {
- if (consumers[i].write_vio == vio || consumers[i].vc == vio->vc_server) {
+ if (consumers[i].alive && (consumers[i].write_vio == vio || consumers[i].vc == vio->vc_server)) {
return consumers + i;
}
}
--
To stop receiving notification emails like this one, please contact
zwoop@apache.org.
[trafficserver] 04/04: Select the current cert when looking for
issuer
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 7.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 912c44685864bea0bd8bae2fda0e2c927e6bb13b
Author: Randall Meyer <ra...@yahoo.com>
AuthorDate: Tue Mar 6 08:22:19 2018 -0800
Select the current cert when looking for issuer
This is needed when working with multiple cert chains
Fixes issue #3032
(cherry picked from commit a470492a82b4ab3aa13e944e4206f254ed9d9015)
---
iocore/net/OCSPStapling.cc | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/iocore/net/OCSPStapling.cc b/iocore/net/OCSPStapling.cc
index d764c75..b5b7e46 100644
--- a/iocore/net/OCSPStapling.cc
+++ b/iocore/net/OCSPStapling.cc
@@ -82,6 +82,13 @@ stapling_get_issuer(SSL_CTX *ssl_ctx, X509 *x)
if (inctx == nullptr) {
return nullptr;
}
+
+#ifdef SSL_CTX_select_current_cert
+ if (!SSL_CTX_select_current_cert(ssl_ctx, x)) {
+ Warning("OCSP: could not select current certifcate chain %p", x);
+ }
+#endif
+
if (X509_STORE_CTX_init(inctx, st, nullptr, nullptr) == 0) {
goto end;
}
--
To stop receiving notification emails like this one, please contact
zwoop@apache.org.