[GitHub] trafficserver issue #1551: rectify a minor error in test_server_intercept.lu...
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/1551 [approve ci] --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1394: Traffic Server - 6.2.1: Client streams count not ...
Github user masaori335 closed the issue at: https://github.com/apache/trafficserver/issues/1394 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1455: Set nullptr to ua_session after it is destoryed
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/1455 We need to backport this to 7.1.x ( I'll send another PR after we verified this works well) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1455: Set nullptr to ua_session after it is dest...
GitHub user masaori335 opened a pull request: https://github.com/apache/trafficserver/pull/1455 Set nullptr to ua_session after it is destoryed Issue: TS crushes by EXC_BAD_ACCESS in Http2ConnectionState::release_stream() under heavy load Cause: While total_connections_in is larger than max_connections_per_thread_in (in NetHandler::manage_keep_alive_queue()), Http2ConnectionState::release_stream() is called recurcively from add_to_keep_alive_queue(). At the bottom of recursion, ua_session is destroyed and Http2ConnectionState::release_stream() access to it. Fix: 1. Set nullptr to ua_session after it is destoryed 2. Swap calls of add_to_keep_alive_queue() and cancel_active_timeout() for ua_session nullptr check You can merge this pull request into a Git repository by running: $ git pull https://github.com/masaori335/trafficserver fix_h2_bad_access Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1455.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1455 commit 16d36720b15068d0c0bae0ca1fc321d0e1fa7a71 Author: Masaori Koshiba <masa...@apache.org> Date: 2017-02-15T08:31:54Z Set nullptr to ua_session after it is destoryed Issue: TS crushes by EXC_BAD_ACCESS in Http2ConnectionState::release_stream() under heavy load Cause: While total_connections_in is larger than max_connections_per_thread_in (in NetHandler::manage_keep_alive_queue()), Http2ConnectionState::release_stream() is called recurcively from add_to_keep_alive_queue(). At the bottom of recursion, ua_session is destroyed and Http2ConnectionState::release_stream() access to it. Fix: 1. Set nullptr to ua_session after it is destoryed 2. Swap calls of add_to_keep_alive_queue() and cancel_active_timeout() for ua_session nullptr check --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1442: Doc: Add an example of disabled HTTP/2 ove...
GitHub user masaori335 opened a pull request: https://github.com/apache/trafficserver/pull/1442 Doc: Add an example of disabled HTTP/2 over TLS You can merge this pull request into a Git repository by running: $ git pull https://github.com/masaori335/trafficserver doc-proto Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1442.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1442 commit 51c74aa2c33398b3960d52995320c3d0df31824e Author: Masaori Koshiba <masa...@apple.com> Date: 2017-02-14T00:59:57Z Doc: Add an example of disabled HTTP/2 over TLS --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1438: BoringSSL doesn't have BIO_sock_non_fatal_...
Github user masaori335 commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1438#discussion_r100724390 --- Diff: iocore/net/BIO_fastopen.cc --- @@ -27,6 +27,67 @@ #include "BIO_fastopen.h" +// For BoringSSL, which for some reason doesn't have this function. +// (In BoringSSL, sock_read() and sock_write() use the internal +// bio_fd_non_fatal_error() instead.) #1437 +// +// The following is copied from +// https://github.com/openssl/openssl/blob/3befffa39dbaf2688d823fcf2bdfc07d2487be48/crypto/bio/bss_sock.c --- End diff -- I understood. Let's add the attribution to the NOTICE file and use code from OpenSSL. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1437: Build fail with BoringSSL
GitHub user masaori335 reopened an issue:
https://github.com/apache/trafficserver/issues/1437
Build fail with BoringSSL
I'm trying build TS with BoringSSL, but I got this.
```
BIO_fastopen.cc:76:9: error: use of undeclared identifier
'BIO_sock_non_fatal_error'
if (BIO_sock_non_fatal_error(errno)) {
^
BIO_fastopen.cc:99:9: error: use of undeclared identifier
'BIO_sock_non_fatal_error'
if (BIO_sock_non_fatal_error(errno)) {
^
2 errors generated.
```
BoringSSL doesn't have `BIO_sock_non_fatal_error`. `bio_fd_non_fatal_error`
looks similar API, but it is internal API.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1438: BoringSSL doesn't have BIO_sock_non_fatal_...
Github user masaori335 commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1438#discussion_r100647168
--- Diff: iocore/net/BIO_fastopen.cc ---
@@ -27,6 +27,67 @@
#include "BIO_fastopen.h"
+// For BoringSSL, which for some reason doesn't have this function.
+// (In BoringSSL, sock_read() and sock_write() use the internal
+// bio_fd_non_fatal_error() instead.) #1437
+//
+// The following is copied from
+//
https://github.com/openssl/openssl/blob/3befffa39dbaf2688d823fcf2bdfc07d2487be48/crypto/bio/bss_sock.c
--- End diff --
How about just add a function like this? We don't support WINDOWS and don't
need those "#ifdef". Just checking error codes looks enough.
```
static int
non_fetal_error(int err)
{
if (err == EWOULDBLOCK || err == ENOTCONN || err == EINTR || err ==
EAGAIN || err == EPROTO || err == EINPROGRESS || err == EALREADY) {
return 1;
}
return 0;
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1438: BoringSSL doesn't have BIO_sock_non_fatal_...
Github user masaori335 commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1438#discussion_r100646468 --- Diff: iocore/net/BIO_fastopen.cc --- @@ -27,6 +27,67 @@ #include "BIO_fastopen.h" +// For BoringSSL, which for some reason doesn't have this function. +// (In BoringSSL, sock_read() and sock_write() use the internal +// bio_fd_non_fatal_error() instead.) #1437 +// +// The following is copied from +// https://github.com/openssl/openssl/blob/3befffa39dbaf2688d823fcf2bdfc07d2487be48/crypto/bio/bss_sock.c --- End diff -- It looks like we need to add "OpenSSL License" in LICENSE file, because this is just copy. (Sorry for I should have noticed this before merge) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1438: BoringSSL doesn't have BIO_sock_non_fatal_...
Github user masaori335 closed the pull request at: https://github.com/apache/trafficserver/pull/1438 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1437: Build fail with BoringSSL
Github user masaori335 closed the issue at: https://github.com/apache/trafficserver/issues/1437 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1437: Build fail with BoringSSL
GitHub user masaori335 opened an issue:
https://github.com/apache/trafficserver/issues/1437
Build fail with BoringSSL
I'm trying build TS with BoringSSL, but I got this.
```
BIO_fastopen.cc:76:9: error: use of undeclared identifier
'BIO_sock_non_fatal_error'
if (BIO_sock_non_fatal_error(errno)) {
^
BIO_fastopen.cc:99:9: error: use of undeclared identifier
'BIO_sock_non_fatal_error'
if (BIO_sock_non_fatal_error(errno)) {
^
2 errors generated.
```
BoringSSL doesn't have `BIO_sock_non_fatal_error`. `bio_fd_non_fatal_error`
looks similar API, but it is internal API.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1394: Traffic Server - 6.2.1: Client streams count not ...
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/issues/1394 @jaaju Thanks! It looks like the code is dropped accidentally, when back port TS-4813 to 6.2.x. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1394: Traffic Server - 6.2.1: Client streams count not ...
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/issues/1394 I confirmed that this issue happens only 6.2.x. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1395: Update client streams count in Http2ConnectionSta...
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/1395 [approve ci] --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1394: Traffic Server - 6.2.1: Client streams count not ...
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/issues/1394 @jaaju Does this happen on 7.x? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #990: TS-3216: Add HPKP Support
Github user masaori335 closed the pull request at: https://github.com/apache/trafficserver/pull/990 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #990: TS-3216: Add HPKP Support
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/990 Sorry, no updates. Now, I'm thinking about close this Pull-Request once and come back after ssl_multicert.config is replaced by lua. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1196: TS-4217: Change stream state after sending...
GitHub user masaori335 opened a pull request: https://github.com/apache/trafficserver/pull/1196 TS-4217: Change stream state after sending HEADERS frame Change stream state to CLOSED after sending a HEADERS w/ END_STREAM flag to avoid sending a unnecessary DATA frame w/ END_STREAM flag. (cherry picked from commit f5c2a2d8d3b4dd4afc1c32ec32627d56c16f78e0) You can merge this pull request into a Git repository by running: $ git pull https://github.com/masaori335/trafficserver 6.2.x_ts-4217 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1196.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1196 commit 0c3f8ade5b160f09b60b87629fa3fb43f4937a63 Author: Masakazu Kitajo <mas...@apache.org> Date: 2016-08-21T14:15:28Z TS-4217: Change stream state after sending HEADERS frame Change stream state to CLOSED after sending a HEADERS w/ END_STREAM flag to avoid sending a unnecessary DATA frame w/ END_STREAM flag. (cherry picked from commit f5c2a2d8d3b4dd4afc1c32ec32627d56c16f78e0) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1195: TS-4934: Remove invalid assert
GitHub user masaori335 opened a pull request: https://github.com/apache/trafficserver/pull/1195 TS-4934: Remove invalid assert (cherry picked from commit 6e72b2a8e52a421f2e3bc88cf476460123e88fc3) You can merge this pull request into a Git repository by running: $ git pull https://github.com/masaori335/trafficserver 6.2.x_ts-4934 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1195.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1195 commit 1f807a940eb561a6e738eeec4c06a1832f6f4392 Author: Masaori Koshiba <masa...@apache.org> Date: 2016-10-05T05:17:20Z TS-4934: Remove invalid assert (cherry picked from commit 6e72b2a8e52a421f2e3bc88cf476460123e88fc3) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1194: TS-4905: Set parent NULL after destroy() i...
GitHub user masaori335 opened a pull request: https://github.com/apache/trafficserver/pull/1194 TS-4905: Set parent NULL after destroy() is called (cherry picked from commit 60d3d1fc542f9d33317e35ced399bef1f9c89516) You can merge this pull request into a Git repository by running: $ git pull https://github.com/masaori335/trafficserver 6.2.x_ts-4905 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1194.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1194 commit c4b2704b17775f5dbe42997a60fa900d16d30d1d Author: Masaori Koshiba <masa...@apache.org> Date: 2016-10-04T08:56:45Z TS-4905: Set parent NULL after destroy() is called (cherry picked from commit 60d3d1fc542f9d33317e35ced399bef1f9c89516) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1190: TS-4466: Add links to other versions of the docum...
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/1190 OK, let's leave it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1189: TS-4554: Add some limitations in Http2DependencyT...
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/1189 `PriorityQueue<T, Comp>::erase` is added by TS-4537, so we need backport this too. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1190: TS-4466: Add links to other versions of the docum...
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/1190 @PSUdaemon Should I rebase and squash those commits to one commit? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1190: TS-4466: Add links to other versions of th...
GitHub user masaori335 opened a pull request: https://github.com/apache/trafficserver/pull/1190 TS-4466: Add links to other versions of the documentation (back-port) Backport https://github.com/apache/trafficserver/pull/654 You can merge this pull request into a Git repository by running: $ git pull https://github.com/masaori335/trafficserver 6.2.x_ts-4466 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1190.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1190 commit 48e21a507e4f1a745575c5e242d0c1b110907810 Author: Masakazu Kitajo <mas...@apache.org> Date: 2016-05-21T08:31:33Z TS-4466: Add links to other versions of the documentation (cherry picked from commit 70a23a925be4800efbd24d82217e6d22084f2e14) commit f85938fff0e4237a40a3f92e4409e32b99dfa855 Author: Masakazu Kitajo <mas...@apache.org> Date: 2016-05-21T08:46:48Z TS-4466: Specify language explicitly (cherry picked from commit 59fb3d33391bc4f9adc0417099ea96cddac88cba) Conflicts: ci/jenkins/bin/docs.sh commit f8254e5ec09cbbf31fbc33c6454628a133232c97 Author: Masakazu Kitajo <mas...@apache.org> Date: 2016-05-24T07:16:38Z TS-4466: Improve a layout and behavior of the switcher (cherry picked from commit ca35a0528b83c74ed7aa2f05cb73041648505b86) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1189: TS-4554: Add some limitations in Http2Depe...
GitHub user masaori335 opened a pull request: https://github.com/apache/trafficserver/pull/1189 TS-4554: Add some limitations in Http2DependencyTree - Set max depth of Http2DependencyTree When the depth over the maximum, new node will be a children of root node. - Limit number of Http2DependencyTree node - Remove node from Http2DependencyTree when delete streams (cherry picked from commit c71fa8c5e4c4c80217e7927752fd6febb5812ba7) Conflicts: proxy/http2/Http2ConnectionState.cc You can merge this pull request into a Git repository by running: $ git pull https://github.com/masaori335/trafficserver 6.2.x_ts-4554 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1189.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1189 commit 4b3711ff1628f263f6f1225fbb69e9bb0ca0002e Author: Masaori Koshiba <masa...@apache.org> Date: 2016-07-28T11:06:09Z TS-4554: Add some limitations in Http2DependencyTree - Set max depth of Http2DependencyTree When the depth over the maximum, new node will be a children of root node. - Limit number of Http2DependencyTree node - Remove node from Http2DependencyTree when delete streams (cherry picked from commit c71fa8c5e4c4c80217e7927752fd6febb5812ba7) Conflicts: proxy/http2/Http2ConnectionState.cc --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1162: TS-5019: Add total header length checks in HPACK ...
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/1162 Updated. Please take another look. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1162: TS-5019: Add total header length checks in...
GitHub user masaori335 opened a pull request: https://github.com/apache/trafficserver/pull/1162 TS-5019: Add total header length checks in HPACK (back-port) You can merge this pull request into a Git repository by running: $ git pull https://github.com/masaori335/trafficserver 6.2.x_ts-5019 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1162.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1162 commit 7557ca0eb1a1d6090d59d9aaf584994e89d9028a Author: Masaori Koshiba <masa...@apache.org> Date: 2016-10-27T23:18:58Z TS-5019: Add total header length checks in HPACK --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1161: TS-5018: Fix CID 1365301: Control flow iss...
GitHub user masaori335 opened a pull request: https://github.com/apache/trafficserver/pull/1161 TS-5018: Fix CID 1365301: Control flow issues (DEADCODE) I think Coverity says that when `state` is not `PARSER_IN_REGEX`, `extracting_token` is always false. You can merge this pull request into a Git repository by running: $ git pull https://github.com/masaori335/trafficserver ts-5018 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1161.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1161 commit c45acda3199f0842ffaee8f3a52d8cde57d73fcb Author: Masaori Koshiba <masa...@apache.org> Date: 2016-10-27T22:25:52Z TS-5018: Fix CID 1365301: Control flow issues (DEADCODE) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1160: TS-5019: Add header length checks in HPACK
GitHub user masaori335 opened a pull request: https://github.com/apache/trafficserver/pull/1160 TS-5019: Add header length checks in HPACK You can merge this pull request into a Git repository by running: $ git pull https://github.com/masaori335/trafficserver ts-5019 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1160.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1160 commit 959975f0e37de0f557c8f6aa6ea7890fa96b6899 Author: Masaori Koshiba <masa...@apache.org> Date: 2016-10-27T23:18:58Z TS-5019: Add header length checks in HPACK --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1144: TS-4865: Fix CID 1362785, CID 1362784
GitHub user masaori335 opened a pull request: https://github.com/apache/trafficserver/pull/1144 TS-4865: Fix CID 1362785, CID 1362784 - [TS-4865](https://issues.apache.org/jira/browse/TS-4865) You can merge this pull request into a Git repository by running: $ git pull https://github.com/masaori335/trafficserver ts-4865 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1144.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1144 commit 6fef99ab495ddf68cdb135dece2f9827d11ba72a Author: Masaori Koshiba <masa...@apache.org> Date: 2016-10-26T18:59:32Z TS-4865: Fix CID 1362785, CID 1362784 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1100: TS-4916: Fix for an H2-infinite-loop deadl...
Github user masaori335 commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1100#discussion_r83155537 --- Diff: iocore/aio/.diags.log.meta --- @@ -0,0 +1 @@ +creation_time = 1476307057 --- End diff -- Do we need this file? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1088: TS-4915: Crash from hostdb in PriorityQueueLess
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/1088 @shinrich It looks working correctly. What is problem with the vector `W(10), Z(40), and Y(30)` in that case? This PriorityQueue is implemented as BinaryHeap in an array. So left child node(`Z(40)`) could be larger than right child node(`Y(30)`). And we don't need to sort the array. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1080: TS-4934: Remove invalid assert
Github user masaori335 closed the pull request at: https://github.com/apache/trafficserver/pull/1080 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1080: Remove invalid assert
GitHub user masaori335 opened a pull request: https://github.com/apache/trafficserver/pull/1080 Remove invalid assert [TS-4934](https://issues.apache.org/jira/browse/TS-4934) The active timeout could be happen with any states of stream. So this assert doesn't make sense. You can merge this pull request into a Git repository by running: $ git pull https://github.com/masaori335/trafficserver ts-4934 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1080.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1080 commit d5e95097177bafd7a924183fd656cacec68e00ae Author: Masaori Koshiba <masa...@apache.org> Date: 2016-10-05T05:17:20Z Remove invalid assert --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1052: TS-4813: Fix lingering stream.
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/1052 Looks good to me ð --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1075: TS-4905: Set parent NULL after destroy() i...
Github user masaori335 commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1075#discussion_r81887418
--- Diff: proxy/http/Http1ClientTransaction.cc ---
@@ -67,6 +67,7 @@ Http1ClientTransaction::transaction_done()
// If the parent session is not in the closed state, the destroy will
not occur.
if (parent) {
parent->destroy();
+parent = NULL;
--- End diff --
Yeah, I'm not 100% sure this is right or not. But from the backtrace,
`destroy()` calls `free()`. In this case we need to set parent NULL.
```
#0 Http1ClientSession::free (this=0x60ae0001cc80) at
Http1ClientSession.cc:100
#1 0x005c2386 in ProxyClientSession::handle_api_return
(this=0x60ae0001cc80, event=6) at ProxyClientSession.cc:206
#2 0x005c1f25 in ProxyClientSession::do_api_callout
(this=0x60ae0001cc80, id=TS_HTTP_SSN_CLOSE_HOOK) at ProxyClientSession.cc:177
#3 0x0065f787 in Http1ClientSession::destroy (this=0x60ae0001cc80)
at Http1ClientSession.cc:93
#4 0x00664d20 in Http1ClientTransaction::transaction_done
(this=0x60ae0001cf60) at Http1ClientTransaction.cc:69
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1075: TS-4905: Set parent NULL after destroy() i...
GitHub user masaori335 opened a pull request: https://github.com/apache/trafficserver/pull/1075 TS-4905: Set parent NULL after destroy() is called [TS-4905](https://issues.apache.org/jira/browse/TS-4905) You can merge this pull request into a Git repository by running: $ git pull https://github.com/masaori335/trafficserver ts-4905 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1075.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1075 commit 9b6cd5d444bba0b5476ef67f98eea504dd6fa596 Author: Masaori Koshiba <masa...@apache.org> Date: 2016-10-04T08:56:45Z TS-4905: Set parent NULL after destroy() is called --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1067: TS-4914: Fix response headers on 304 response
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/1067 [approve ci] --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1062: [TS-4908] Remove duplicated closing continuation.
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/1062 [approve ci] --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1052: TS-4813: Fix lingering stream.
Github user masaori335 commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1052#discussion_r80833386
--- Diff: proxy/http2/Http2ConnectionState.cc ---
@@ -1174,9 +1174,9 @@ Http2ConnectionState::send_a_data_frame(Http2Stream
*stream, size_t _len
void
Http2ConnectionState::send_data_frames(Http2Stream *stream)
{
- if (stream->get_state() == HTTP2_STREAM_STATE_CLOSED ||
stream->get_state() == HTTP2_STREAM_STATE_HALF_CLOSED_LOCAL) {
+/* if (stream->get_state() == HTTP2_STREAM_STATE_CLOSED ||
stream->get_state() == HTTP2_STREAM_STATE_HALF_CLOSED_LOCAL) {
return;
- }
+ } */
--- End diff --
The motivation of this check is to respect HTTP/2 spec. RFC 7540 says below
> An endpoint MUST NOT send frames other than PRIORITY on a closed stream.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #990: TS-3216: Add HPKP Support
Github user masaori335 commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/990#discussion_r80831720
--- Diff: iocore/net/SSLUtils.cc ---
@@ -1895,10 +1963,17 @@ ssl_store_ssl_context(const SSLConfigParams
*params, SSLCertLookup *lookup, cons
keyblock = ssl_context_enable_tickets(ctx, NULL);
}
+ // Generate HPKP header if hpkp is enabled.
+ if (sslMultCertSettings.hpkp_enabled >= 0 ?
sslMultCertSettings.hpkp_enabled : params->hpkp_enabled) {
+hpkp = ssl_context_enable_hpkp(params, sslMultCertSettings);
--- End diff --
It looks like `SSLInitServerContext` doesn't handle SSLCertContext. What
I'm missing?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #990: TS-3216: Add HPKP Support
Github user masaori335 commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/990#discussion_r80829654
--- Diff: proxy/hdrs/HdrToken.cc ---
@@ -209,6 +210,8 @@ static HdrTokenFieldInfo
_hdrtoken_strs_field_initializers[] = {
{"Proxy-Authorization", MIME_SLOTID_NONE,
MIME_PRESENCE_PROXY_AUTHORIZATION, (HTIF_HOPBYHOP | HTIF_PROXYAUTH)},
{"Proxy-Connection", MIME_SLOTID_PROXY_CONNECTION,
MIME_PRESENCE_PROXY_CONNECTION, (HTIF_COMMAS | HTIF_MULTVALS | HTIF_HOPBYHOP)},
{"Public", MIME_SLOTID_NONE, MIME_PRESENCE_PUBLIC, (HTIF_COMMAS |
HTIF_MULTVALS)},
+ {"Public-Key-Pins", MIME_SLOTID_NONE, MIME_PRESENCE_NONE, HTIF_NONE},
--- End diff --
@SolidWallOfCode Should I add those line end of this array? or just remove?
Actually I'm not strongly want to add WKS strings, I just followed
Strict-Transport-Security header for HSTS.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #990: TS-3216: Add HPKP Support
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/990 I'm going to fix issues and add a test by TSQA. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1037: TS-4883: Fix Thread::start call in EventProcessor...
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/1037 [approve ci] --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1037: TS-4883: Fix Thread::start call in EventProcessor...
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/1037 Looks good to me --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1018: TS-4833: Check stream is not closed when restart ...
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/1018 yeah, this is workaround till TS-4607 is fixed:) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1018: TS-4833: Check stream is not closed when r...
GitHub user masaori335 opened a pull request: https://github.com/apache/trafficserver/pull/1018 TS-4833: Check stream is not closed when restart it I found below in dumped stream object in [frame #0](https://issues.apache.org/jira/browse/TS-4833?focusedCommentId=15475142=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15475142) ``` _state=HTTP2_STREAM_STATE_HALF_CLOSED_REMOTE closed=true ``` I think restarting closed stream is root cause. You can merge this pull request into a Git repository by running: $ git pull https://github.com/masaori335/trafficserver ts-4833 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1018.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1018 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #990: TS-3216: Add HPKP Support
Github user masaori335 commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/990#discussion_r78311103
--- Diff: iocore/net/SSLUtils.cc ---
@@ -344,6 +361,11 @@ set_context_cert(SSL *ssl)
ctx = cc->ctx;
}
+ if (cc && cc->hpkp) {
+netvc->hpkp = cc->hpkp;
--- End diff --
Right, it could be problem.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #990: TS-3216: Add HPKP Support
Github user masaori335 commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/990#discussion_r78310812
--- Diff: iocore/net/SSLUtils.cc ---
@@ -344,6 +361,11 @@ set_context_cert(SSL *ssl)
ctx = cc->ctx;
}
+ if (cc && cc->hpkp) {
+netvc->hpkp = cc->hpkp;
--- End diff --
SSLCertContext still owns that.
Hmm, copying the values of hpkp is only one choice?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #990: TS-3216: Add HPKP Support
Github user masaori335 commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/990#discussion_r78309860
--- Diff: iocore/net/SSLUtils.cc ---
@@ -1866,10 +1934,17 @@ ssl_store_ssl_context(const SSLConfigParams
*params, SSLCertLookup *lookup, cons
keyblock = ssl_context_enable_tickets(ctx, NULL);
}
+ // Generate HPKP header if hpkp is enabled.
+ if (sslMultCertSettings.hpkp_enabled >= 0 ?
sslMultCertSettings.hpkp_enabled : params->hpkp_enabled) {
--- End diff --
It is possible:) `sslMultCertSettings.hpkp_enabled` is initialized by `-1`,
so settings in ssl_multicert.config (`0` or `1`) is preceded if they're
written. This is expected behavior, right?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #999: TS-4759: Fix stream states management
Github user masaori335 closed the pull request at: https://github.com/apache/trafficserver/pull/999 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #999: TS-4759: Fix stream states management
GitHub user masaori335 opened a pull request: https://github.com/apache/trafficserver/pull/999 TS-4759: Fix stream states management Below is the situation when this failure is happen. 1. h2spec sends a HEADERS frame with END_HEADERS flag and without END_STREAM flag. 2. TS returns a HEADERS frame and two DATA frames immediately. And TS set server side stream state `closed`. 3. h2spec sends RST_STREAM with a length other than 4 cotets. 4. TS ignores RST_STREAM to closed stream. There are two problems. - h2spec assumes server side stream state is open. ( This is fixed by h2spec v1.5.0 ) - At #2, TS should change server side stream state to `half-close (local)`. And send RST_STREAM frame to client and make state `closed`. To fix this - Change stream state to `half-close (local)` from `idle` or `open` when send a frame w/ END_STREAM flag - Make send_a_data_frame to return HTTP2_SEND_A_DATA_FRAME_DONE when send DATA frame w/ END_STREAM flag - Set stream state CLOSED when error is happen You can merge this pull request into a Git repository by running: $ git pull https://github.com/masaori335/trafficserver ts-4759 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/999.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #999 commit a907e554efcfd4789ad13e7bf3df32ed2de273ea Author: Masaori Koshiba <masa...@apache.org> Date: 2016-08-29T06:19:00Z TS-4759: Fix stream states management - Change stream state from IDLE or OPEN to HALF_CLOSED_LOCAL when send a frame w/ END_STREAM - Make send_a_data_frame to return HTTP2_SEND_A_DATA_FRAME_DONE when send DATA frame w/ END_STREAM - Set stream state CLOSED when error is happen --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #990: TS-3216: Add HPKP Support
GitHub user masaori335 opened a pull request: https://github.com/apache/trafficserver/pull/990 TS-3216: Add HPKP Support - A Pull-Request for hpkp-003.patch on [TS-3216](https://issues.apache.org/jira/browse/TS-3216) - Differences from hpkp-003.patch on TS-3216 is below - Rebase on latest master - Remove calculation of pins from certs - Add a configuration to specify comma separated list of pins Recently I found a use case that pinning only root and intermediate certificate. ( e.g. [Analysis github.com on report-uri](https://report-uri.io/home/pkp_analyse/https%3A%2F%2Fgithub.com) ) To support this use case and add many backup pins, add a configuration of list of pins. You can merge this pull request into a Git repository by running: $ git pull https://github.com/masaori335/trafficserver ts-3216 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/990.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #990 commit e83aba58688d4d63b0eb8b718553510dd93a1c74 Author: Masaori Koshiba <masa...@apache.org> Date: 2016-08-30T02:53:24Z TS-3216: Add HPKP Support --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #951: TS-4797: Allow backslash-escape in header_r...
Github user masaori335 closed the pull request at: https://github.com/apache/trafficserver/pull/951 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #951: TS-4797: Allow backslash-escape in header_rewrite ...
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/951 Please take another look. I added test cases which @maskit pointed out. And allow backslash-escape outside of quoted-string too. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #951: TS-4797: Allow backslash-escape in header_r...
Github user masaori335 commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/951#discussion_r77287760
--- Diff: plugins/header_rewrite/parser.cc ---
@@ -50,6 +52,12 @@ Parser::Parser(const std::string ) : _cond(false),
_empty(false)
_tokens.push_back(std::string(1, line[i]));
}
continue; /* always eat whitespace */
+} else if (line[i] == '\\') {
+ // erase a backslash in quoted-string
+ if (inquote && extracting_token) {
--- End diff --
My commit message might be not suitable. What I want to fix is we can't use
**double quote** inside of quoted string.
We can make this more general, but it makes header_rewrite rules and parser
code more complicated.
Do you have any specific use cases to escape something outside of quoted
string?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #951: Allow backslash-escape in header_rewrite ru...
GitHub user masaori335 opened a pull request: https://github.com/apache/trafficserver/pull/951 Allow backslash-escape in header_rewrite rules You can merge this pull request into a Git repository by running: $ git pull https://github.com/masaori335/trafficserver ts-4797 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/951.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #951 commit ab1269b88000250273ef4e1629433306953238a4 Author: Masaori Koshiba <masa...@apache.org> Date: 2016-08-30T10:23:46Z Allow backslash-escape in header_rewrite rules --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #917: TS-4167: Change default value of proxy.config.http...
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/917 Looks good to me ð --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #888: TS-4665: Http2 not terminating stream with short c...
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/888 Looks good to me. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #895: TS-4700: Change the default timeout for HTTP/2
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/895 Looks good ð --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #886: TS-4217: Change stream state after sending HEADERS...
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/886 Looks good to me --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #863: TS-4750: Fix Connection Leak warnings.
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/863 The change seems reasonable. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #846: TS-4726: Remove unnecessary assert in Proxy...
Github user masaori335 closed the pull request at: https://github.com/apache/trafficserver/pull/846 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #847: TS-4729: Remove dead assaignment in Http2Stream
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/847 #849 is the new one by @shinrich --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #847: TS-4729: Remove dead assaignment in Http2St...
Github user masaori335 closed the pull request at: https://github.com/apache/trafficserver/pull/847 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #847: TS-4729: Remove dead assaignment in Http2St...
GitHub user masaori335 opened a pull request: https://github.com/apache/trafficserver/pull/847 TS-4729: Remove dead assaignment in Http2Stream [TS-4729](https://issues.apache.org/jira/browse/TS-4729) You can merge this pull request into a Git repository by running: $ git pull https://github.com/masaori335/trafficserver ts-4729 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/847.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #847 commit dd48279f66d503312c3f3b5a1ad534bda5d3a65c Author: Masaori Koshiba <masa...@apache.org> Date: 2016-08-10T03:06:54Z TS-4729: Remove dead assaignment in Http2Stream --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #846: TS-4726: Remove unnecessary assert in ProxyClientT...
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/846 Yes, it is moved to `ProxyClientTransaction::destroy()` by TS-4507 https://github.com/apache/trafficserver/blob/master/proxy/ProxyClientTransaction.cc#L85 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #830: TS-4554: Add some limitations in Http2Depen...
Github user masaori335 closed the pull request at: https://github.com/apache/trafficserver/pull/830 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #846: TS-4726: Remove unnecessary assert in Proxy...
GitHub user masaori335 opened a pull request: https://github.com/apache/trafficserver/pull/846 TS-4726: Remove unnecessary assert in ProxyClientTransaction::release [TS-4726](https://issues.apache.org/jira/browse/TS-4726) You can merge this pull request into a Git repository by running: $ git pull https://github.com/masaori335/trafficserver ts-4726 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/846.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #846 commit d3ae487d9f7f9350b57f628aa25d39c8e829ad7d Author: Masaori Koshiba <masa...@apache.org> Date: 2016-08-09T07:32:29Z TS-4726: Remove unnecessary assert in ProxyClientTransaction::release --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #842: TS-4717: Http2 stack explosion.
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/842 Sorry, my local master was old. The crash with h2spec(4.3) is not related this Pull-Request. Filed this issue as [TS-4726](https://issues.apache.org/jira/browse/TS-4726). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #842: TS-4717: Http2 stack explosion.
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/842 I'm seeing crash with h2spec. I don't see this with latest master. h2spec : https://paste.apache.org/SU02 ATS(w/ --enable-debug) : https://paste.apache.org/mPdu --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #838: TS-4713: Remove obsolete TSFetchClientProto...
Github user masaori335 closed the pull request at: https://github.com/apache/trafficserver/pull/838 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #830: TS-4554: Set max depth of Http2DependencyTree
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/830 @maskit Possibly, but not sure. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #838: TS-4713: Remove obsolete TSFetchClientProto...
GitHub user masaori335 opened a pull request: https://github.com/apache/trafficserver/pull/838 TS-4713: Remove obsolete TSFetchClientProtoStackSet/Get You can merge this pull request into a Git repository by running: $ git pull https://github.com/masaori335/trafficserver ts-4713 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/838.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #838 commit 87b898c146ad4b4408030390e7b82fcaee423792 Author: Masaori Koshiba <masa...@apache.org> Date: 2016-08-03T02:26:31Z TS-4713: Remove obsolete TSFetchClientProtoStackSet/Get --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #833: TS-3474: HTTP/2 Server Push support
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/833 Looks good. Please squash commits. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #830: TS-4554: Set max depth of Http2DependencyTree
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/830 Yes, we don't have to follow it, but we should follow it as much as possible. Because it could be happen that images are sent before CSS/JS, if we don't follow it. How about use SETTINGS_MAX_CONCURRENT_STREAMS as the max depth? It looks appropriate and we don't need new configures. I'm seeing that with latest Chrome ( 51.0.2704.106 ). Chrome had that bug in Feb and reverted the changes. IIUC, the bug is about the order of streams not depth of streams. Now (maybe from Jun), it looks like they fixed the order and rolled it out. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #830: TS-4554: Set max depth of Http2DependencyTree
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/830 Chrome 51:) The tree of it has all streams in only one branch (all headers frame has exclusive flag). So the depth is equal to steams. For FireFox, 8 - 16 is big enough. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #830: TS-4554: Set max depth of Http2DependencyTree
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/830 1) 256 seems crazy deep, is that a reasonable default? SETTINGS_MAX_CONCURRENT_STREAMS is 100 at least. And "idle" streams (that are not counted for concurrent streams) can be node of tree. Those can be said for odd-numbered streams which is used by client and even-numbered streams which is used by server. So I chose 256. If we ignore server-push cases, it could be 128. 2) Do we want to make this configurable instead of static? Are there use cases where someone want more (or less) ? Hmm, if someone wants to increase SETTINGS_MAX_CONCURRENT_STREAMS, it looks like this should be increased too. But this is a limit of recursion, so I don't think this should be configurable for now. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #833: TS-3474: HTTP/2 Server Push support
Github user masaori335 commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/833#discussion_r72921757
--- Diff: proxy/http2/Http2ConnectionState.cc ---
@@ -1189,6 +1217,93 @@ Http2ConnectionState::send_headers_frame(Http2Stream
*stream)
}
void
+Http2ConnectionState::send_push_promise_frame(Http2Stream *stream, const
char *url, int url_len)
+{
+ HTTPHdr h1_hdr, h2_hdr;
+ uint8_t *buf= NULL;
+ 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) {
+return;
+ }
+
+ DebugHttp2Stream(ua_session, stream->get_id(), "Send PUSH_PROMISE
frame");
+
+ h1_hdr.create(HTTP_TYPE_REQUEST);
+ h1_hdr.url_set(url, url_len);
+ h1_hdr.method_set("GET", 3);
+ http2_generate_h2_header_from_1_1(_hdr, _hdr);
+
+ buf_len = h1_hdr.length_get() * 2; // Make it double just in case
+ h1_hdr.destroy();
+ buf = (uint8_t *)ats_malloc(buf_len);
+ if (buf == NULL) {
+h2_hdr.destroy();
+return;
+ }
+ Http2ErrorCode result = http2_encode_header_blocks(_hdr, buf,
buf_len, _blocks_size, *(this->remote_hpack_handle));
+ if (result != HTTP2_ERROR_NO_ERROR) {
+h2_hdr.destroy();
+ats_free(buf);
+return;
+ }
+
+ // Send a PUSH_PROMISE frame
+ if (header_blocks_size <=
BUFFER_SIZE_FOR_INDEX(buffer_size_index[HTTP2_FRAME_TYPE_PUSH_PROMISE]) -
HTTP2_FRAME_HEADER_LEN) {
+payload_length = header_blocks_size;
+flags |= HTTP2_FLAGS_PUSH_PROMISE_END_HEADERS;
+ } else {
+payload_length =
BUFFER_SIZE_FOR_INDEX(buffer_size_index[HTTP2_FRAME_TYPE_PUSH_PROMISE]) -
HTTP2_FRAME_HEADER_LEN;
+ }
+ 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;
+ Http2PushPromise push_promise;
+ 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, );
+ sent += payload_length;
+
+ // Send CONTINUATION frames
+ flags = 0;
+ while (sent < header_blocks_size) {
+DebugHttp2Stream(ua_session, stream->get_id(), "Send CONTINUATION
frame");
+payload_length =
MIN(BUFFER_SIZE_FOR_INDEX(buffer_size_index[HTTP2_FRAME_TYPE_CONTINUATION]) -
HTTP2_FRAME_HEADER_LEN,
+ header_blocks_size - sent);
+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, );
+sent += payload_length;
+ }
+ ats_free(buf);
+
+ stream = this->create_stream(id);
+ if (!stream) {
+return;
+ }
+ stream->change_state(HTTP2_FRAME_TYPE_PUSH_PROMISE, 0);
--- End diff --
Should `flags` be passed?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #833: TS-3474: HTTP/2 Server Push support
Github user masaori335 commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/833#discussion_r72921096
--- Diff: proxy/http2/Http2ConnectionState.cc ---
@@ -1189,6 +1217,93 @@ Http2ConnectionState::send_headers_frame(Http2Stream
*stream)
}
void
+Http2ConnectionState::send_push_promise_frame(Http2Stream *stream, const
char *url, int url_len)
+{
+ HTTPHdr h1_hdr, h2_hdr;
+ uint8_t *buf= NULL;
+ 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) {
+return;
+ }
+
+ DebugHttp2Stream(ua_session, stream->get_id(), "Send PUSH_PROMISE
frame");
+
+ h1_hdr.create(HTTP_TYPE_REQUEST);
+ h1_hdr.url_set(url, url_len);
+ h1_hdr.method_set("GET", 3);
+ http2_generate_h2_header_from_1_1(_hdr, _hdr);
+
+ buf_len = h1_hdr.length_get() * 2; // Make it double just in case
+ h1_hdr.destroy();
+ buf = (uint8_t *)ats_malloc(buf_len);
+ if (buf == NULL) {
+h2_hdr.destroy();
+return;
+ }
+ Http2ErrorCode result = http2_encode_header_blocks(_hdr, buf,
buf_len, _blocks_size, *(this->remote_hpack_handle));
+ if (result != HTTP2_ERROR_NO_ERROR) {
+h2_hdr.destroy();
+ats_free(buf);
+return;
+ }
+
+ // Send a PUSH_PROMISE frame
+ if (header_blocks_size <=
BUFFER_SIZE_FOR_INDEX(buffer_size_index[HTTP2_FRAME_TYPE_PUSH_PROMISE]) -
HTTP2_FRAME_HEADER_LEN) {
--- End diff --
`HTTP2_FRAME_TYPE_PUSH_PROMISE` is used for frame payload size in below
(L1263), so it doesn't include frame header size.
If I understand correctly, we should minus 4 byte (length of Promised
Stream ID).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #833: TS-3474: HTTP/2 Server Push support
Github user masaori335 commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/833#discussion_r72919885
--- Diff: proxy/http2/Http2ConnectionState.cc ---
@@ -41,7 +41,7 @@ static const int buffer_size_index[HTTP2_FRAME_TYPE_MAX]
= {
-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_PUSH_PROMISE
+ BUFFER_SIZE_INDEX_128, // HTTP2_FRAME_TYPE_PUSH_PROMISE
--- End diff --
`128` is enough size for PUSH_PROMISE frame? It contains Header Block
Fragment, so 16K (like HEADERS frame and CONTINUATION frame) is appropriate?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #830: TS-4554: Set max depth of Http2DependencyTr...
GitHub user masaori335 opened a pull request: https://github.com/apache/trafficserver/pull/830 TS-4554: Set max depth of Http2DependencyTree - Set max depth of Http2DependencyTree When the depth over the maximum, new node will be a children of root node. - Remove node from Http2DependencyTree when delete streams. You can merge this pull request into a Git repository by running: $ git pull https://github.com/masaori335/trafficserver ts-4554 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/830.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #830 commit 248a4873bc8889493ab3aaebc166d5e03d66 Author: Masaori Koshiba <masa...@apache.org> Date: 2016-07-28T11:06:09Z TS-4554: Set max depth of Http2DependencyTree - Set max depth of Http2DependencyTree When the depth over the maximum, new node will be a children of root node. - Remove node from Http2DependencyTree when delete streams --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #829: TS-4703: Adds an API call to retrieve transaction ...
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/829 That's good idea:) @petarpenkov would you add `TSHttpSsnClientProtocolGet(TSHttpSsn ssnp)` which is wrapper of `ProxyClientSession::get_protocol_string()` ? ( Session version of `TSHttpTxnClientProtocolGet(TSHttpTnx tnxp)` ) When we want to write some plugins to extend HTTP/2 (e.g. origin-frame, cache-digest), it is useful to get client protocol at session hooks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #753: TS-4705: Proposal: NetVC Context
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/753 @oknet AFAIK, it's same. Please set type of JIRA 'improvement'. Currently all code changes are tracked by JIRA. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #829: TS-4703: Adds an API call to retrieve transaction ...
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/829 @shukitchan I don't think so. We need one more API, `TSHttpSsnClientProtocolGet(TSHttpSsn ssnp)`, for TS-2987. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #753: Proposal: NetVC Context
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/753 BTW, which is JIRA ticket for this? And would you use the ticket number in commit message? Our commit log format requires it. https://cwiki.apache.org/confluence/display/TS/CommitPolicies --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #753: Proposal: NetVC Context
Github user masaori335 commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/753#discussion_r72574922 --- Diff: iocore/net/UnixConnection.cc --- @@ -320,14 +320,15 @@ Connection::connect(sockaddr const *target, NetVCOptions const ) int res; - this->setRemote(target); + if (target != NULL) +this->setRemote(target); // apply dynamic options with this.addr initialized apply_options(opt); cleaner cleanup(this, ::_cleanup); // mark for close until we succeed. - res = ::connect(fd, target, ats_ip_size(target)); + res = ::connect(fd, >addr.sa, ats_ip_size(>addr.sa)); --- End diff -- @oknet Ah, got it. Thanks! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #829: TS-4703: Adds an API call to retrieve trans...
Github user masaori335 commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/829#discussion_r72550556
--- Diff: proxy/InkAPI.cc ---
@@ -4655,6 +4655,15 @@ TSHttpTxnClientReqGet(TSHttpTxn txnp, TSMBuffer
*bufp, TSMLoc *obj)
return TS_ERROR;
}
+const char *
+TSHttpTxnClientProtocolGet(TSHttpTxn txnp)
+{
+ sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
+
+ HttpSM *sm = reinterpret_cast(txnp);
+ return sm->ua_session->get_protocol_string();
--- End diff --
And we can get `client_protocol` from HttpSM directly:)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #829: TS-4703: Adds an API call to retrieve trans...
Github user masaori335 commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/829#discussion_r72548973
--- Diff: proxy/InkAPI.cc ---
@@ -4655,6 +4655,15 @@ TSHttpTxnClientReqGet(TSHttpTxn txnp, TSMBuffer
*bufp, TSMLoc *obj)
return TS_ERROR;
}
+const char *
+TSHttpTxnClientProtocolGet(TSHttpTxn txnp)
+{
+ sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
+
+ HttpSM *sm = reinterpret_cast(txnp);
+ return sm->ua_session->get_protocol_string();
--- End diff --
It is better to check `sm->ua_session` is not NULL, before call
`get_protocol_string()`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #753: Proposal: NetVC Context
Github user masaori335 commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/753#discussion_r72402523
--- Diff: iocore/net/UnixNetVConnection.cc ---
@@ -1296,7 +1294,7 @@ UnixNetVConnection::connectUp(EThread *t, int fd)
}
if (fd == NO_FD) {
-res = con.connect(_addr.sa, options);
+res = con.connect(NULL, options);
--- End diff --
Should be `` ? `_addr.sa` was always `NULL` in here ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #825: TS-4694: Some refactoring after SPDY is rem...
Github user masaori335 closed the pull request at: https://github.com/apache/trafficserver/pull/825 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #825: TS-4694: Some refactoring after SPDY is rem...
Github user masaori335 commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/825#discussion_r7506 --- Diff: proxy/ProxyClientTransaction.h --- @@ -211,7 +211,11 @@ class ProxyClientTransaction : public VConnection virtual bool allow_half_open() const = 0; - virtual const char *get_protocol_string() const = 0; + virtual const char * --- End diff -- Currently plugins can not set client_protocol of HttpSM ( which is going to be added by this patch ), but it could be set in the future. And it also has consistency for other stats in HttpSM (e.g. client_sec_protocol, client_cipher_suite). So, let's leave it string. I'll file a new tickets for optimization. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #825: TS-4694: Some refactoring after SPDY is rem...
Github user masaori335 commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/825#discussion_r72179084 --- Diff: proxy/ProxyClientTransaction.h --- @@ -211,7 +211,11 @@ class ProxyClientTransaction : public VConnection virtual bool allow_half_open() const = 0; - virtual const char *get_protocol_string() const = 0; + virtual const char * --- End diff -- Yeah, it's better to reduce strlen calls. I start thinking about returning int or enum from here (something like `TS_SSN_PROTOCOL_INDEX_HTTP_x_x`) and convert it to string in logging phase. Do we really need to return string at here? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #825: TS-4694: Some refactoring after SPDY is rem...
GitHub user masaori335 opened a pull request: https://github.com/apache/trafficserver/pull/825 TS-4694: Some refactoring after SPDY is removed - Remove PluginIdentity class from base classes of Http2ClientSession - Add get_protocol_string() to ProxyClientSession to identify if the session is HTTP/2 or HTTP/1.x - Add "client_protocol" to HttpSM to track client protocol versions - Drop HTTP/0.9 support from cqpv (HTTP/0.9 is already dropped by TS-3327) You can merge this pull request into a Git repository by running: $ git pull https://github.com/masaori335/trafficserver ts-4694 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/825.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #825 commit 4a9c564ff7727cab7929a29e38f5ff5ae0c62c62 Author: Masaori Koshiba <masa...@apache.org> Date: 2016-07-22T06:17:50Z TS-4694: Some refactoring after SPDY is removed - Remove PluginIdentity class from base classes of Http2ClientSession - Add get_protocol_string() to ProxyClientSession to identify if the session is HTTP/2 or HTTP/1.x - Add "client_protocol" to HttpSM to track client protocol versions - Drop HTTP/0.9 support from cqpv (HTTP/0.9 is already dropped by TS-3327) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
