GitHub user shukitchan opened a pull request:
https://github.com/apache/trafficserver/pull/798
TS-4653: esi plugin - disable HTTP_COOKIE variable by default and impâ¦
â¦lement a whitelist mechanism to allow the specified cookies for it
Original code and idea contributed
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/798
[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
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/798
@struct , i can add the wildcard support tonight.
e.g. whitelistCookie *
That will mean we allow all cookies when we are using HTTP_COOKIE esi
variables
---
If your
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/798
@zwoop can you take a look at this as 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
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/798
wildcard support is added.
---
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 shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/779
@jpeach , I have done the cleanup according to the comments. please take a
look. thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/798
@bryancall / @jpeach / @zwoop any comments here? 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 shukitchan commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/798#discussion_r71086266
--- Diff: plugins/experimental/esi/esi.cc ---
@@ -61,6 +61,7 @@ struct OptionInfo {
};
static HandlerManager *gHandlerManager = NULL
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/798
@zwoop , i just did the cleanup. Good to 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
Github user shukitchan commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/779#discussion_r71105108
--- Diff: plugins/experimental/ts_lua/ts_lua.c ---
@@ -140,13 +141,15 @@ TSRemapDoRemap(void *ih, TSHttpTxn rh,
TSRemapRequestInfo *rri
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/779
@jpeach cleanup is done accordingly. pls review.
---
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
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1065
it looks fine.
---
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
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1065
[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
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1064
i think back port is done differently.
---
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
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1064
[ci approve]
---
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
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1064
we should probably close #1065
---
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 shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1064
the change 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
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1065
pls state in the jira that it need a backport instead.
---
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 shukitchan closed the pull request at:
https://github.com/apache/trafficserver/pull/1065
---
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
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1064
ð
---
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
Github user shukitchan closed the pull request at:
https://github.com/apache/trafficserver/pull/1064
---
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
GitHub user shukitchan opened a pull request:
https://github.com/apache/trafficserver/pull/1089
TS-4592: Add support for process ID and txn ID in ts_lua
@zwoop please take a look and review. Thanks.
You can merge this pull request into a Git repository by running:
$ git pull
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1089
[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
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1089
@zwoop it should be good now. Please take a look.
And of course I will squash before merge.
---
If your project is set up for it, you can reply to this email and have your
reply
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1089
@zwoop & @jpeach , it is updated according to the comment. Please review
again. Thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appea
Github user shukitchan closed the pull request at:
https://github.com/apache/trafficserver/pull/1089
---
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
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1122
ð
looks good. data_ptr is simply (and redundantly) for reporting error. And
we don't actually need it.
---
If your project is set up for it, you can reply to this emai
GitHub user shukitchan opened a pull request:
https://github.com/apache/trafficserver/pull/1127
[TS-4990] support new apis in ts_lua
support new apis in ts_lua
@jpeach pls take a look and review. Thanks.
You can merge this pull request into a Git repository by running
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1127
@jpeach pls take a look if you get a chance. 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
GitHub user shukitchan opened a pull request:
https://github.com/apache/trafficserver/pull/1158
TS-5015: Convert storage configuration file to Lua
documentation is TBD. Also I want to clean up a bit since it is messy.
@jpeach, pls take a look and review.
You can merge this
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1158
[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
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1127
@jpeach, I have updated the PR according to the comments. Please take a
look.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1158
After the summit, here are the things I learn I should do
1) Add a flag to allow backward compatibility.
2) combine volume.config and hosting.config as well in this new lua
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1127
thanks. will squash 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
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1183
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
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1182
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
Github user shukitchan closed the pull request at:
https://github.com/apache/trafficserver/pull/1127
---
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
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1183
yeah... just one extra empty line that clang-format does not like.
---
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 shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1183
[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
GitHub user shukitchan opened a pull request:
https://github.com/apache/trafficserver/pull/1201
TS-4994: make state configuration in ts_lua plugin
@jpeach pls take a look when you have time.
You can merge this pull request into a Git repository by running:
$ git pull https
GitHub user shukitchan opened a pull request:
https://github.com/apache/trafficserver/pull/1217
TS-5041: fix resource leak in ts_lua
@zwoop please take a look. Thanks.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/shukitchan
Github user shukitchan closed the pull request at:
https://github.com/apache/trafficserver/pull/1217
---
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
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/issues/1251
If you called TSSkipRemappingSet() after doing those Set functions, then
the remap related functionality will be skipped.
---
If your project is set up for it, you can reply to this
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/issues/1251
is it only ts-lua plugin not working? or all remap plugins are not working?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/issues/1251
i mean if you use other remap plugins in your remap rule (e.g. conf_remap)
, do they also not work?
just trying to see if it is a ts_lua specific problem or not.
---
If your
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/issues/1251
oh. also if you set the host/port in read request hook (i.e. before mapping
happens), it may not match the second line at all (the one with the lua
plugin).
It may leverage the
Github user shukitchan closed the issue at:
https://github.com/apache/trafficserver/issues/1251
---
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 shukitchan closed the pull request at:
https://github.com/apache/trafficserver/pull/779
---
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
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/798
@sudheerv can you help to do a review for 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
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/798
updated just now. will merge after squashing commits into one. 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
Github user shukitchan closed the pull request at:
https://github.com/apache/trafficserver/pull/798
---
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
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/829
perhaps we can close TS-2987 as duplicate to TS-4703 ?
---
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 shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/829
good point. Perhaps we can do it in this PR?
---
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 shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/851
Yeah. That's indeed a good thing to finally kill off XML. I will give some
time to review it today/tomorrow.
---
If your project is set up for it, you can reply to this email and
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/851
ð
Good one. I can now have an string array of host names. Loop through them
and create log object for each of them that is slightly different from one
another.
---
If your
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/859
For new API in InkAPI.cc, please email to dev@ for an API review. Thanks.
After that I will continue to review the lua related portion.
---
If your project is set up for it, you can
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/859
I think @jpeach and me replied to your email. Did you not get the response??
Check out here - https://www.mail-archive.com/dev@trafficserver.apache.org/
---
If your project is set
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/859
oh and you need to subscribe to the dev@ mailing list to get those emails.
---
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 shukitchan commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/884#discussion_r75608137
--- Diff: doc/admin-guide/files/records.config.en.rst ---
@@ -2651,7 +2651,7 @@ Logging Configuration
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/408
@SolidWallOfCode , can you also take a look pls?
We are proposing a unit test framework for cpp api plugins. We have used
the webp transform plugin and updated it with unit tests to
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/776
@zwoop, @jacksontj and I have a short conversation a couple weeks ago. It
seemed like @jacksontj and his teammates are not pursuing the brotli works for
now.
So @bgaff, can we
Github user shukitchan commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/859#discussion_r75625639
--- Diff: proxy/InkAPI.cc ---
@@ -2208,6 +2208,20 @@ TSUrlPathSet(TSMBuffer bufp, TSMLoc obj, const char
*value, int length)
return
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/859
In general, this patch is trying to expose the "nuke_proxy_stuff"
functionality as an API. And add lua plugin API for that as well.
1) Can we achieve the same with
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/889
ð
---
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
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/859
I think it makes sense. Just make it look similar to "client_request".
I think we can close this one. And please change the title on the jira and
open a new pull reques
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/906
i think it looks good. will try it out tonight before merging
---
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 shukitchan closed the pull request at:
https://github.com/apache/trafficserver/pull/906
---
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
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/906
no worries. I fixed that already.
---
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
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1261
ð
---
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
GitHub user shukitchan opened a pull request:
https://github.com/apache/trafficserver/pull/1277
TS-5041: fix leak in ts_lua plugin
@jpeach , pls help to review
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/shukitchan
Github user shukitchan closed the pull request at:
https://github.com/apache/trafficserver/pull/1277
---
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
GitHub user shukitchan opened a pull request:
https://github.com/apache/trafficserver/pull/1278
TS-5101: fix error handling for ts_lua
@jpeach @zwoop
pls help to review
You can merge this pull request into a Git repository by running:
$ git pull https://github.com
Github user shukitchan closed the pull request at:
https://github.com/apache/trafficserver/pull/1278
---
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
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/776
discussed with Bryan.
We will submit a PR that will change the gzip plugin instead of a separate
plugin like this PR.
---
If your project is set up for it, you can reply to this
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/776
You can see @PSUdaemon comment above. That's pretty much the idea behind
the efforts of rewriting it to merge with gzip plugin.
---
If your project is set up for it, you can rep
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/776
I personally don't mind doing both. i.e. get this PR done right to merge
and also changing gzip plugin to support brotli. How does that sound?
---
If your project is set up for it
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/issues/1506
interesting. That's done in the response "Via" header. But not in the
request "Via" header.
https://github.com/apache/trafficserver/blob/master/proxy/ht
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1516
I would like to do the review on this.
---
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
Github user shukitchan commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1516#discussion_r103586416
--- Diff: plugins/esi/combo_handler.cc ---
@@ -136,6 +138,24 @@ struct InterceptData {
~InterceptData();
};
+/*
+ * This
Github user shukitchan commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1516#discussion_r103587579
--- Diff: plugins/esi/combo_handler.cc ---
@@ -136,6 +138,24 @@ struct InterceptData {
~InterceptData();
};
+/*
+ * This
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1516
I am not in favor with the "private" related changes because i think it can
catch some existing users by surprise. And we did not mention anything in the
document as wel
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1516
The max-age related code is also a bit different from before. Previously we
can have max-age larger than 31536. Now with this change, I think we can no
longer have max-age larger than
Github user shukitchan commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1516#discussion_r103589725
--- Diff: plugins/esi/combo_handler.cc ---
@@ -187,6 +207,94 @@ InterceptData::~InterceptData()
}
}
+void
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1516
will try to compile it tonight and see if i have more comments.
---
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 shukitchan commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1516#discussion_r103590528
--- Diff: plugins/esi/combo_handler.cc ---
@@ -187,6 +207,94 @@ InterceptData::~InterceptData()
}
}
+void
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1516
the patch compiles fine. And it seems to run through clang-format as well
already. thanks. pls check out the comments above.
---
If your project is set up for it, you can reply to this
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1158
it looks like .luaconf is the winner.
I will be updating this accordingly. Also will create new issue to make
sure metrics.config and logging.config are changed accordingly as well
GitHub user shukitchan opened an issue:
https://github.com/apache/trafficserver/issues/1519
Update metrics.config and logging.config to new naming convention
to metrics.luaconf and logging.luaconf accordingly.
---
If your project is set up for it, you can reply to this
GitHub user shukitchan opened a pull request:
https://github.com/apache/trafficserver/pull/1520
Report protocol in request via header
See #1506
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/shukitchan/trafficserver issue-1506
Github user shukitchan commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1520#discussion_r104076857
--- Diff: proxy/http/HttpTransactHeaders.cc ---
@@ -741,26 +741,14 @@
HttpTransactHeaders::insert_via_header_in_request(HttpTransact::State *s
Github user shukitchan commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1520#discussion_r104270172
--- Diff: proxy/http/HttpTransactHeaders.cc ---
@@ -741,26 +741,14 @@
HttpTransactHeaders::insert_via_header_in_request(HttpTransact::State *s
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1534
ð
Looks good.
Just one question. Should the internal "protocol_contains" functions return
"StringView" as well?
Obviously we should not be changin
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1534
ð
---
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
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1551
thanks for spotting the mistake.
ð
---
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
Github user shukitchan closed the pull request at:
https://github.com/apache/trafficserver/pull/1551
---
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
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1516
I think we should keep the existing behavior since this is not an
experimental plugin and we are not in the middle of releasing a major version.
---
If your project is set up for it
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1557
I will take a closer look in the coming days.
In the meanwhile, can you update the documentation for the gzip plugin as
well for this change?
---
If your project is set up for it
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1516
feature flag or feature gate should be fine.
i think it is a bug, too.
but unfortunately i think we still need to support that till the next major
release.
---
If your project
Github user shukitchan commented on the issue:
https://github.com/apache/trafficserver/pull/1520
#1534 is merged. So closing this one. Thanks @SolidWallOfCode
---
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 shukitchan closed the pull request at:
https://github.com/apache/trafficserver/pull/1520
---
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
1 - 100 of 121 matches
Mail list logo
