[jira] [Commented] (TS-3967) Set stream state to close after a RST_STEAM has been sent
[ https://issues.apache.org/jira/browse/TS-3967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15099266#comment-15099266 ] ASF subversion and git services commented on TS-3967: - Commit 1cb1426acec814210ab1b3f3ddec70cc9fbe2600 in trafficserver's branch refs/heads/6.1.x from [~maskit] [ https://git-wip-us.apache.org/repos/asf?p=trafficserver.git;h=1cb1426 ] TS-3967: Set stream state to close after a RST_STEAM has been sent This closes #389. (cherry picked from commit 8df989ce6f6b9480891bbb8c49ac2780ecaa1490) > Set stream state to close after a RST_STEAM has been sent > - > > Key: TS-3967 > URL: https://issues.apache.org/jira/browse/TS-3967 > Project: Traffic Server > Issue Type: Improvement > Components: HTTP/2 >Reporter: Bryan Call >Assignee: Masakazu Kitajo > Fix For: 6.1.0 > > Attachments: patch_for_6.1.diff, ts-3967-2.diff, ts-3967.diff > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TS-3967) Set stream state to close after a RST_STEAM has been sent
[ https://issues.apache.org/jira/browse/TS-3967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15099120#comment-15099120 ] Leif Hedstrom commented on TS-3967: --- Is there no way to cherry pick this with whatever causes the conflicts, such that we avoid a special patch just for 6.1? > Set stream state to close after a RST_STEAM has been sent > - > > Key: TS-3967 > URL: https://issues.apache.org/jira/browse/TS-3967 > Project: Traffic Server > Issue Type: Improvement > Components: HTTP/2 >Reporter: Bryan Call >Assignee: Masakazu Kitajo > Fix For: 6.2.0 > > Attachments: patch_for_6.1.diff, ts-3967-2.diff, ts-3967.diff > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TS-3967) Set stream state to close after a RST_STEAM has been sent
[ https://issues.apache.org/jira/browse/TS-3967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15091335#comment-15091335 ] Masakazu Kitajo commented on TS-3967: - [~zwoop] I was waiting a comment from [~bcall], but I've committed it on my own responsibility. > Set stream state to close after a RST_STEAM has been sent > - > > Key: TS-3967 > URL: https://issues.apache.org/jira/browse/TS-3967 > Project: Traffic Server > Issue Type: Improvement > Components: HTTP/2 >Reporter: Bryan Call >Assignee: Masakazu Kitajo > Fix For: 6.2.0 > > Attachments: ts-3967-2.diff, ts-3967.diff > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TS-3967) Set stream state to close after a RST_STEAM has been sent
[ https://issues.apache.org/jira/browse/TS-3967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15091328#comment-15091328 ] ASF GitHub Bot commented on TS-3967: Github user asfgit closed the pull request at: https://github.com/apache/trafficserver/pull/389 > Set stream state to close after a RST_STEAM has been sent > - > > Key: TS-3967 > URL: https://issues.apache.org/jira/browse/TS-3967 > Project: Traffic Server > Issue Type: Improvement > Components: HTTP/2 >Reporter: Bryan Call >Assignee: Bryan Call > Fix For: 6.2.0 > > Attachments: ts-3967-2.diff, ts-3967.diff > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TS-3967) Set stream state to close after a RST_STEAM has been sent
[ https://issues.apache.org/jira/browse/TS-3967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15091327#comment-15091327 ] ASF subversion and git services commented on TS-3967: - Commit 8df989ce6f6b9480891bbb8c49ac2780ecaa1490 in trafficserver's branch refs/heads/master from [~maskit] [ https://git-wip-us.apache.org/repos/asf?p=trafficserver.git;h=8df989c ] TS-3967: Set stream state to close after a RST_STEAM has been sent This closes #389. > Set stream state to close after a RST_STEAM has been sent > - > > Key: TS-3967 > URL: https://issues.apache.org/jira/browse/TS-3967 > Project: Traffic Server > Issue Type: Improvement > Components: HTTP/2 >Reporter: Bryan Call >Assignee: Bryan Call > Fix For: 6.2.0 > > Attachments: ts-3967-2.diff, ts-3967.diff > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TS-3967) Set stream state to close after a RST_STEAM has been sent
[ https://issues.apache.org/jira/browse/TS-3967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15091267#comment-15091267 ] Leif Hedstrom commented on TS-3967: --- Did this get committed yet? > Set stream state to close after a RST_STEAM has been sent > - > > Key: TS-3967 > URL: https://issues.apache.org/jira/browse/TS-3967 > Project: Traffic Server > Issue Type: Improvement > Components: HTTP/2 >Reporter: Bryan Call >Assignee: Bryan Call > Fix For: 6.2.0 > > Attachments: ts-3967-2.diff, ts-3967.diff > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TS-3967) Set stream state to close after a RST_STEAM has been sent
[ https://issues.apache.org/jira/browse/TS-3967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15090589#comment-15090589 ] ASF GitHub Bot commented on TS-3967: Github user maskit commented on the pull request: https://github.com/apache/trafficserver/pull/389#issuecomment-170236549 rebased to follow the changes in master > Set stream state to close after a RST_STEAM has been sent > - > > Key: TS-3967 > URL: https://issues.apache.org/jira/browse/TS-3967 > Project: Traffic Server > Issue Type: Improvement > Components: HTTP/2 >Reporter: Bryan Call >Assignee: Bryan Call > Fix For: 6.2.0 > > Attachments: ts-3967-2.diff, ts-3967.diff > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TS-3967) Set stream state to close after a RST_STEAM has been sent
[ https://issues.apache.org/jira/browse/TS-3967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15086851#comment-15086851 ] Masaori Koshiba commented on TS-3967: - This patch looks good to me. I don't see any errors in tests with h2spec-v1.2.0. Also, no errors with clang-analyzer. Let's land this patch in 6.1.0. > Set stream state to close after a RST_STEAM has been sent > - > > Key: TS-3967 > URL: https://issues.apache.org/jira/browse/TS-3967 > Project: Traffic Server > Issue Type: Improvement > Components: HTTP/2 >Reporter: Bryan Call >Assignee: Bryan Call > Fix For: 6.1.0 > > Attachments: ts-3967-2.diff, ts-3967.diff > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TS-3967) Set stream state to close after a RST_STEAM has been sent
[ https://issues.apache.org/jira/browse/TS-3967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15085699#comment-15085699 ] Leif Hedstrom commented on TS-3967: --- [~bcall] [~masaori] Any update on this? Should we move out to 6.2.0? > Set stream state to close after a RST_STEAM has been sent > - > > Key: TS-3967 > URL: https://issues.apache.org/jira/browse/TS-3967 > Project: Traffic Server > Issue Type: Improvement > Components: HTTP/2 >Reporter: Bryan Call >Assignee: Bryan Call > Fix For: 6.1.0 > > Attachments: ts-3967-2.diff, ts-3967.diff > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TS-3967) Set stream state to close after a RST_STEAM has been sent
[
https://issues.apache.org/jira/browse/TS-3967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15066098#comment-15066098
]
ASF GitHub Bot commented on TS-3967:
Github user masaori335 commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/389#discussion_r48120660
--- Diff: proxy/http2/HTTP2.cc ---
@@ -665,11 +666,19 @@ http2_decode_header_blocks(HTTPHdr *hdr, const
uint8_t *buf_start, const uint8_t
}
}
+// turn on that we have a trailer header
+char trailer_name[] = "trailer";
+if (name_len == (sizeof(trailer_name) - 1) && strncmp(name,
trailer_name, sizeof(trailer_name) - 1) == 0) {
+ trailing_header = true;
--- End diff --
oh, I didn't notice '&' in argument. sorry:p
> Set stream state to close after a RST_STEAM has been sent
> -
>
> Key: TS-3967
> URL: https://issues.apache.org/jira/browse/TS-3967
> Project: Traffic Server
> Issue Type: Improvement
> Components: HTTP/2
>Reporter: Bryan Call
>Assignee: Bryan Call
> Fix For: 6.1.0
>
> Attachments: ts-3967-2.diff, ts-3967.diff
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TS-3967) Set stream state to close after a RST_STEAM has been sent
[
https://issues.apache.org/jira/browse/TS-3967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15066091#comment-15066091
]
ASF GitHub Bot commented on TS-3967:
Github user maskit commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/389#discussion_r48120169
--- Diff: proxy/http2/HTTP2.cc ---
@@ -665,11 +666,19 @@ http2_decode_header_blocks(HTTPHdr *hdr, const
uint8_t *buf_start, const uint8_t
}
}
+// turn on that we have a trailer header
+char trailer_name[] = "trailer";
+if (name_len == (sizeof(trailer_name) - 1) && strncmp(name,
trailer_name, sizeof(trailer_name) - 1) == 0) {
+ trailing_header = true;
--- End diff --
`trailing_header` is for caller. It suggests existence of trailing header
to outside of this function.
`is_trailing_header` is for callee. It suggests that this block is trailing
header to skip the pseudo header checks.
> Set stream state to close after a RST_STEAM has been sent
> -
>
> Key: TS-3967
> URL: https://issues.apache.org/jira/browse/TS-3967
> Project: Traffic Server
> Issue Type: Improvement
> Components: HTTP/2
>Reporter: Bryan Call
>Assignee: Bryan Call
> Fix For: 6.1.0
>
> Attachments: ts-3967-2.diff, ts-3967.diff
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TS-3967) Set stream state to close after a RST_STEAM has been sent
[ https://issues.apache.org/jira/browse/TS-3967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15065978#comment-15065978 ] ASF GitHub Bot commented on TS-3967: Github user masaori335 commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/389#discussion_r48112070 --- Diff: proxy/http2/HTTP2.cc --- @@ -665,11 +666,19 @@ http2_decode_header_blocks(HTTPHdr *hdr, const uint8_t *buf_start, const uint8_t } } +// turn on that we have a trailer header +char trailer_name[] = "trailer"; --- End diff -- `const` ? Should be defined in HTTP2.h ? > Set stream state to close after a RST_STEAM has been sent > - > > Key: TS-3967 > URL: https://issues.apache.org/jira/browse/TS-3967 > Project: Traffic Server > Issue Type: Improvement > Components: HTTP/2 >Reporter: Bryan Call >Assignee: Bryan Call > Fix For: 6.1.0 > > Attachments: ts-3967-2.diff, ts-3967.diff > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TS-3967) Set stream state to close after a RST_STEAM has been sent
[
https://issues.apache.org/jira/browse/TS-3967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15065975#comment-15065975
]
ASF GitHub Bot commented on TS-3967:
Github user masaori335 commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/389#discussion_r48111940
--- Diff: proxy/http2/HTTP2.cc ---
@@ -665,11 +666,19 @@ http2_decode_header_blocks(HTTPHdr *hdr, const
uint8_t *buf_start, const uint8_t
}
}
+// turn on that we have a trailer header
+char trailer_name[] = "trailer";
+if (name_len == (sizeof(trailer_name) - 1) && strncmp(name,
trailer_name, sizeof(trailer_name) - 1) == 0) {
+ trailing_header = true;
--- End diff --
`is_trailing_header` ? It looks like `trailing_header` is not used in below
code.
> Set stream state to close after a RST_STEAM has been sent
> -
>
> Key: TS-3967
> URL: https://issues.apache.org/jira/browse/TS-3967
> Project: Traffic Server
> Issue Type: Improvement
> Components: HTTP/2
>Reporter: Bryan Call
>Assignee: Bryan Call
> Fix For: 6.1.0
>
> Attachments: ts-3967-2.diff, ts-3967.diff
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TS-3967) Set stream state to close after a RST_STEAM has been sent
[ https://issues.apache.org/jira/browse/TS-3967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15065974#comment-15065974 ] ASF GitHub Bot commented on TS-3967: Github user masaori335 commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/389#discussion_r48111868 --- Diff: proxy/http2/RegressionHPACK.cc --- @@ -441,6 +441,7 @@ REGRESSION_TEST(HPACK_Decode)(RegressionTest *t, int, int *pstatus) box = REGRESSION_TEST_PASSED; Http2DynamicTable dynamic_table; + bool trailing_header; --- End diff -- Should be initialised? > Set stream state to close after a RST_STEAM has been sent > - > > Key: TS-3967 > URL: https://issues.apache.org/jira/browse/TS-3967 > Project: Traffic Server > Issue Type: Improvement > Components: HTTP/2 >Reporter: Bryan Call >Assignee: Bryan Call > Fix For: 6.1.0 > > Attachments: ts-3967-2.diff, ts-3967.diff > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TS-3967) Set stream state to close after a RST_STEAM has been sent
[ https://issues.apache.org/jira/browse/TS-3967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15065425#comment-15065425 ] Masakazu Kitajo commented on TS-3967: - [~bcall] Could you review the pull request? It is based on your patch, but I added some fixes. > Set stream state to close after a RST_STEAM has been sent > - > > Key: TS-3967 > URL: https://issues.apache.org/jira/browse/TS-3967 > Project: Traffic Server > Issue Type: Improvement > Components: HTTP/2 >Reporter: Bryan Call >Assignee: Bryan Call > Fix For: 6.1.0 > > Attachments: ts-3967-2.diff, ts-3967.diff > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TS-3967) Set stream state to close after a RST_STEAM has been sent
[ https://issues.apache.org/jira/browse/TS-3967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15065424#comment-15065424 ] ASF GitHub Bot commented on TS-3967: GitHub user maskit opened a pull request: https://github.com/apache/trafficserver/pull/389 TS-3967: Set stream state to close after a RST_STEAM has been sent https://issues.apache.org/jira/browse/TS-3967 The original patches are on the JIRA ticket. I updated the patches to resolve conflicts, and made little changes to satisfy h2spec tests. You can merge this pull request into a Git repository by running: $ git pull https://github.com/maskit/trafficserver ts-3967 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/389.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 #389 commit 54f4907c0efefa943c4ea9c332cda34dd1a29426 Author: Masakazu Kitajo Date: 2015-12-19T12:52:05Z TS-3967: Set stream state to close after a RST_STEAM has been sent > Set stream state to close after a RST_STEAM has been sent > - > > Key: TS-3967 > URL: https://issues.apache.org/jira/browse/TS-3967 > Project: Traffic Server > Issue Type: Improvement > Components: HTTP/2 >Reporter: Bryan Call >Assignee: Bryan Call > Fix For: 6.1.0 > > Attachments: ts-3967-2.diff, ts-3967.diff > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TS-3967) Set stream state to close after a RST_STEAM has been sent
[
https://issues.apache.org/jira/browse/TS-3967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14965325#comment-14965325
]
Masakazu Kitajo commented on TS-3967:
-
Ok, I have no strong objection to add trailing header support, but I feel
something is wrong around state handling. I'll file another ticket if I find
any.
I found another coredump scenario. We should check the state when receiving
window update for a specific stream too.
{noformat}
[Oct 21 00:58:49.516] Server {0xb059} DEBUG: (http2_cs) [0] Received
WINDOW_UPDATE frame.
[Oct 21 00:58:49.516] Server {0xb059} DEBUG: (http2_cs) [0] Send DATA frame
Process 50096 stopped
* thread #2: tid = 0x3b9c14, 0x000100022ddc
traffic_server`FetchSM::ext_get_user_data(this=0x) + 12 at
FetchSM.cc:689, name = '[ET_SSL 0]', stop reason = EXC_BAD_ACCESS (code=1,
address=0x1d8)
frame #0: 0x000100022ddc
traffic_server`FetchSM::ext_get_user_data(this=0x) + 12 at
FetchSM.cc:689
686 void *
687 FetchSM::ext_get_user_data()
688 {
-> 689return user_data;
690 }
691
692 TSMBuffer
(lldb) bt
* thread #2: tid = 0x3b9c14, 0x000100022ddc
traffic_server`FetchSM::ext_get_user_data(this=0x) + 12 at
FetchSM.cc:689, name = '[ET_SSL 0]', stop reason = EXC_BAD_ACCESS (code=1,
address=0x1d8)
* frame #0: 0x000100022ddc
traffic_server`FetchSM::ext_get_user_data(this=0x) + 12 at
FetchSM.cc:689
frame #1: 0x0001001ae155
traffic_server`Http2ConnectionState::send_data_frame(this=0x00a32118,
fetch_sm=0x) + 437 at Http2ConnectionState.cc:904
frame #2: 0x0001001b18a9
traffic_server`rcv_window_update_frame(cs=0x00a31eb0,
cstate=0x00a32118, frame=0xb058f460) + 969 at
Http2ConnectionState.cc:584
{noformat}
> Set stream state to close after a RST_STEAM has been sent
> -
>
> Key: TS-3967
> URL: https://issues.apache.org/jira/browse/TS-3967
> Project: Traffic Server
> Issue Type: Improvement
> Components: HTTP/2
>Reporter: Bryan Call
>Assignee: Bryan Call
> Fix For: 6.1.0
>
> Attachments: ts-3967.diff
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TS-3967) Set stream state to close after a RST_STEAM has been sent
[ https://issues.apache.org/jira/browse/TS-3967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14964234#comment-14964234 ] Bryan Call commented on TS-3967: I had to add trailing header support because it was failing a h2spec test. Setting the connection to state to HTTP2_STREAM_STATE_CLOSED wont send any data frames back after that has happened. It made the trailing headers test in h2spec fail, so unfortunately they are tied together. The h2spec test without this change will get a RST_SRTREAM frame back, but unfortunately test ignores that and wants to see a response come back which ATS does at the moment. > Set stream state to close after a RST_STEAM has been sent > - > > Key: TS-3967 > URL: https://issues.apache.org/jira/browse/TS-3967 > Project: Traffic Server > Issue Type: Improvement > Components: HTTP/2 >Reporter: Bryan Call >Assignee: Bryan Call > Fix For: 6.1.0 > > Attachments: ts-3967.diff > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TS-3967) Set stream state to close after a RST_STEAM has been sent
[ https://issues.apache.org/jira/browse/TS-3967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14961825#comment-14961825 ] Masakazu Kitajo commented on TS-3967: - I reviewed the patch and ran some test. First, the patch needs at least two NULL checks, one in rcv_headers_frame() and one in send_rst_stream_frame(). We have to check the return value after calling find_stream(). Second, a stream state is set to close indeed, however, the stream won't be deleted until the connection has been closed. I assume this is expected and is left in stream_list to achieve the behavior which I quoted on TS-3958. It has to be done with care because some logic depend on return value of find_stream(). Third, it seems the patch contains (partial?) support for trailing headers (TS-3812). This may lead unexpected behavior, so it would be better to separate it if it's possible. > Set stream state to close after a RST_STEAM has been sent > - > > Key: TS-3967 > URL: https://issues.apache.org/jira/browse/TS-3967 > Project: Traffic Server > Issue Type: Improvement > Components: HTTP/2 >Reporter: Bryan Call >Assignee: Bryan Call > Fix For: 6.1.0 > > Attachments: ts-3967.diff > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
