GitHub user maskit opened an issue:
https://github.com/apache/trafficserver/issues/1627
TSHttpTxnAborted is not documented
TSHttpTxnAborted is not documented.
It returns TSReturnCode (TS_SUCCESS | TS_ERROR), and it's very unclear
whether a transaction is aborted
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1612
@bryancall Looks good to me, but I don't see which part was the bug.
---
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
GitHub user maskit opened an issue:
https://github.com/apache/trafficserver/issues/1607
Some headers are not sent on 408 timeout responses
Date, Via and Server header are not sent on 408 timeout response.
Currently, only Connection header is sent.
`HttpTransact
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1604
Please add Makefile.am.
---
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
Github user maskit commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1583#discussion_r107818082
--- Diff: proxy/http/HttpSM.cc ---
@@ -2771,7 +2771,8 @@ HttpSM::tunnel_handler_post(int event, void *data)
}
if (event
Github user maskit commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1601#discussion_r107577489
--- Diff: example/protocol/Protocol.c ---
@@ -122,33 +122,29 @@ TSPluginInit(int argc, const char *argv[])
server_port = 4666
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1583
@oknet The assert has gone. At least it works with H2. 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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1583
I might be able to reproduce the issue, but only with H2. I couldn't
reproduce it with H1.
What I did are:
- Add sleep(5) into consume() of PostBuffer plugin
- Set
GitHub user maskit opened an issue:
https://github.com/apache/trafficserver/issues/1591
Advertised header table size is not used
Current implementation doesn't use the advertised header table size.
To reproduce:
```
$ nghttp -nv -c 1024 https://localhost
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1502
[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
Github user maskit closed the pull request at:
https://github.com/apache/trafficserver/pull/1590
---
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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1502
@zhilhuan The build issue is already fixed on master but the commit
(c251f2ba72d0520d25f8099af78a540f1586f55b) is not in your branch, I think.
Could you rebase again? Then all builds should
GitHub user maskit opened a pull request:
https://github.com/apache/trafficserver/pull/1590
Fix a wrong option charactar for access_key
The character doesn't match with line 536
```
{const_cast("access_key"), required_argument, nullptr, 'a'},
```
You can merge
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1502
[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
Github user maskit closed the pull request at:
https://github.com/apache/trafficserver/pull/1585
---
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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1585
[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
GitHub user maskit opened a pull request:
https://github.com/apache/trafficserver/pull/1585
Reset inactive timeout everytime H2 DATA frames are sent
The timer wasn't reset if transmission was resumed by WINDOW_UPDATE frames.
You can merge this pull request into a Git repository
Github user maskit closed the issue at:
https://github.com/apache/trafficserver/issues/1399
---
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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/issues/1399
Fixed on 6.2.x, 7.1.x and master.
---
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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1426
@zwoop Yes, we should. I wrote the reason on #1399.
---
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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1534
I can see "h2" if I build it on Linux with gcc but not on Mac with clang.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitH
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1557
Having one plugin for multiple compression algorithms is fine with me.
However, I'm not a big fun of adding it with "else if" statements, though we
already did that for deflate. It
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1534
It looks Http2ClientSession::PROTO_TAG is empty to me. I see two spaces
between "http/1.1" and "tls/1.2".
---
If your project is set up for it, you can reply to this
Github user maskit commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1534#discussion_r104723567
--- Diff: proxy/http/HttpTransactHeaders.cc ---
@@ -685,6 +687,23 @@
HttpTransactHeaders::insert_server_header_in_response(const char *server_tag
Github user maskit commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1534#discussion_r104721541
--- Diff: proxy/http/HttpSM.cc ---
@@ -8057,14 +8059,13 @@ HttpSM::is_redirect_required()
// Fill in the client protocols used. Return
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1537
Could you also update the documentation?
```
$ git grep basic-auth.c doc
doc/developer-guide/plugins/example-plugins/basic-authorization/index.en.rst:The
sample basic
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/issues/1500
previous discussion: https://issues.apache.org/jira/browse/TS-4277
---
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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/issues/1478
It seems use of transform plugins (e.g. gzip plugin) cause this issue.
I'm not sure why range requests matter here.
https://github.com/apache/trafficserver/blob/7.1.x/proxy/http
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1526
The fix seems reasonable, but why 7.1.1? It should be 7.2.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
Github user maskit commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1520#discussion_r104074965
--- Diff: proxy/http/HttpTransactHeaders.cc ---
@@ -741,26 +741,14 @@
HttpTransactHeaders::insert_via_header_in_request(HttpTransact::State *s
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/issues/1506
Just FYI:
https://issues.apache.org/jira/browse/TS-4457
https://github.com/apache/trafficserver/pull/1066
---
If your project is set up for it, you can reply to this email and have
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1485
[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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1485
@zizhong Could you include a reference for the original commit in your
commit message?
https://cwiki.apache.org/confluence/display/TS/Git#Git-Cherry-Pick
---
If your project is set up
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/issues/1448
@ykopel Could you send the patch as a pull request so we can run our CI
build with your patch?
I'm not familiar with transparent mode but the motivation and fix sound
reasonable to me
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/issues/1399
@shenzhang920 Thank you for the pull-requests and your patience.
The fix has been merged on master and 6.2.x.
@zwoop This is not a critical issue but I think we should backport
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/issues/1425
`config_dir` is a read-only configuration. Some of configurations are not
configurable at runtime.
https://docs.trafficserver.apache.org/en/latest/admin-guide/files
Github user maskit closed the pull request at:
https://github.com/apache/trafficserver/pull/1432
---
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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1410
Done.
This pull-request includes only TS-4665 now, since TS-4729 has been
backported as a part of #1434.
---
If your project is set up for it, you can reply to this email and have your
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1436
[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
GitHub user maskit opened an issue:
https://github.com/apache/trafficserver/issues/1431
CI builds fail on some platforms
Our CI builds fail on CentOS6 and Ubuntu12 because of an error occurred in
check-unused-dependencies script.
The reason of the error seems
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1430
#1399
---
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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1429
OK, got it. Thanks.
There are some ways to modify / replace commits. You could use `git commit
--amend`, or reset the last commit and start the cherry-pick all over again.
You'll
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1430
[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
Github user maskit closed the pull request at:
https://github.com/apache/trafficserver/pull/1428
---
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
GitHub user maskit opened a pull request:
https://github.com/apache/trafficserver/pull/1428
Fix a build issue that use of undeclared variables
The change made with #1426 causes build error on some platforms which
doesn't support congestion control. I fixed the issue and changed
Github user maskit closed the pull request at:
https://github.com/apache/trafficserver/pull/1426
---
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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1415
@shenzhang920 Thank you for the new pull-request. You can just close this
pull request.
After merging the new pull request, you can open a new one for 6.2.x with
cherry-picked commit
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1426
[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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1415
@shenzhang920 This should be fixed on master too, right? If there's the
same issue on master, we should fix it on master first, and then we can
cherry-pick the commit for 6.2.x.
Could you
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1415
[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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1415
@shenzhang920 I understand why you put the blocks there because the logic
was originally in `new_connection()`. However, I'm not a big fun of putting the
same code into two (or more) places
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1410
Re-pushed with Conflicts info.
---
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
GitHub user maskit opened a pull request:
https://github.com/apache/trafficserver/pull/1410
6.2.x: TS-4729 and TS-4665: H2 not terminating stream with short chunked
response
Backporting #888. Just cherry-picked the two commits, and resolved few
conflicts.
You can merge this pull
GitHub user maskit opened a pull request:
https://github.com/apache/trafficserver/pull/1409
Log crc field and psql field correctly with H2
The two field values were logged incorrectly if a resopnse body is short.
Because a write VIO isn't pass to a consumer if whole
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1365
I think it should be backported to 7.1.x because returning NULL seems to be
a sort of API breakage if old versions didn't return NULL in the same situation.
---
If your project is set up
Github user maskit commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1365#discussion_r97994347
--- Diff: proxy/ProxyClientSession.h ---
@@ -214,6 +214,19 @@ class ProxyClientSession : public VConnection
ink_hrtime ssn_start_time
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1368
I'm +1 on backporting to 7.1.x.
I took a quick look and it seems to be easy, `git apply` fails though.
---
If your project is set up for it, you can reply to this email and have your
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/issues/1367
Yes, I solved the problem, but the coalescing issue is still alive. This
kind of bugs can be made again until we get some measure for it.
---
If your project is set up for it, you can
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/888
@jaaju Unfortunately, this hasn't backported to 6.x yet, and it wouldn't be
in 6.2.1 because the release is almost there (RC0 passed a voting).
I'm going to try to backport in a couple
Github user maskit commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1342#discussion_r97648065
--- Diff: plugins/experimental/money_trace/money_trace.cc ---
@@ -120,17 +120,13 @@ mt_check_request_header(TSHttpTxn txnp)
if (!hdr_value
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/issues/1367
I faced a similar problem over 2 years ago.
https://issues.apache.org/jira/browse/TS-2792
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user maskit commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1067#discussion_r97269280
--- Diff: proxy/http/HttpTransact.cc ---
@@ -7948,9 +7948,11 @@ HttpTransact::build_response(State *s, HTTPHdr
*base_response, HTTPHdr *outgoing
Github user maskit commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1342#discussion_r97264664
--- Diff: plugins/experimental/money_trace/money_trace.cc ---
@@ -120,17 +120,13 @@ mt_check_request_header(TSHttpTxn txnp)
if (!hdr_value
Github user maskit commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1262#discussion_r94286274
--- Diff: proxy/http2/Http2ConnectionState.cc ---
@@ -728,18 +732,18 @@ rcv_continuation_frame(Http2ConnectionState ,
const Http2Frame
Github user maskit commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1262#discussion_r94285648
--- Diff: proxy/http2/Http2ConnectionState.cc ---
@@ -127,10 +128,11 @@ rcv_data_frame(Http2ConnectionState , const
Http2Frame )
// Check
Github user maskit commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1262#discussion_r94285819
--- Diff: proxy/http2/Http2ConnectionState.cc ---
@@ -683,8 +686,10 @@ rcv_continuation_frame(Http2ConnectionState ,
const Http2Frame
Github user maskit commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1262#discussion_r94286295
--- Diff: proxy/http2/Http2ConnectionState.cc ---
@@ -301,11 +304,11 @@ rcv_headers_frame(Http2ConnectionState , const
Http2Frame
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/1160
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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/833
Because I thought it's the easiest way. I could parse query parameters and
use a specific parameter as a trigger but it needs more codes. I wanted to keep
the example plugin simple. I'd happy
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/833
@bryancall That's odd. I just rebased again but I don't see any problems.
> After removing TSUrlHttpQueryGet() from the plugin I am seeing many
server pushes for the URL in the l
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/972
It 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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/999
Looks OK. I understand the necessity of doing this but I'm not thrilled
adding more state flags to Http2Stream class. I can live with them but I hope
they are managed in `_state` in the future
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/833
It works 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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/833
It doesn't work correctly after the rebasing. I'm working on 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
Github user maskit closed the pull request at:
https://github.com/apache/trafficserver/pull/913
---
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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/913
Rebased to obtain the functions from master.
---
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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/833
Rebased for now. We still needs discuss about `url` argument.
---
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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/951
:+1:
---
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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/951
As I commented inline, this introduces an unexpected behavior. I suggest
using the conservative change I wrote on the comment.
Also, it's a trivial thing but I'd suggest using &quo
Github user maskit commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/951#discussion_r77433626
--- Diff: plugins/header_rewrite/parser.cc ---
@@ -50,6 +52,12 @@ Parser::Parser(const std::string ) : _cond(false),
_empty(false
Github user maskit closed the pull request at:
https://github.com/apache/trafficserver/pull/921
---
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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/921
Waiting +1s.
---
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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/921
@jacksontj Could you take a look at this please? Because the line seems to
be added with your PR #653.
---
If your project is set up for it, you can reply to this email and have your
reply
GitHub user maskit opened a pull request:
https://github.com/apache/trafficserver/pull/921
TS-4776: Emulate the effect of O_DIRECTORY if it is not defined
Emulate the effect of `O_DIRECTORY` with `stat()` if it is not defined.
According to man page of `open()`, it returns `ENOTDIR
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/913
CI builds fail because the functions are not in the code yet.
---
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
GitHub user maskit opened a pull request:
https://github.com/apache/trafficserver/pull/913
TS-2237: Add unit tests for escapify_url and pure_escapify_url
Unit tests for #866
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/maskit
Github user maskit closed the pull request at:
https://github.com/apache/trafficserver/pull/886
---
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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/907
ð
---
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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/895
I'm on the same page with @zwoop . But 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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/888
Reviewed quickly. Looks ok, but I'm not sure about this change. +0 from me.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user maskit closed the pull request at:
https://github.com/apache/trafficserver/pull/885
---
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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/866
ð
---
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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/885
Updated.
- Use PATH_MAX
- Use ink_strlcat
---
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
GitHub user maskit opened a pull request:
https://github.com/apache/trafficserver/pull/886
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
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/866
> are you ok with the change if we make a new version of
LogUtils::escapify_url that does not include this change that is used by
TSStringPercentEncode? So the change only affects core lo
GitHub user maskit opened a pull request:
https://github.com/apache/trafficserver/pull/885
TS-4546: Fix a build error on OmniOS
https://issues.apache.org/jira/browse/TS-4546
OmniOS doesn't have d_type in dirent structure, so use stat() instead.
You can merge this pull request
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/866
I'd say it is not sufficient.
As a workaround for most cases, it works, and personally I'm OK with it as
is. However, I think the behavior of `TSStringPercentEncode` shouldn't
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/866
``HttpRequestData::get_string()`` unescapes an URL and it is called from
``UrlMatcher<Data, Result>::Match(RequestData *rdata, Result *result)``. I
think this is the code Sudheer men
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/866
I wrote a unit test for CURRENT `escapify_url`.
http://pastebin.com/XZ4x8bKg
With your change, you would need to change the last expected value in the
test cases.
---
If your
Github user maskit commented on the issue:
https://github.com/apache/trafficserver/pull/866
I don't think this is the right approach. With this change, "%%20" will be
encoded to "%25%20", right? What if "%%20" was not encoded string? It should be
encode
1 - 100 of 111 matches
Mail list logo
