[GitHub] trafficserver issue #1626: enable proxy.config.http.negative_revalidating_en...
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/issues/1626 What about this - .. ts:cv:: CONFIG proxy.config.http.negative_revalidating_enabled INT 0 :reloadable: :overridable: Negative revalidating allows |TS| to return stale content if revalidation to the origin fails due to network or HTTP errors. If it is enabled, than caching the negative response, the current (possibly stale) content is preserved and served. Note this is considered only on a revalidation of already cached content and failure means unable to communicate via HTTP to the origin. If the origin provides a valid response that is not considered a failure in this context. A value of ``0`` disables serving stale content and a value of ``1`` enables keeping and serving stale content if revalidation fails. .. ts:cv:: CONFIG proxy.config.http.negative_revalidating_lifetime INT 1800 How long, in seconds, to consider a stale cached document valid if If :ts:cv:`proxy.config.http.negative_revalidating_enabled` is enabled and |TS| receives a negative (``5xx`` only) response from the origin server during revalidation. --- 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 #1620: Adds a new condition, %{IP:}
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/pull/1620 You can commit it, but I'm going to modify the documents to use the correct term "IP address" instead of just "IP" which, sir, is *wrong*. --- 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 #1626: enable proxy.config.http.negative_revalidating_en...
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/issues/1626 The documentation is not completely clear. If this is enabled, then on *subsequent* requests to the failed object, ATS will revalidate the negatively cached item against the origin? And in that case, if the revalidation fails, it will serve stale content? Although that seems wrong because the cached object would be the negative reply. --- 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 #1620: Adds a new condition, %{IP:}
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1620#discussion_r109226751 --- Diff: doc/admin-guide/plugins/header_rewrite.en.rst --- @@ -291,6 +293,35 @@ INCOMING-PORT TCP port, as a decimal integer, on which the incoming client connection was made. +IP +~~ +:: + +cond %{IP:} + +This is one of four possible IPs associated with the transaction, with the --- End diff -- "IP" is "intellectual propety". I think you mean "IP addresses". --- 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 #1623: Delete dead code
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1623#discussion_r109226190 --- Diff: proxy/logging/LogBuffer.cc --- @@ -112,14 +112,6 @@ LogBufferHeader::log_filename() return addr; } -/*- - LogBuffer::LogBuffer --- End diff -- I talked with @danobi about this and my view is a local comment should be minimal and the real meat in the Architecture section of the documentation. This commentary has obviously not been maintained and I don't see it being any different in the future. At least in the normal documentation there's some hope of it being updated. --- 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 #1621: Clean up globalregisterplugin
Github user SolidWallOfCode closed the pull request at: https://github.com/apache/trafficserver/pull/1621 --- 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 #1621: Clean up globalregisterplugin
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/pull/1621 Looks good. Thanks @persiaAziz. --- 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 #1608: FTP Support
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1608#discussion_r108767725 --- Diff: proxy/http/HttpConfig.cc --- @@ -1129,6 +1129,8 @@ HttpConfig::startup() // Local Manager HttpEstablishStaticConfigLongLong(c.synthetic_port, "proxy.config.admin.synthetic_port"); + // FTP protocol enabled + HttpEstablishStaticConfigByte(c.forward_proxy_ftp_enabled, "proxy.config.ftp_enabled"); --- End diff -- Is that usable, though? TS is going to dealt with the request header by that point and already committed to HTTP or FTP. --- 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 #1618: Coverity1373100:Uninitialized member
Github user SolidWallOfCode closed the pull request at: https://github.com/apache/trafficserver/pull/1618 --- 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 #1618: Coverity1373100:Uninitialized member
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/pull/1618 Let's use member initialization. --- 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 #1614: TS-4976: Regularize plugins - query_remap
Github user SolidWallOfCode closed the pull request at: https://github.com/apache/trafficserver/pull/1614 --- 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 #1616: Fix string compare for protocol tag logic in Unix...
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/pull/1616 Dang, that's not going to work either. Because it's a prefix it's not wrong if the incoming tag is shorter than the internal tag. --- 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 #1615: Doc: Update plugin stats documentation.
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/pull/1615 I'll see what I can do, @jpeach. This already includes code from the `example/statistic` plugin. In practice double registering is normally done by avoiding the double register path but I'll see if I can make an example for that. I presume that otherwise you think the update 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 pull request #1616: Fix string compare for protocol tag logic ...
GitHub user SolidWallOfCode opened a pull request: https://github.com/apache/trafficserver/pull/1616 Fix string compare for protocol tag logic in UnixNetVConnection. Switch to using the `StringView` based `strcmp` which handles length issues. The current code is a bit weak on that (it assumes the incoming `tag` is never longer). Make the implementation not inline, it's a bit big for that. You can merge this pull request into a Git repository by running: $ git pull https://github.com/SolidWallOfCode/trafficserver i-agree-with-phil-7 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1616.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 #1616 commit 50c7bec9852e951dcdf161d6bbb508319b002342 Author: Alan M. Carroll <solidwallofc...@yahoo-inc.com> Date: 2017-03-28T20:51:53Z Fix string compare for protocol tag logic in UnixNetVConnection. --- 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 #1615: Doc: Update plugin stats documentation.
GitHub user SolidWallOfCode opened a pull request: https://github.com/apache/trafficserver/pull/1615 Doc: Update plugin stats documentation. I wanted to tell a co-worked to use the plugin stats but noticed they were not in fact well documented. You can merge this pull request into a Git repository by running: $ git pull https://github.com/SolidWallOfCode/trafficserver doc-stats Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1615.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 #1615 commit 998e986d96699a17f5c2df695ed790da05c887f2 Author: Alan M. Carroll <a...@apache.org> Date: 2017-03-28T17:32:32Z Doc: Update plugin stats documentation. --- 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 #1614: TS-4976: Regularize plugins - query_remap
GitHub user SolidWallOfCode opened a pull request: https://github.com/apache/trafficserver/pull/1614 TS-4976: Regularize plugins - query_remap Incremental work on #1114. This also include updates to the documentation about `query_remap`. You can merge this pull request into a Git repository by running: $ git pull https://github.com/SolidWallOfCode/trafficserver ts-4976-query-remap Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1614.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 #1614 commit 3c559ae50a4ebc377d8a7820c372b7a8bebb63a0 Author: Alan M. Carroll <a...@apache.org> Date: 2017-03-28T14:43:46Z TS-4976: Regularize plugins - query_remap --- 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 #1604: TS-4976: Regularize plugins - protocol_sta...
Github user SolidWallOfCode closed the pull request at: https://github.com/apache/trafficserver/pull/1604 --- 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 #1594: Early intervention APIs
Github user SolidWallOfCode closed the pull request at: https://github.com/apache/trafficserver/pull/1594 --- 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 #1497: Doc: change .svg to .png.
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/pull/1497 Replaced by #1606. --- 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 #1606: require sphinx version 1.5.1
Github user SolidWallOfCode closed the pull request at: https://github.com/apache/trafficserver/pull/1606 --- 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 #1497: Doc: change .svg to .png.
Github user SolidWallOfCode closed the pull request at: https://github.com/apache/trafficserver/pull/1497 --- 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 #1606: require sphinx version 1.5.1
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/pull/1606 Thanks, @persiaAziz. This closes #1497 - require a sufficient version of Sphinx Docs. --- 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 #1606: require sphinx version 1.5.1
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1606#discussion_r107989978 --- Diff: doc/checkvers.py --- @@ -28,9 +28,9 @@ # Check whether we have a recent version of sphinx. EPEL and CentOS are completely crazy and I don't understand their # packaging at all. The test below works on Ubuntu and places where sphinx is installed sanely AFAICT. if options.checkvers: -print 'checking for sphinx version >= 1.2... ', -# Need at least 1.2 because of some command line options stuff HRP added. -# Also 1.2 guarantees sphinx.version_info is available. +print 'checking for sphinx version >= 1.5.1... ', +# Need at least 1.5.1 to use svg +# Also 1.5.1 guarantees sphinx.version_info is available. --- End diff -- I think you made this comment inaccurate. --- 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 #1604: TS-4976: Regularize plugins - protocol_stack
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/pull/1604 Yeah, I force pushed immediately after setting up the pull request when I noticed a typo and that seems to have bollixed the PR builds. --- 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 #1604: TS-4976: Regularize plugins - protocol_sta...
GitHub user SolidWallOfCode opened a pull request: https://github.com/apache/trafficserver/pull/1604 TS-4976: Regularize plugins - protocol_stack Incremental work on #1114 plus a small documentation fixup. You can merge this pull request into a Git repository by running: $ git pull https://github.com/SolidWallOfCode/trafficserver ts-4976-protocol-stack Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1604.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 #1604 commit 0b3a4f8c30c12cc7caa972ad1caa8cdcfe64d513 Author: Alan M. Carroll <a...@apache.org> Date: 2017-03-24T12:53:13Z TS-4976: Regularize plugins - protocol_stack --- 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 #1601: TS-4976: Regularize plugins - protocol
Github user SolidWallOfCode closed the pull request at: https://github.com/apache/trafficserver/pull/1601 --- 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 #1601: TS-4976: Regularize plugins - protocol
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/pull/1601 @maskit 's comment reminds me that I also remove a number of `printf` statements which seemed to duplicate the debug output. --- 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 #1601: TS-4976: Regularize plugins - protocol
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1601#discussion_r107704029 --- Diff: example/protocol/Protocol.c --- @@ -122,33 +122,29 @@ TSPluginInit(int argc, const char *argv[]) server_port = 4666; if (argc < 3) { -TSDebug("protocol", "Usage: protocol.so accept_port server_port"); +TSDebug(PLUGIN_NAME, "Usage: protocol.so accept_port server_port"); printf("[protocol_plugin] Usage: protocol.so accept_port server_port\n"); --- End diff -- Probably not, I must have missed that one. --- 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 #1583: Set HttpSM::tunnel_handler_post to handle ...
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1583#discussion_r107681516 --- Diff: proxy/http/HttpSM.cc --- @@ -2771,7 +2771,8 @@ HttpSM::tunnel_handler_post(int event, void *data) } if (event != HTTP_TUNNEL_EVENT_DONE) { -if ((event == VC_EVENT_WRITE_COMPLETE) || (event == VC_EVENT_EOS)) { +if ((event == VC_EVENT_WRITE_COMPLETE) || (event == VC_EVENT_EOS) || (event == VC_EVENT_ERROR) || --- End diff -- @shinrich and I discussed this a bit. It's a big ugly to have these two long conditionals that are identical but not obviously so. What I would recommend is adding an inline function like `isTxnTerminalEvent` that covers `EOS`, `ERROR`, `INACTIVITY_TIMEOUT` and `INACTIVITY_TIMEOUT` and have ``` if (event == VC_EVENT_WRITE_COMPLETE || isEventTxTerminal(event)) { ``` which would be easier to understand and easier to see the set of events are the same here and on line 2784. Our opinion is this would be useful in no small number of other places in the code as well and help prevent problems like this in the future. --- 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 #1601: TS-4976: Regularize plugins - protocol
GitHub user SolidWallOfCode opened a pull request: https://github.com/apache/trafficserver/pull/1601 TS-4976: Regularize plugins - protocol Incremental work on #1114. This also updates the documentation on building a protocol plugin. In addition to changes required by the standardization of the plugin code, other errors were correct. Literal references were converted to links as much as possible. You can merge this pull request into a Git repository by running: $ git pull https://github.com/SolidWallOfCode/trafficserver ts-4976-protocol Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1601.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 #1601 commit c759ec8572d9d2f144f2a1128d70e30dcfea06d0 Author: Alan M. Carroll <a...@apache.org> Date: 2017-03-23T01:35:26Z TS-4976: Regularize plugins - protocol --- 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 #1596: TS-4976: Regularize plugins - output_heade...
Github user SolidWallOfCode closed the pull request at: https://github.com/apache/trafficserver/pull/1596 --- 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 #1594: Early intervention APIs
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1594#discussion_r107026702 --- Diff: proxy/InkAPI.cc --- @@ -9112,6 +9113,65 @@ TSSslContextDestroy(TSSslContext ctx) SSLReleaseContext(reinterpret_cast(ctx)); } +void +TSRegisterProtocolSet(TSVConn sslp, TSNextProtocolSet ps) +{ + NetVConnection *vc= reinterpret_cast(sslp); + SSLNetVConnection *ssl_vc = dynamic_cast(vc); + if (ssl_vc) { +ssl_vc->registerNextProtocolSet(reinterpret_cast(ps)); + } +} + +TSNextProtocolSet +TSUnregisterProtocol(TSNextProtocolSet protoset, const char *protocol) +{ + SSLNextProtocolSet *snps = reinterpret_cast(protoset); + if (snps) { +snps->unregisterEndpoint(protocol, nullptr); +return reinterpret_cast(snps); + } + return nullptr; +} + +TSAcceptor +TSAcceptorGet(TSVConn sslp) +{ + NetVConnection *vc= reinterpret_cast(sslp); + SSLNetVConnection *ssl_vc = dynamic_cast(vc); + return ssl_vc ? reinterpret_cast(ssl_vc->accept_object) : nullptr; +} + +extern std::vector naVec; +TSAcceptor +TSAcceptorGetbyID(int ID) +{ + Debug("ssl", "getNetAccept in INK API.cc %p", naVec.at(ID)); + return reinterpret_cast(naVec.at(ID)); +} + +int +TSAcceptorIDGet(TSAcceptor acceptor) +{ + NetAccept *na = reinterpret_cast(acceptor); + return na ? na->id : -1; +} + +int +TSAcceptorCount() +{ + return naVec.size(); +} + +// clones the protoset associated with netAccept +TSNextProtocolSet +TSGetcloneProtoSet(TSAcceptor tna) +{ + NetAccept *na = reinterpret_cast(tna); + // clone protoset + return (na && na->snpa) ? reinterpret_cast(na->snpa->cloneProtoSet()) : nullptr; --- End diff -- Style: maybe ``` return reinterpret_cast(na && na->snpa ? na->snpa->cloneProtoSet() : nullptr); ``` The key point being the type of the ternary operator is not mixed and exposed to the `return` statement. Not sure that's worth it but sometimes it can be a compilation issue. --- 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 #1594: Early intervention APIs
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1594#discussion_r107020509 --- Diff: iocore/net/SSLNetVConnection.cc --- @@ -1338,9 +1338,8 @@ SSLNetVConnection::sslClientHandShakeEvent(int ) } void -SSLNetVConnection::registerNextProtocolSet(const SSLNextProtocolSet *s) +SSLNetVConnection::registerNextProtocolSet(SSLNextProtocolSet *s) --- End diff -- Let's change this to `assignNextProtocolSet` --- 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 #1594: Early intervention APIs
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1594#discussion_r106937502 --- Diff: proxy/InkAPI.cc --- @@ -9112,6 +9113,66 @@ TSSslContextDestroy(TSSslContext ctx) SSLReleaseContext(reinterpret_cast(ctx)); } +void +TSRegisterProtocolSet(TSVConn sslp, TSNextProtocolSet ps) +{ + NetVConnection *vc= reinterpret_cast(sslp); + SSLNetVConnection *ssl_vc = dynamic_cast(vc); + if (ssl_vc) { +ssl_vc->clearnpnSet(); --- End diff -- I discussed this with @persiaAziz and we concluded the `assert` is no longer useful given that the purpose of this API is to change the value even if it's already set. Therefore these two methods should be unified and the `assert` removed. --- 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 #1596: TS-4976: Regularize plugins - output_heade...
GitHub user SolidWallOfCode opened a pull request: https://github.com/apache/trafficserver/pull/1596 TS-4976: Regularize plugins - output_header Incremental work on #1114. You can merge this pull request into a Git repository by running: $ git pull https://github.com/SolidWallOfCode/trafficserver ts-4976-output-header Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1596.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 #1596 commit 9a8226c9cc10f7e97c3139cfcfc03abee30e6dc5 Author: Alan M. Carroll <a...@apache.org> Date: 2017-03-18T21:34:01Z TS-4976: Regularize plugins - output_header --- 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 #1595: TS-4976: Regularize plugins - lifecycle
Github user SolidWallOfCode closed the pull request at: https://github.com/apache/trafficserver/pull/1595 --- 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 #1594: Early intervention APIs
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/pull/1594 The regression tests are dumping core, obviously that needs to be 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 #1594: Early intervention APIs
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1594#discussion_r106767390 --- Diff: proxy/http/HttpProxyServerMain.cc --- @@ -214,7 +215,7 @@ MakeHttpProxyAcceptor(HttpProxyAcceptor , HttpProxyPort , unsigned SCOPED_MUTEX_LOCK(lock, ssl_plugin_mutex, this_ethread()); ssl_plugin_acceptors.push(ssl); - +ssl->port= --- End diff -- Probably a bad name, do to punning (e.g. is this a TCP port?). "proxy_port" is a better 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 #1594: Early intervention APIs
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1594#discussion_r106767260 --- Diff: proxy/InkAPI.cc --- @@ -9112,6 +9113,66 @@ TSSslContextDestroy(TSSslContext ctx) SSLReleaseContext(reinterpret_cast(ctx)); } +void +TSRegisterProtocolSet(TSVConn sslp, TSNextProtocolSet ps) +{ + NetVConnection *vc= reinterpret_cast(sslp); + SSLNetVConnection *ssl_vc = dynamic_cast(vc); + if (ssl_vc) { +ssl_vc->clearnpnSet(); --- End diff -- As noted above, do we need to clear and then set, instead of just set? --- 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 #1594: Early intervention APIs
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1594#discussion_r106767060 --- Diff: iocore/net/P_SSLNetVConnection.h --- @@ -259,6 +260,12 @@ class SSLNetVConnection : public UnixNetVConnection /// Set by asynchronous hooks to request a specific operation. SslVConnOp hookOpRequested; + void + clearnpnSet() --- End diff -- The naming should be consistent with `registerNextProtocolSet` since they are inverses of each other. In fact, it might be reasonable to have only one and pass `nullptr` to clear it (e.g., just `setNextProtocolSet`). --- 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 #1594: Early intervention APIs
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1594#discussion_r106766812 --- Diff: iocore/net/P_SSLNetVConnection.h --- @@ -295,7 +302,7 @@ class SSLNetVConnection : public UnixNetVConnection HANDSHAKE_HOOKS_DONE } sslHandshakeHookState; - const SSLNextProtocolSet *npnSet; + mutable SSLNextProtocolSet *npnSet; --- End diff -- Does this need to be `mutable`? Where does a `const SSLNetVConnection` have its NPN set modified? --- 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 #1595: TS-4976: Regularize plugins - lifecycle
GitHub user SolidWallOfCode opened a pull request: https://github.com/apache/trafficserver/pull/1595 TS-4976: Regularize plugins - lifecycle Incremental work on #1114. You can merge this pull request into a Git repository by running: $ git pull https://github.com/SolidWallOfCode/trafficserver ts-4976-lifecycle Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1595.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 #1595 commit 00bcd87baaf3b23de05640d2751b777aaf8d16ca Author: Alan M. Carroll <a...@apache.org> Date: 2017-03-17T23:18:37Z TS-4976: Regularize plugins - lifecycle --- 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 #1592: TS-4976: Regularize plugins - hello
Github user SolidWallOfCode closed the pull request at: https://github.com/apache/trafficserver/pull/1592 --- 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 #1592: TS-4976: Regularize plugins - hello
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/pull/1592 All the changes are related to the hello plugin or its use in the documentation. I will try to be more verbose in the future, however. --- 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 #1497: Doc: change .svg to .png.
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/pull/1497 I think I will update to require Sphinx 1.5.1 instead. That's in general availability. --- 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 #1547: Fix ssl hook state logic.
Github user SolidWallOfCode closed the pull request at: https://github.com/apache/trafficserver/pull/1547 --- 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 #1502: issue #1501: reconstruct to load the defau...
Github user SolidWallOfCode closed the pull request at: https://github.com/apache/trafficserver/pull/1502 --- 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 #1592: TS-4976: Regularize plugins - hello
GitHub user SolidWallOfCode opened a pull request: https://github.com/apache/trafficserver/pull/1592 TS-4976: Regularize plugins - hello Incremental work on #1114. You can merge this pull request into a Git repository by running: $ git pull https://github.com/SolidWallOfCode/trafficserver ts-4976-hello Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1592.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 #1592 commit c12e6c144e72f1febced520a2e7c917f12f558d3 Author: Alan M. Carroll <a...@apache.org> Date: 2017-03-17T15:33:24Z TS-4976: Regularize plugins - hello --- 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 #1581: TS-4976: Regularize plugins - file
Github user SolidWallOfCode closed the pull request at: https://github.com/apache/trafficserver/pull/1581 --- 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 #1567: [RFC] Diags scrubbing mechanism
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1567#discussion_r106505497 --- Diff: lib/ts/Diags.h --- @@ -226,6 +242,10 @@ class Diags mutable ink_mutex tag_table_lock; // prevents reconfig/read races DFA *activated_tags[2]; // 1 table for debug, 1 for action + // Scrubbing variables -- replaces predefined regexp matches with the replacement string + bool scrub_enabled = false; + Scrubber *scrubber = nullptr; --- End diff -- My long range plan is KIWF on `Vec.h` so we'll live with this 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 #1567: [RFC] Diags scrubbing mechanism
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1567#discussion_r106505205 --- Diff: proxy/logging/LogObject.h --- @@ -299,6 +299,10 @@ class LogObject : public RefCountObj unsigned m_buffer_manager_idx; LogBufferManager *m_buffer_manager; + // scrubbing data -- helps us scrub buffers of sensitive data + bool scrub_enabled = false; --- End diff -- Hmmm, I'd need to think about if that make sense. --- 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 #1567: [RFC] Diags scrubbing mechanism
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/pull/1567 Looking better. --- 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 #1567: [RFC] Diags scrubbing mechanism
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1567#discussion_r106482070 --- Diff: proxy/logging/LogObject.h --- @@ -299,6 +299,10 @@ class LogObject : public RefCountObj unsigned m_buffer_manager_idx; LogBufferManager *m_buffer_manager; + // scrubbing data -- helps us scrub buffers of sensitive data + bool scrub_enabled = false; --- End diff -- Are both of these needed? Does not `scubber` suffice by being null or not null? --- 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 #1567: [RFC] Diags scrubbing mechanism
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1567#discussion_r106481662 --- Diff: lib/ts/Scrubber.h --- @@ -0,0 +1,89 @@ +/** @file + + The Scrubber class helps replace patterns of text inside a buffer with + replacement text. + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#pragma once +#include +#include "pcre.h" + +#include "ts/ink_assert.h" +#include "ts/ink_memory.h" +#include "ts/MemView.h" + +struct Scrub { + ~Scrub() + { +if (compiled_re) { + pcre_free(const_cast(compiled_re)); +} + } + static const int OVECCOUNT = 30; + ts::StringView pattern; + ts::StringView replacement; + const pcre *compiled_re; + int ovector[OVECCOUNT]; +}; + +/* + * Class that helps scrub specific strings from buffers + */ +class Scrubber +{ +public: + /* + * Parses config & constructs Scrubber + */ + Scrubber(const char *config); + Scrubber(Scrubber ) = delete; + ~Scrubber(); + + /* + * Add another expression to scrub for + * + * @returns whether or not the addition was successful + */ + bool scrub_add(const ts::StringView pattern, const ts::StringView replacement); + + /* + * Scrubs the buffer in place with all the Scrub objects stored in this class. + */ + void scrub_buffer(char *buffer) const; + + /* + * Config getter. Caller should NOT free + */ + char * + get_config() + { +return config; + }; + +private: + /* + * Scrubs the buffer in place with the passed in Scrub object + */ + void scrub_buffer(char *buffer, Scrub *scrub) const; + + char *config = nullptr; --- End diff -- I recommend using `ats_scoped_str` here, to avoid having to explicitly clean up. --- 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 #1567: [RFC] Diags scrubbing mechanism
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1567#discussion_r106481402 --- Diff: lib/ts/Scrubber.h --- @@ -0,0 +1,89 @@ +/** @file + + The Scrubber class helps replace patterns of text inside a buffer with + replacement text. + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#pragma once +#include +#include "pcre.h" + +#include "ts/ink_assert.h" +#include "ts/ink_memory.h" +#include "ts/MemView.h" + +struct Scrub { + ~Scrub() + { +if (compiled_re) { + pcre_free(const_cast(compiled_re)); +} + } + static const int OVECCOUNT = 30; --- End diff -- Where does `30` come from? POOMA? --- 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 #1567: [RFC] Diags scrubbing mechanism
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1567#discussion_r106480902 --- Diff: lib/ts/Scrubber.cc --- @@ -0,0 +1,201 @@ +/** @file + + Implementation for Scrubber.h + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#include "Scrubber.h" + +Scrubber::Scrubber(const char *config) +{ + bool state_expecting_regex = true; + const ts::StringView delimiters("->;,", ts::StringView::literal); + ts::StringView regex; + ts::StringView replacement; + ts::StringView text(config); + + this->config = ats_strdup(config); + + // Loop over config string while extracting tokens + while (text) { +ts::StringView token = (text.ltrim()).extractPrefix(); +if (token) { + // If we hit a delimiter, we change states and skip the token + if (!ts::StringView(token).ltrim(delimiters)) { +state_expecting_regex = !state_expecting_regex; +continue; + } + + // Store our current token + if (state_expecting_regex) { +regex = token; + } else { +replacement = token; + } + + // If we have a pair of (regex, replacement) tokens, we can go ahead and store those values into Diags + if (regex && replacement) { +scrub_add(regex, replacement); +regex.clear(); +replacement.clear(); + } +} + } + + // This means that there was some kind of config or parse error. + // This doesn't catch *all* errors, and is just a heuristic so we can't actually print any meaningful message. + if ((regex && !replacement) || (!regex && replacement)) { +// Error("Error in scrubbing config parsing. Not enabling scrubbing."); + } +} + +Scrubber::~Scrubber() +{ + if (config) { +ats_free(config); + } + + for (auto s : scrubs) { +delete s; + } +} + +bool +Scrubber::scrub_add(const ts::StringView pattern, const ts::StringView replacement) +{ + pcre *re; + const char *error; + int erroroffset; + Scrub *s; + + // Temp storage on stack is probably OK. It's unlikely someone will have long enough strings to blow the stack + char _pattern[pattern.size() + 1]; + sprintf(_pattern, "%.*s", static_cast(pattern.size()), pattern.ptr()); + + // compile the regular expression + re = pcre_compile(_pattern, 0, , , NULL); + if (!re) { +// Error("Unable to compile PCRE scrubbing pattern"); +// Error("Scrubbing pattern failed at offset %d: %s.", erroroffset, error); +return false; + } + + // add the scrub pattern to our list + s = new Scrub; + s->pattern = pattern; + s->replacement = replacement; + s->compiled_re = re; + scrubs.push_back(s); + + return true; +} + +void +Scrubber::scrub_buffer(char *buffer) const +{ + // apply every Scrub in the vector, in order + for (auto s : scrubs) { +scrub_buffer(buffer, s); + } +} + +void +Scrubber::scrub_buffer(char *buffer, Scrub *scrub) const +{ + char *buffer_ptr; + int num_matched; + int match_len; + int replacement_len; + int buffer_len = strlen(buffer); + + // execute regex + num_matched = pcre_exec(scrub->compiled_re, nullptr, buffer, buffer_len, 0, 0, scrub->ovector, scrub->OVECCOUNT); + if (num_matched < 0) { +switch (num_matched) { +case PCRE_ERROR_NOMATCH: + return; +default: + // Error("PCRE matching error %d\n", num_matched); + break; +} + } + + /* + * When scrubbing the buffer in place, there are 2 scenarios we need to consider: + * +
[GitHub] trafficserver pull request #1567: [RFC] Diags scrubbing mechanism
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1567#discussion_r106456327 --- Diff: lib/ts/Scrubber.cc --- @@ -0,0 +1,201 @@ +/** @file + + Implementation for Scrubber.h + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#include "Scrubber.h" + +Scrubber::Scrubber(const char *config) +{ + bool state_expecting_regex = true; + const ts::StringView delimiters("->;,", ts::StringView::literal); + ts::StringView regex; + ts::StringView replacement; + ts::StringView text(config); + + this->config = ats_strdup(config); + + // Loop over config string while extracting tokens + while (text) { +ts::StringView token = (text.ltrim()).extractPrefix(); +if (token) { + // If we hit a delimiter, we change states and skip the token + if (!ts::StringView(token).ltrim(delimiters)) { +state_expecting_regex = !state_expecting_regex; +continue; + } + + // Store our current token + if (state_expecting_regex) { +regex = token; + } else { +replacement = token; + } + + // If we have a pair of (regex, replacement) tokens, we can go ahead and store those values into Diags + if (regex && replacement) { +scrub_add(regex, replacement); +regex.clear(); +replacement.clear(); + } +} + } + + // This means that there was some kind of config or parse error. + // This doesn't catch *all* errors, and is just a heuristic so we can't actually print any meaningful message. + if ((regex && !replacement) || (!regex && replacement)) { +// Error("Error in scrubbing config parsing. Not enabling scrubbing."); + } +} + +Scrubber::~Scrubber() +{ + if (config) { +ats_free(config); + } + + for (auto s : scrubs) { +delete s; + } +} + +bool +Scrubber::scrub_add(const ts::StringView pattern, const ts::StringView replacement) +{ + pcre *re; + const char *error; + int erroroffset; + Scrub *s; + + // Temp storage on stack is probably OK. It's unlikely someone will have long enough strings to blow the stack + char _pattern[pattern.size() + 1]; + sprintf(_pattern, "%.*s", static_cast(pattern.size()), pattern.ptr()); + + // compile the regular expression + re = pcre_compile(_pattern, 0, , , NULL); + if (!re) { +// Error("Unable to compile PCRE scrubbing pattern"); +// Error("Scrubbing pattern failed at offset %d: %s.", erroroffset, error); +return false; + } + + // add the scrub pattern to our list + s = new Scrub; + s->pattern = pattern; + s->replacement = replacement; + s->compiled_re = re; + scrubs.push_back(s); + + return true; +} + +void +Scrubber::scrub_buffer(char *buffer) const +{ + // apply every Scrub in the vector, in order + for (auto s : scrubs) { +scrub_buffer(buffer, s); + } +} + +void +Scrubber::scrub_buffer(char *buffer, Scrub *scrub) const +{ + char *buffer_ptr; + int num_matched; + int match_len; + int replacement_len; + int buffer_len = strlen(buffer); + + // execute regex + num_matched = pcre_exec(scrub->compiled_re, nullptr, buffer, buffer_len, 0, 0, scrub->ovector, scrub->OVECCOUNT); + if (num_matched < 0) { +switch (num_matched) { +case PCRE_ERROR_NOMATCH: + return; +default: + // Error("PCRE matching error %d\n", num_matched); + break; +} + } + + /* + * When scrubbing the buffer in place, there are 2 scenarios we need to consider: + * +
[GitHub] trafficserver pull request #1567: [RFC] Diags scrubbing mechanism
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1567#discussion_r106447714 --- Diff: lib/ts/Scrubber.cc --- @@ -0,0 +1,201 @@ +/** @file + + Implementation for Scrubber.h + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#include "Scrubber.h" + +Scrubber::Scrubber(const char *config) +{ + bool state_expecting_regex = true; + const ts::StringView delimiters("->;,", ts::StringView::literal); + ts::StringView regex; + ts::StringView replacement; + ts::StringView text(config); + + this->config = ats_strdup(config); + + // Loop over config string while extracting tokens + while (text) { +ts::StringView token = (text.ltrim()).extractPrefix(); +if (token) { + // If we hit a delimiter, we change states and skip the token + if (!ts::StringView(token).ltrim(delimiters)) { +state_expecting_regex = !state_expecting_regex; +continue; + } + + // Store our current token + if (state_expecting_regex) { +regex = token; + } else { +replacement = token; + } + + // If we have a pair of (regex, replacement) tokens, we can go ahead and store those values into Diags + if (regex && replacement) { +scrub_add(regex, replacement); +regex.clear(); +replacement.clear(); + } +} + } + + // This means that there was some kind of config or parse error. + // This doesn't catch *all* errors, and is just a heuristic so we can't actually print any meaningful message. + if ((regex && !replacement) || (!regex && replacement)) { +// Error("Error in scrubbing config parsing. Not enabling scrubbing."); + } +} + +Scrubber::~Scrubber() +{ + if (config) { +ats_free(config); + } + + for (auto s : scrubs) { +delete s; + } +} + +bool +Scrubber::scrub_add(const ts::StringView pattern, const ts::StringView replacement) +{ + pcre *re; + const char *error; + int erroroffset; + Scrub *s; + + // Temp storage on stack is probably OK. It's unlikely someone will have long enough strings to blow the stack + char _pattern[pattern.size() + 1]; + sprintf(_pattern, "%.*s", static_cast(pattern.size()), pattern.ptr()); + + // compile the regular expression + re = pcre_compile(_pattern, 0, , , NULL); + if (!re) { +// Error("Unable to compile PCRE scrubbing pattern"); +// Error("Scrubbing pattern failed at offset %d: %s.", erroroffset, error); +return false; + } + + // add the scrub pattern to our list + s = new Scrub; + s->pattern = pattern; + s->replacement = replacement; + s->compiled_re = re; + scrubs.push_back(s); + + return true; +} + +void +Scrubber::scrub_buffer(char *buffer) const +{ + // apply every Scrub in the vector, in order + for (auto s : scrubs) { +scrub_buffer(buffer, s); + } +} + +void +Scrubber::scrub_buffer(char *buffer, Scrub *scrub) const +{ + char *buffer_ptr; + int num_matched; + int match_len; + int replacement_len; + int buffer_len = strlen(buffer); + + // execute regex + num_matched = pcre_exec(scrub->compiled_re, nullptr, buffer, buffer_len, 0, 0, scrub->ovector, scrub->OVECCOUNT); + if (num_matched < 0) { +switch (num_matched) { +case PCRE_ERROR_NOMATCH: + return; +default: + // Error("PCRE matching error %d\n", num_matched); + break; +} + } + + /* + * When scrubbing the buffer in place, there are 2 scenarios we need to consider: + * +
[GitHub] trafficserver pull request #1567: [RFC] Diags scrubbing mechanism
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1567#discussion_r106446324 --- Diff: lib/ts/Scrubber.cc --- @@ -0,0 +1,201 @@ +/** @file + + Implementation for Scrubber.h + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#include "Scrubber.h" + +Scrubber::Scrubber(const char *config) +{ + bool state_expecting_regex = true; + const ts::StringView delimiters("->;,", ts::StringView::literal); + ts::StringView regex; + ts::StringView replacement; + ts::StringView text(config); + + this->config = ats_strdup(config); + + // Loop over config string while extracting tokens + while (text) { +ts::StringView token = (text.ltrim()).extractPrefix(); +if (token) { + // If we hit a delimiter, we change states and skip the token + if (!ts::StringView(token).ltrim(delimiters)) { +state_expecting_regex = !state_expecting_regex; +continue; + } + + // Store our current token + if (state_expecting_regex) { +regex = token; + } else { +replacement = token; + } + + // If we have a pair of (regex, replacement) tokens, we can go ahead and store those values into Diags + if (regex && replacement) { +scrub_add(regex, replacement); +regex.clear(); +replacement.clear(); + } +} + } + + // This means that there was some kind of config or parse error. + // This doesn't catch *all* errors, and is just a heuristic so we can't actually print any meaningful message. + if ((regex && !replacement) || (!regex && replacement)) { --- End diff -- Why check this, if nothing is done? --- 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 #1567: [RFC] Diags scrubbing mechanism
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1567#discussion_r106445847 --- Diff: lib/ts/Scrubber.cc --- @@ -0,0 +1,201 @@ +/** @file + + Implementation for Scrubber.h + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#include "Scrubber.h" + +Scrubber::Scrubber(const char *config) +{ + bool state_expecting_regex = true; + const ts::StringView delimiters("->;,", ts::StringView::literal); --- End diff -- Make this `static` also, so that it's constructed at process start. --- 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 #1567: [RFC] Diags scrubbing mechanism
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1567#discussion_r106446114 --- Diff: lib/ts/Scrubber.cc --- @@ -0,0 +1,201 @@ +/** @file + + Implementation for Scrubber.h + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#include "Scrubber.h" + +Scrubber::Scrubber(const char *config) +{ + bool state_expecting_regex = true; + const ts::StringView delimiters("->;,", ts::StringView::literal); + ts::StringView regex; + ts::StringView replacement; + ts::StringView text(config); --- End diff -- This is problematic, because the `StringView` will be pointing in to external memory. This needs to point to the local memory in `this->config`. --- 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 #1567: [RFC] Diags scrubbing mechanism
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1567#discussion_r106445625 --- Diff: lib/ts/Diags.h --- @@ -226,6 +242,10 @@ class Diags mutable ink_mutex tag_table_lock; // prevents reconfig/read races DFA *activated_tags[2]; // 1 table for debug, 1 for action + // Scrubbing variables -- replaces predefined regexp matches with the replacement string + bool scrub_enabled = false; + Scrubber *scrubber = nullptr; --- End diff -- This is a good place to use `std::unique_ptr`. --- 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 #1567: [RFC] Diags scrubbing mechanism
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1567#discussion_r106444530 --- Diff: lib/ts/Diags.cc --- @@ -926,3 +917,29 @@ Diags::rebind_stderr(int new_fd) } return false; } + +void +Diags::vprintline(FILE *fp, char *buffer, va_list ap) const +{ + int nbytes = strlen(buffer); + + // check if we need to scrub any patterns + if (unlikely(scrub_enabled)) { +char _buf[MAX_LOG_LINE_SIZE]; +int r = vsnprintf(_buf, MAX_LOG_LINE_SIZE, buffer, ap); +if (r > MAX_LOG_LINE_SIZE) { + sprintf(_buf, "[WARN] Diags log line too long, dropping log\n"); +} else if (r < 0) { + sprintf(_buf, "[WARN] Diags log line failed to encode with vsnprintf\n"); +} +scrubber->scrub_buffer(_buf); +fprintf(fp, "%s", _buf); --- End diff -- Actually, you have `r`, so `memcpy` could be used. --- 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 #1567: [RFC] Diags scrubbing mechanism
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1567#discussion_r106444345 --- Diff: lib/ts/Diags.cc --- @@ -926,3 +917,29 @@ Diags::rebind_stderr(int new_fd) } return false; } + +void +Diags::vprintline(FILE *fp, char *buffer, va_list ap) const +{ + int nbytes = strlen(buffer); + + // check if we need to scrub any patterns + if (unlikely(scrub_enabled)) { +char _buf[MAX_LOG_LINE_SIZE]; +int r = vsnprintf(_buf, MAX_LOG_LINE_SIZE, buffer, ap); +if (r > MAX_LOG_LINE_SIZE) { + sprintf(_buf, "[WARN] Diags log line too long, dropping log\n"); +} else if (r < 0) { + sprintf(_buf, "[WARN] Diags log line failed to encode with vsnprintf\n"); +} +scrubber->scrub_buffer(_buf); +fprintf(fp, "%s", _buf); --- End diff -- Should probably just `strlcpy` here, although the `fprintf` is likely almost as fast. --- 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 #1299: TS-2888: remove vararg and format paramete...
Github user SolidWallOfCode closed the pull request at: https://github.com/apache/trafficserver/pull/1299 --- 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 #1479: Add static type checking to configuration ...
Github user SolidWallOfCode closed the pull request at: https://github.com/apache/trafficserver/pull/1479 --- 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 #1209: Remove unused and never to be used classes...
Github user SolidWallOfCode closed the pull request at: https://github.com/apache/trafficserver/pull/1209 --- 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 #1565: Fix Assertion failure in the regex_revalidate plu...
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/pull/1565 @jpeach I'll take care of updating the docs in a direct commit, it's not worth a 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 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 #1581: TS-4976: Regularize plugins - file
GitHub user SolidWallOfCode opened a pull request: https://github.com/apache/trafficserver/pull/1581 TS-4976: Regularize plugins - file Incremental work on #1114. You can merge this pull request into a Git repository by running: $ git pull https://github.com/SolidWallOfCode/trafficserver ts-4976-file Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1581.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 #1581 commit 9b6009325749a622428f1fc3f86aecd67efa1a7b Author: Alan M. Carroll <a...@apache.org> Date: 2017-03-14T21:13:22Z TS-4976: Regularize plugins - 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 pull request #1569: TS-4976: Regularize plugins - cache_scan.
Github user SolidWallOfCode closed the pull request at: https://github.com/apache/trafficserver/pull/1569 --- 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 #1516: Implement Cache-Control: immutable handling
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/pull/1516 To do: * Change the default value for `_max_age` to `INTMAX`. In `generate` if the value is still `INTMAX` use `31536`. This is consistent with current practice but allows the option of putting in a limit later. * Comment out the check for `public` and add a comment that it should be enabled for ATS 8.0. We'll enable it in our fork, because IIRC that was the reason this work was done. --- 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 #1516: Implement Cache-Control: immutable handling
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/pull/1516 I've read the comments and looked at the changes and I am unclear on what the *behavior* change is with regard to public/private. --- 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 #1574: Explicit std:: name space, seems to make compiler...
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/pull/1574 Did we want to remove the MATH_H checks in `configure.ac`? --- 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 #1567: [WIP] Diags scrubbing mechanism
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1567#discussion_r105936302 --- Diff: lib/ts/Scrubber.cc --- @@ -0,0 +1,138 @@ +#include "Scrubber.h" +#include "MemView.h" + +Scrubber::Scrubber(const char *config) +{ + bool state_expecting_regex = true; + char *regex= nullptr; + char *replacement = nullptr; + const ts::StringView delimiters("->;,", ts::StringView::literal); + ts::StringView text(config); + + this->config = ats_strdup(config); --- End diff -- Another option would be for Scrubber to take ownership of the memory and avoid the extra alloc/copy. Beyond that, since the entire string is already persistent, you can use `StringView` instances for the pieces and not allocate for them at all. E.g., `regex` can be a `StringView` that points in to `config`. --- 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 #1572: Scalar: Fix inconsistencies in numeric com...
Github user SolidWallOfCode closed the pull request at: https://github.com/apache/trafficserver/pull/1572 --- 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 #1572: Scalar: Fix inconsistencies in numeric com...
GitHub user SolidWallOfCode opened a pull request: https://github.com/apache/trafficserver/pull/1572 Scalar: Fix inconsistencies in numeric comparisons. As a design udpate, Scalar has been changed to treat the value of the instance as it's effective value. The numeric comparisons need to be updated to follow this rule. You can merge this pull request into a Git repository by running: $ git pull https://github.com/SolidWallOfCode/trafficserver scalar-fix Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1572.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 #1572 commit 7484620149f5c08bd726f0a2a3b77b9d9a3b3aab Author: Alan M. Carroll <a...@apache.org> Date: 2017-03-13T16:44:44Z Scalar: Fix inconsistencies in numeric comparisons. --- 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 #1571: StringVIew should be made compatible with std::st...
GitHub user SolidWallOfCode opened an issue: https://github.com/apache/trafficserver/issues/1571 StringVIew should be made compatible with std::string_view for C++17 `StringView` is strongly different from the original proposals for string reference classes (e.g. `Boost.StringRef`) but the current proposal for C++17, `std::string_view`, is quite close in design and use. While `StringView` should be retained as it provides a number of essential methods not in `std::string_view`, `StringView` can be made to be a strict superset of `std::string_view` so that any code that uses `std::string_view` can replace `std::string_view` with `StringView`, and any developer used to `std::string_view` could immediately use `StringView`. In addition, `StringView` should promote direct interoperability, providing implicit conversions to and from `std::string_view`. --- 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 #1570: Replace ts::ConstBuffer with StringView
GitHub user SolidWallOfCode opened an issue: https://github.com/apache/trafficserver/issues/1570 Replace ts::ConstBuffer with StringView StringView is now committed to master. It was designed to replace `ts::ConstBuffer` therefore that should now be done and `TSBuffer.h` removed. --- 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 #1534: Use StringView for protocol stack to avoid...
Github user SolidWallOfCode closed the pull request at: 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 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 #1534: Use StringView for protocol stack to avoid callin...
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/pull/1534 Squashing and verifying. --- 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 #1381: Lib Scalar
Github user SolidWallOfCode closed the pull request at: https://github.com/apache/trafficserver/pull/1381 --- 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 #1569: TS-4976: Regularize plugins - cache_scan.
GitHub user SolidWallOfCode opened a pull request: https://github.com/apache/trafficserver/pull/1569 TS-4976: Regularize plugins - cache_scan. Incremental work on #1114. You can merge this pull request into a Git repository by running: $ git pull https://github.com/SolidWallOfCode/trafficserver ts-4976-cache-scan Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1569.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 #1569 commit 5a56ec73d6562d20faad156874d2281797f1be68 Author: Alan M. Carroll <a...@apache.org> Date: 2017-03-11T00:45:58Z TS-4976: Regularize plugins - cache_scan. --- 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 #1564: TS-4976: Regularize plugins - null transfo...
Github user SolidWallOfCode closed the pull request at: https://github.com/apache/trafficserver/pull/1564 --- 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 #1567: Diags scrubbing mechanism
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/pull/1567 Spoke with Dan, we'll get the allocations cleanup before Leif has a stroke. --- 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 #1567: Diags scrubbing mechanism
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/pull/1567 Lots of allocation to be removed. This should be done without any allocation. --- 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 #1567: Diags scrubbing mechanism
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1567#discussion_r105472943 --- Diff: lib/ts/Diags.h --- @@ -92,6 +94,26 @@ struct DiagsConfigState { DiagsModeOutput outputs[DiagsLevel_Count]; // where each level prints }; +struct Scrub { + ~Scrub() + { +if (pattern) { --- End diff -- `ats_scoped_str` might be useful 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 #1567: Diags scrubbing mechanism
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1567#discussion_r105471918 --- Diff: doc/admin-guide/files/records.config.en.rst --- @@ -2975,6 +2975,38 @@ Diagnostic Logging Configuration Specifies at what size to roll the diagnostics log at. +.. ts:cv:: CONFIG proxy.config.diags.scrubs STRING NULL + + Diagnostic log scrubbing is intended an extra security measure to prevent sensitive strings from leaking to + diagnistic logs. For example, if you wanted to scrub the regular expression ``string[a-z]`` from diagnostic + logs and replace it with ``XX``, use as the config string ``string[a-z] -> XX``. Note that the spaces + between the ``->`` are required. + + If you want to scrub multiple regular expressions, use ``;`` to separate the pairs. For example: + ``string[a-z] -> XX ; doge$ -> doggy``. Once again, the spaces before and after the ``;`` are required. + +.. note:: + + Diagnostic log scrubbing supports PCRE format regular expressions. + +.. note:: + + Multiple captures are not supported. (ie. ``s/foo/bar/g`` is not suppored while ``s/foo/bar/`` is). Furthermore, --- End diff -- "Multiple replacements"? That is, can I have capture groups? More than one capture group? Also, "suppored". --- 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 #1534: Use StringView for protocol stack to avoid callin...
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/pull/1534 @bryancall says `StringView` should be changed to be conformant with `string_ref` from C++17. This would involve changing the method naming scheme and adding some additional methods to cover those in `string_ref`. --- 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 #1564: TS-4976: Regularize plugins - null transfo...
GitHub user SolidWallOfCode opened a pull request: https://github.com/apache/trafficserver/pull/1564 TS-4976: Regularize plugins - null transform. Incremental work on #1114. You can merge this pull request into a Git repository by running: $ git pull https://github.com/SolidWallOfCode/trafficserver ts-4976-null-transform Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1564.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 #1564 commit 8b2cb74ce4b88e5cdf45562901485f06a43beab6 Author: Alan M. Carroll <a...@apache.org> Date: 2017-03-10T13:32:52Z TS-4976: Regularize plugins - null transform. --- 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 #1554: TS-4976: Regularize example plugins - blac...
Github user SolidWallOfCode closed the pull request at: https://github.com/apache/trafficserver/pull/1554 --- 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 #1563: MemView: Add literal and array constructor...
Github user SolidWallOfCode closed the pull request at: https://github.com/apache/trafficserver/pull/1563 --- 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 #1563: MemView: Add literal and array constructor...
GitHub user SolidWallOfCode opened a pull request: https://github.com/apache/trafficserver/pull/1563 MemView: Add literal and array constructors. Add specialized constructors for string literals and character arrays. You can merge this pull request into a Git repository by running: $ git pull https://github.com/SolidWallOfCode/trafficserver memview-constructors Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1563.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 #1563 commit dcca03c4cab6c306c7ddcb934f0f5e227157f406 Author: Alan M. Carroll <a...@apache.org> Date: 2017-03-10T10:18:11Z MemView: Add literal and array constructors. --- 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 #1534: Use StringView for protocol stack to avoid...
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1534#discussion_r105075862 --- Diff: iocore/net/P_UnixNetVConnection.h --- @@ -149,21 +149,21 @@ class UnixNetVConnection : public NetVConnection // called when handing an event from this NetVConnection,// // or the NetVConnection creation callback. // - virtual void set_active_timeout(ink_hrtime timeout_in); - virtual void set_inactivity_timeout(ink_hrtime timeout_in); - virtual void cancel_active_timeout(); - virtual void cancel_inactivity_timeout(); - virtual void set_action(Continuation *c); - virtual void add_to_keep_alive_queue(); - virtual void remove_from_keep_alive_queue(); - virtual bool add_to_active_queue(); + void set_active_timeout(ink_hrtime timeout_in) override; + void set_inactivity_timeout(ink_hrtime timeout_in) override; + void cancel_active_timeout() override; + void cancel_inactivity_timeout() override; + void set_action(Continuation *c) override; + void add_to_keep_alive_queue() override; + void remove_from_keep_alive_queue() override; + bool add_to_active_queue() override; virtual void remove_from_active_queue(); --- End diff -- @bryancall Note `remove_from_active_queue` does not override anything, in contrast to the rest of this group of methods. --- 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 #1534: Use StringView for protocol stack to avoid...
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1534#discussion_r105075732 --- Diff: proxy/http/HttpSM.cc --- @@ -8057,14 +8059,13 @@ HttpSM::is_redirect_required() // Fill in the client protocols used. Return the number of entries returned int -HttpSM::populate_client_protocol(const char **result, int n) const +HttpSM::populate_client_protocol(ts::StringView *result, int n) const --- End diff -- This should be 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 issue #1534: Use StringView for protocol stack to avoid callin...
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/pull/1534 @maskit I'll look at that, but zwoop's test output has the 'h2' in 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 pull request #1554: TS-4976: Regularize example plugins - blac...
GitHub user SolidWallOfCode opened a pull request: https://github.com/apache/trafficserver/pull/1554 TS-4976: Regularize example plugins - blacklist Incremental work on #1114. You can merge this pull request into a Git repository by running: $ git pull https://github.com/SolidWallOfCode/trafficserver ts-4976-blacklist Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1554.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 #1554 commit d8882e389816fb9613989202b494806fba6d882b Author: Alan M. Carroll <a...@apache.org> Date: 2017-03-08T14:13:31Z TS-4976: Regularize example plugins - blacklist --- 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 #1537: TS-4976: Regularize example plugin basic_a...
Github user SolidWallOfCode closed the pull request at: https://github.com/apache/trafficserver/pull/1537 --- 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 #1534: Use StringView for protocol stack to avoid callin...
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/pull/1534 Upon further review of this, #1520, and #1506, I think this is just a bug fix, in that there was already code in place to report the protocol stack in the VIA header. Therefore this is primarily an update to use `StringView` (and incidentally fix the #1506 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 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 #1537: TS-4976: Regularize example plugin basic_auth.
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/pull/1537 @maskit For future reference, in your `.bashrc`. ``` function findrst { find . -name '*.rst' -exec grep "$1" {} \; -print } ``` Then `findrst basic-auth` in either the base directory or `doc`. --- 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 #1537: TS-4976: Regularize example plugin basic_auth.
Github user SolidWallOfCode commented on the issue: https://github.com/apache/trafficserver/pull/1537 Allright. --- 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. ---