[GitHub] trafficserver issue #1615: Doc: Update plugin stats documentation.
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1615 > On Mar 28, 2017, at 2:08 PM, Alan M. Carroll <notificati...@github.com> wrote: > > 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. Oh I see. Yeh that's nice. --- If your project is set 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 #1598: Assertion failing when calling TSHttpTxnReenable ...
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/issues/1598 You have to call `TSHttpTxnReenable` from an event thread. The typical pattern is to use `TSContSchedule` to bounce the call back over to the event thread pool once you are 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 #1565: Fix Assertion failure in the regex_revalidate plu...
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1565 Consider updating the API 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 issue #1457: fix TS-4195: crash when stop trafficserver
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1457 With some additional effort, we can fix this reliably. The traditional way to do this is for the signal handler to write a byte to a pipe, which will wake up an event handler to do an ordered shutdown. On linux, you could also use signalfd. Something like this should be the goal 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 issue #1515: Logging cache code map size fix
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1515 Can you look into fixing the APIs so this can't happen again? > On Feb 28, 2017, at 10:32 AM, Gancho Tenev <notificati...@github.com> wrote: > > The size of the cache code map does not correspond to the number of cache result code values. > Discrepancy introduced with commit eadc9cf > > You can view, comment on, or merge this pull request online at: > > https://github.com/apache/trafficserver/pull/1515 > > Commit Summary > > Logging cache code map size fix > File Changes > > M proxy/logging/Log.cc (2) > Patch Links: > > https://github.com/apache/trafficserver/pull/1515.patch > https://github.com/apache/trafficserver/pull/1515.diff > â > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub, or mute the thread. > --- If your project is set 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 #1508: Fix template directory path
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1508#discussion_r103381037 --- Diff: proxy/http/HttpBodyFactory.cc --- @@ -301,14 +299,13 @@ HttpBodyFactory::reconfigure() ats_scoped_str directory_of_template_sets(RecConfigReadConfigPath("proxy.config.body_factory.template_sets_dir", "body_factory")); - directory_of_template_sets = Layout::get()->relative(s); + directory_of_template_sets = Layout::get()->relative(directory_of_template_sets); --- End diff -- At this point, we don't really need to do `relative_to` since `RecConfigReadConfigPath` guarantees us an absolute path. --- If your project is set 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 #1423: traffic_ctl status is broken
Github user jpeach closed the issue at: https://github.com/apache/trafficserver/issues/1423 --- If your project is set 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 #1423: traffic_ctl status is broken
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/issues/1423 ``` [jpeach@sk1 trafficserver]$ /opt/ats/bin/traffic_ctl config status Apache Traffic Server - traffic_server - 7.2.0 - (build # 022120 on Feb 21 2017 at 20:56:37) Started at Tue Feb 21 21:18:56 2017 Last reconfiguration at Tue Feb 21 21:18:55 2017 Configuration is current ``` Must have been PBKAC. --- If your project is set 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 #1460: Removes some unnecessary dependencies on t...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1460#discussion_r102378936 --- Diff: proxy/FetchSM.cc --- @@ -26,6 +26,7 @@ #include #include "HTTP.h" #include "PluginVC.h" +#include // Ugly, but we need a bunch of the public APIs here ... :-/ --- End diff -- Can you make this "api/ts/ts.h" for consistency with the other place you do this? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this 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 #1460: Removes some unnecessary dependencies on t...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1460#discussion_r102379397 --- Diff: proxy/http/remap/RemapConfig.cc --- @@ -778,7 +778,7 @@ remap_load_plugin(const char **argv, int argc, url_mapping *mp, char *errbuf, in } if (stat(c, _buf) != 0) { -const char *plugin_default_path = TSPluginDirGet(); +const char *plugin_default_path = RecConfigReadPrefixPath("proxy.config.plugin.plugin_dir"); --- End diff -- You need to `free()` the result 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 #1460: Removes some unnecessary dependencies on t...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1460#discussion_r102379269 --- Diff: proxy/Plugin.cc --- @@ -231,8 +231,8 @@ plugin_init(bool validateOnly) if (INIT_ONCE) { api_init(); -TSConfigDirGet(); -plugin_dir = TSPluginDirGet(); +RecConfigReadConfigDir(); // ToDo: Why is this needed? It does an initialization? --- End diff -- `RecConfigReadConfigDir()` can't possibly be needed 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 #1460: Removes some unnecessary dependencies on t...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1460#discussion_r102378986 --- Diff: proxy/Main.cc --- @@ -38,6 +38,8 @@ #include "ts/ink_syslog.h" #include "ts/hugepages.h" +#include // This is sadly needed becuase of us using TSThreadInit() for some reason. --- End diff -- s/becuase/because/ --- If your project is set 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 #1460: Removes some unnecessary dependencies on t...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1460#discussion_r102378964 --- Diff: proxy/Main.cc --- @@ -38,6 +38,8 @@ #include "ts/ink_syslog.h" #include "ts/hugepages.h" +#include // This is sadly needed becuase of us using TSThreadInit() for some reason. --- End diff -- Consistency. --- If your project is set 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 #1460: Removes some unnecessary dependencies on t...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1460#discussion_r102379416 --- Diff: proxy/Plugin.cc --- @@ -231,8 +231,8 @@ plugin_init(bool validateOnly) if (INIT_ONCE) { api_init(); -TSConfigDirGet(); -plugin_dir = TSPluginDirGet(); +RecConfigReadConfigDir(); // ToDo: Why is this needed? It does an initialization? +plugin_dir = RecConfigReadPrefixPath("proxy.config.plugin.plugin_dir"); --- End diff -- You need to `free()` the result 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 #1460: Removes some unnecessary dependencies on t...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1460#discussion_r102378869 --- Diff: lib/wccp/Makefile.am --- @@ -20,8 +20,8 @@ include $(top_srcdir)/build/tidy.mk AM_CPPFLAGS = \ - -I$(abs_top_srcdir)/lib \ - -I$(abs_top_srcdir)/proxy/api/ts + -I$(abs_top_srcdir)/lib --- End diff -- Missing a line continuation 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 issue #1425: PROXY_CONFIG_CONFIG_DIR doesn't change sysconfig ...
Github user jpeach closed the issue at: https://github.com/apache/trafficserver/issues/1425 --- If your project is set 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 #1468: Allow overriding proxy.config.config_dir.
Github user jpeach closed the pull request at: https://github.com/apache/trafficserver/pull/1468 --- If your project is set 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 #1479: Add static type checking to configuration overrid...
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1479 I don't get how this is better than the previous version. That was purely compile time and this compares the global `typeid` constants at runtime. It doesn't seem 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 #1477: Uses static type checking on the overridab...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1477#discussion_r102254881 --- Diff: proxy/InkAPI.cc --- @@ -7772,395 +7772,372 @@ TSSkipRemappingSet(TSHttpTxn txnp, int flag) } // Little helper function to find the struct member -static void * -_conf_to_memberp(TSOverridableConfigKey conf, OverridableHttpConfigParams *overridableHttpConfig, OverridableDataType *typep) +#define GET_OVERIDE(var)\ + do { \ +type = overridable_typeof(var); \ +return static_cast(&(var)); \ + } while (0) + +template +OverridableDataType +overridable_typeof(T ) +{ + static_assert(ts_always_false::value, "Invalid overridable type or member"); + return OVERRIDABLE_TYPE_NULL; +} + +template <> +OverridableDataType +overridable_typeof(MgmtByte &) +{ + return OVERRIDABLE_TYPE_BYTE; +} + +template <> +OverridableDataType +overridable_typeof(MgmtInt &) +{ + return OVERRIDABLE_TYPE_INT; +} + +template <> +OverridableDataType +overridable_typeof(MgmtFloat &) { - // The default is "Byte", make sure to override that for those configs which are "Int". - OverridableDataType typ = OVERRIDABLE_TYPE_BYTE; - void *ret = nullptr; + return OVERRIDABLE_TYPE_FLOAT; +} + +template <> +OverridableDataType +overridable_typeof(MgmtString &) +{ + return OVERRIDABLE_TYPE_STRING; +} +static void * +_conf_to_memberp(TSOverridableConfigKey conf, OverridableHttpConfigParams *configs, OverridableDataType ) +{ switch (conf) { case TS_CONFIG_URL_REMAP_PRISTINE_HOST_HDR: -ret = >maintain_pristine_host_hdr; +GET_OVERIDE(configs->maintain_pristine_host_hdr); break; case TS_CONFIG_HTTP_CHUNKING_ENABLED: -ret = >chunking_enabled; +GET_OVERIDE(configs->chunking_enabled); break; case TS_CONFIG_HTTP_NEGATIVE_CACHING_ENABLED: -ret = >negative_caching_enabled; +GET_OVERIDE(configs->negative_caching_enabled); break; case TS_CONFIG_HTTP_NEGATIVE_CACHING_LIFETIME: -typ = OVERRIDABLE_TYPE_INT; -ret = >negative_caching_lifetime; +GET_OVERIDE(configs->negative_caching_lifetime); break; case TS_CONFIG_HTTP_CACHE_WHEN_TO_REVALIDATE: -ret = >cache_when_to_revalidate; +GET_OVERIDE(configs->cache_when_to_revalidate); break; case TS_CONFIG_HTTP_KEEP_ALIVE_ENABLED_IN: -ret = >keep_alive_enabled_in; +GET_OVERIDE(configs->keep_alive_enabled_in); break; case TS_CONFIG_HTTP_KEEP_ALIVE_ENABLED_OUT: -ret = >keep_alive_enabled_out; +GET_OVERIDE(configs->keep_alive_enabled_out); break; case TS_CONFIG_HTTP_KEEP_ALIVE_POST_OUT: -ret = >keep_alive_post_out; +GET_OVERIDE(configs->keep_alive_post_out); break; case TS_CONFIG_HTTP_SERVER_SESSION_SHARING_MATCH: -ret = >server_session_sharing_match; +GET_OVERIDE(configs->server_session_sharing_match); break; case TS_CONFIG_NET_SOCK_RECV_BUFFER_SIZE_OUT: -typ = OVERRIDABLE_TYPE_INT; -ret = >sock_recv_buffer_size_out; +GET_OVERIDE(configs->sock_recv_buffer_size_out); break; case TS_CONFIG_NET_SOCK_SEND_BUFFER_SIZE_OUT: -typ = OVERRIDABLE_TYPE_INT; -ret = >sock_send_buffer_size_out; +GET_OVERIDE(configs->sock_send_buffer_size_out); break; case TS_CONFIG_NET_SOCK_OPTION_FLAG_OUT: -typ = OVERRIDABLE_TYPE_INT; -ret = >sock_option_flag_out; +GET_OVERIDE(configs->sock_option_flag_out); break; case TS_CONFIG_HTTP_FORWARD_PROXY_AUTH_TO_PARENT: -ret = >fwd_proxy_auth_to_parent; +GET_OVERIDE(configs->fwd_proxy_auth_to_parent); break; case TS_CONFIG_HTTP_ANONYMIZE_REMOVE_FROM: -ret = >anonymize_remove_from; +GET_OVERIDE(configs->anonymize_remove_from); break; case TS_CONFIG_HTTP_ANONYMIZE_REMOVE_REFERER: -ret = >anonymize_remove_referer; +GET_OVERIDE(configs->anonymize_remove_referer); break; case TS_CONFIG_HTTP_ANONYMIZE_REMOVE_USER_AGENT: -ret = >anonymize_remove_user_agent; +GET_OVERIDE(configs->anonymize_remove_user_agent); break; case TS_CONFIG_HTTP_ANONYMIZE_REMOVE_COOKIE: -ret = >anonymize_remove_cookie; +GET_OVERIDE(configs->anonymize_remove_cookie); break; case TS_CONFIG_HTTP_ANONYMIZE_REMOVE_CLIENT_IP: -r
[GitHub] trafficserver pull request #1477: Uses static type checking on the overridab...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1477#discussion_r102254435 --- Diff: proxy/InkAPI.cc --- @@ -7772,395 +7772,372 @@ TSSkipRemappingSet(TSHttpTxn txnp, int flag) } // Little helper function to find the struct member -static void * -_conf_to_memberp(TSOverridableConfigKey conf, OverridableHttpConfigParams *overridableHttpConfig, OverridableDataType *typep) +#define GET_OVERIDE(var)\ + do { \ +type = overridable_typeof(var); \ +return static_cast(&(var)); \ + } while (0) + +template +OverridableDataType +overridable_typeof(T ) +{ + static_assert(ts_always_false::value, "Invalid overridable type or member"); --- End diff -- Can't you do ``` static_assert(false, ""); ``` --- If your project is set 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 #1475: ASAN issue in traffic_manager metrics Lua binding...
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/issues/1475 Check the verbose log to ensure that Lua was excluded from ASAN correctly. --- If your project is set 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 #1461: Changes debug builds with Intel to just -g
Github user jpeach closed the pull request at: https://github.com/apache/trafficserver/pull/1461 --- If your project is set 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 #1469: Introduce a Result error object.
Github user jpeach closed the pull request at: https://github.com/apache/trafficserver/pull/1469 --- If your project is set 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 #1469: Introduce a Result error object.
GitHub user jpeach opened a pull request: https://github.com/apache/trafficserver/pull/1469 Introduce a Result error object. Introduce a `Result` object to carry an abstract error with a corresponding message. We already had `config_parse_error`, but this is slightly more general. I primarily added this because of the unhelpful error message you get when `storage.config` fails to load. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jpeach/trafficserver jpeach/result Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1469.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 #1469 commit d64b410ff0a1d92af541a6a5cfa6e94521298dc3 Author: James Peach <jpe...@apache.org> Date: 2017-02-18T21:54:31Z Add a Result class to carry error status. commit c721275f475666a205d39f74e682d011029665c6 Author: James Peach <jpe...@apache.org> Date: 2017-02-18T21:54:52Z Use Result to show Cache configuration parse errors. commit e9e7094e7f9951914965ccf9d722badcf777f036 Author: James Peach <jpe...@apache.org> Date: 2017-02-18T22:19:45Z Rename Macher template parameters to avoid a name collision. commit 7e35d96c3f49a7d71aa431e1e4e62f4eb7fe96b4 Author: James Peach <jpe...@apache.org> Date: 2017-02-18T22:41:53Z Replace config_parse_error with Result. --- If your project is set 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 #1468: Allow overriding proxy.config.config_dir.
GitHub user jpeach opened a pull request: https://github.com/apache/trafficserver/pull/1468 Allow overriding proxy.config.config_dir. Explicitly check whether proxy.config.config_dir has been overridden in the environment before trying to load any configuration files. This fixes #1425. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jpeach/trafficserver jpeach/config-dir Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1468.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 #1468 commit 0f379fdb48347f7cb24183e5405a03cd0534ba4d Author: James Peach <jpe...@apache.org> Date: 2017-02-18T23:25:44Z Allow overriding proxy.config.config_dir. Explicitly check whether proxy.config.config_dir has been overridden in the environment before trying to load any configuration files. --- If your project is set 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 #1457: fix TS-4195: crash when stop trafficserver
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1457 Is it still possible to use leak checkers that reconcile at exit after this fix? --- If your project is set 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 #1423: traffic_ctl status is broken
GitHub user jpeach opened an issue: https://github.com/apache/trafficserver/issues/1423 traffic_ctl status is broken ``` # /opt/ats/bin/traffic_ctl config status traffic_ctl: [11] Invalid parameters passed into function call. ``` --- If your project is set 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 #1357: TS-5022: reduce memory allocation in clientcert l...
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1357 Whilst the configuration of SSL keys and certificates is a bit unfortunate, I think that it is important to be consistent. Separately, you have to define the base directory for relative paths, otherwise the current working directory of whatever does the loading becomes an ABI. --- If your project is set 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 #1325: #1323 Restores the NULL string where appropriate
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1325 Afaict this is just strcmp then? > On Jan 13, 2017, at 7:03 PM, Leif Hedstrom <notificati...@github.com> wrote: > > @zwoop commented on this pull request. > > In lib/records/RecUtils.cc: > > > @@ -419,7 +419,7 @@ RecDataSetFromString(RecDataT data_type, RecData *data_dst, const char *data_str > data_src.rec_float = atof(data_string); > break; >case RECD_STRING: > -if (data_string && strcmp((data_string), "nullptr") == 0) { > +if (data_string && (strlen(data_string) == 4) && strncmp((data_string), "NULL", 4) == 0) { > Because amc wanted me to use strncmp(), so if you don't check the length, then it'd also match any string starting with NULL. > > â > You are receiving this because you commented. > Reply to this email directly, view it on GitHub, or mute the thread. > --- If your project is set 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 #1325: #1323 Restores the NULL string where appro...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1325#discussion_r96091697 --- Diff: lib/records/RecUtils.cc --- @@ -419,7 +419,7 @@ RecDataSetFromString(RecDataT data_type, RecData *data_dst, const char *data_str data_src.rec_float = atof(data_string); break; case RECD_STRING: -if (data_string && strcmp((data_string), "nullptr") == 0) { +if (data_string && (strlen(data_string) == 4) && strncmp((data_string), "NULL", 4) == 0) { --- End diff -- Why do you need the extra `strlen` 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 #1309: Fixes for building with LibreSSL
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1309#discussion_r95204594 --- Diff: example/cppapi/websocket/WSBuffer.cc --- @@ -157,7 +157,7 @@ WSBuffer::read_buffered_message(std::string , int ) std::string WSBuffer::ws_digest(std::string const ) { -#if OPENSSL_VERSION_NUMBER < 0x1010L +#if OPENSSL_VERSION_NUMBER < 0x1010L || defined(LIBRESSL_VERSION_NUMBER) --- End diff -- Do we really need all this ifdeffery? Can we just unconditionally use the modern API? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1294: TS-5059: Port TCP Fast Open BIO to OpenSSL...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1294#discussion_r94717030 --- Diff: iocore/net/BIO_fastopen.cc --- @@ -58,26 +57,26 @@ fastopen_bwrite(BIO *bio, const char *in, int insz) errno = 0; BIO_clear_retry_flags(bio); - ink_assert(bio->num != NO_FD); + ink_assert(BIO_get_fd(bio, nullptr) != NO_FD); --- End diff -- Rather than calling `BIO_get_fd` multiple times, call it once and stash the fd in a local variable. Same for the write path. --- If your project is set 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 #1294: TS-5059: Port TCP Fast Open BIO to OpenSSL...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1294#discussion_r94717201 --- Diff: iocore/net/BIO_fastopen.cc --- @@ -27,28 +27,27 @@ #include "BIO_fastopen.h" -static int -fastopen_create(BIO *bio) -{ - bio->init = 0; - bio->num = NO_FD; - bio->flags = 0; - bio->ptr = nullptr; +#if OPENSSL_VERSION_NUMBER < 0x1010L +#define BIO_set_data(a, _ptr) ((a)->ptr = (_ptr)) +#define BIO_get_data(a) ((a)->ptr) +#define BIO_get_shutdown(a) ((a)->shutdown) +#define BIO_meth_get_ctrl(biom) ((biom)->ctrl) +#define BIO_meth_get_create(biom) ((biom)->create) +#define BIO_meth_get_destroy(biom) ((biom)->destroy) +#endif - return 1; -} +static int (*fastopen_create)(BIO *) = BIO_meth_get_create(const_cast(BIO_s_socket())); static int fastopen_destroy(BIO *bio) { if (bio) { // We expect this BIO to not own the socket, so we must always // be in NOCLOSE mode. -ink_assert(bio->shutdown == BIO_NOCLOSE); -fastopen_create(bio); +ink_assert(BIO_get_shutdown(bio) == BIO_NOCLOSE); } - return 1; + return BIO_meth_get_destroy(const_cast(BIO_s_socket()))(bio); --- End diff -- Seems pretty unfortunate that they make you cast away the const 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 #1294: TS-5059: Port TCP Fast Open BIO to OpenSSL...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1294#discussion_r94717094 --- Diff: iocore/net/BIO_fastopen.cc --- @@ -116,39 +115,21 @@ static long fastopen_ctrl(BIO *bio, int cmd, long larg, void *ptr) { switch (cmd) { - case BIO_C_SET_FD: -ink_assert(larg == BIO_CLOSE || larg == BIO_NOCLOSE); -ink_assert(bio->num == NO_FD); - -bio->init = 1; -bio->shutdown = larg; -bio->num = *reinterpret_cast(ptr); -return 0; - case BIO_C_SET_CONNECT: // We only support BIO_set_conn_address(), which sets a sockaddr. ink_assert(larg == 2); -bio->ptr = ptr; +BIO_set_data(bio, ptr); return 0; - // We are unbuffered so unconditionally succeed on BIO_flush(). - case BIO_CTRL_FLUSH: -return 1; - - case BIO_CTRL_PUSH: - case BIO_CTRL_POP: -return 0; - - default: -#if DEBUG -ink_abort("unsupported BIO control cmd=%d larg=%ld ptr=%p", cmd, larg, ptr); -#endif - -return 0; + case BIO_C_SET_FD: +ink_assert(larg == BIO_CLOSE || larg == BIO_NOCLOSE); +ink_assert(BIO_get_fd(bio, nullptr) == NO_FD); --- End diff -- I don't think you need to worry about keeping this any more. --- If your project is set 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 #1294: TS-5059: Port TCP Fast Open BIO to OpenSSL...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1294#discussion_r94530155 --- Diff: iocore/net/BIO_fastopen.cc --- @@ -44,8 +57,7 @@ fastopen_destroy(BIO *bio) if (bio) { // We expect this BIO to not own the socket, so we must always // be in NOCLOSE mode. -ink_assert(bio->shutdown == BIO_NOCLOSE); -fastopen_create(bio); +ink_assert(BIO_get_shutdown(bio) == BIO_NOCLOSE); --- End diff -- The `Data` struct is getting leaked here, but (see above) I think we can get away without 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 #1294: TS-5059: Port TCP Fast Open BIO to OpenSSL...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1294#discussion_r94529780 --- Diff: iocore/net/BIO_fastopen.cc --- @@ -27,13 +27,26 @@ #include "BIO_fastopen.h" +#if OPENSSL_VERSION_NUMBER < 0x1010L +#define BIO_set_data(a, ptr) ((a)->ptr = (ptr)) --- End diff -- Check the preprocessed source, but I think this should be: ```C #define BIO_set_data(a, _ptr) ((a)->ptr = (_ptr)) ``` Same for the others below. --- If your project is set 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 #1294: TS-5059: Port TCP Fast Open BIO to OpenSSL...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1294#discussion_r94529615 --- Diff: iocore/net/BIO_fastopen.cc --- @@ -160,9 +174,24 @@ static const BIO_METHOD fastopen_methods = { .destroy = fastopen_destroy, .callback_ctrl = nullptr, }; +#else +static BIO_METHOD *fastopen_methods = nullptr; +#endif const BIO_METHOD * BIO_s_fastopen() { +#if OPENSSL_VERSION_NUMBER < 0x1010L return _methods; +#else + if (!fastopen_methods) { +fastopen_methods = BIO_meth_new(BIO_TYPE_SOCKET, "fastopen"); +BIO_meth_set_write(fastopen_methods, fastopen_bwrite); +BIO_meth_set_read(fastopen_methods, fastopen_bread); +BIO_meth_set_ctrl(fastopen_methods, fastopen_ctrl); +BIO_meth_set_create(fastopen_methods, fastopen_create); +BIO_meth_set_destroy(fastopen_methods, fastopen_destroy); --- End diff -- I think this would be a little cleaner (and avoid the race condition) if you used a static initializer: ```C struct fastopen_bio { fastopen_bio() { #if OPENSSL_VERSION_NUMBER < 0x1010L static const BIO_METHOD m = { ... }; method = #else method = BIO_meth_new(BIO_TYPE_SOCKET, "fastopen"); BIO_meth_set_write(method, fastopen_bwrite); ... #endif } const BIO_METHOD *method; }; static fastopen_bio fastopen; const BIO_METHOD * BIO_s_fastopen() { return fastopen.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 #1294: TS-5059: Port TCP Fast Open BIO to OpenSSL...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1294#discussion_r94530947 --- Diff: iocore/net/BIO_fastopen.cc --- @@ -27,13 +27,26 @@ #include "BIO_fastopen.h" +#if OPENSSL_VERSION_NUMBER < 0x1010L +#define BIO_set_data(a, ptr) ((a)->ptr = (ptr)) +#define BIO_get_data(a) ((a)->ptr) +#define BIO_set_init(a, init) ((a)->init = (init)) +#define BIO_set_shutdown(a, shut) ((a)->shutdown = (shut)) +#define BIO_get_shutdown(a) ((a)->shutdown) +#endif + +struct Data { + Data() : fd(NO_FD), dst(nullptr) {} + int fd; + const sockaddr *dst; +}; --- End diff -- Actually, to support `BIO_set_conn_address`, you'll need your own `ctrl` method that handles `BIO_C_SET_CONNECT` and forwards the remaining commands to the socket method. --- If your project is set 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 #1294: TS-5059: Port TCP Fast Open BIO to OpenSSL...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1294#discussion_r94530637 --- Diff: iocore/net/BIO_fastopen.cc --- @@ -27,13 +27,26 @@ #include "BIO_fastopen.h" +#if OPENSSL_VERSION_NUMBER < 0x1010L +#define BIO_set_data(a, ptr) ((a)->ptr = (ptr)) +#define BIO_get_data(a) ((a)->ptr) +#define BIO_set_init(a, init) ((a)->init = (init)) +#define BIO_set_shutdown(a, shut) ((a)->shutdown = (shut)) +#define BIO_get_shutdown(a) ((a)->shutdown) +#endif + +struct Data { + Data() : fd(NO_FD), dst(nullptr) {} + int fd; + const sockaddr *dst; +}; --- End diff -- I think that we can work around needing a separate `Data` struct. Since we actually have a socket we can now directly hook the `ctrl` method from the socket `BIO`: ```C const BIO_METHOD * s = BIO_s_socket(); BIO_meth_set_ctrl(fastopen.method, BIO_meth_get_ctrl(s)); ``` We can set the `ptr` field with `BIO_set_data`. We can use `BIO_get_fd` to get the file descriptor when we need 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 #1294: TS-5059: Port TCP Fast Open BIO to OpenSSL...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1294#discussion_r94530049 --- Diff: iocore/net/BIO_fastopen.cc --- @@ -27,13 +27,26 @@ #include "BIO_fastopen.h" +#if OPENSSL_VERSION_NUMBER < 0x1010L +#define BIO_set_data(a, ptr) ((a)->ptr = (ptr)) +#define BIO_get_data(a) ((a)->ptr) +#define BIO_set_init(a, init) ((a)->init = (init)) +#define BIO_set_shutdown(a, shut) ((a)->shutdown = (shut)) +#define BIO_get_shutdown(a) ((a)->shutdown) +#endif + +struct Data { + Data() : fd(NO_FD), dst(nullptr) {} + int fd; + const sockaddr *dst; +}; + static int fastopen_create(BIO *bio) { - bio->init = 0; - bio->num = NO_FD; - bio->flags = 0; - bio->ptr = nullptr; + Data *data = new Data; --- End diff -- Right, the reinitialization in `fastopen_destroy` is just being defensive. --- If your project is set 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 #981: TS-4821: Make hwloc a required dependency
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/981 Ubuntu 12 IIRC. But it is a problem with the virtualized hardware not the OS itself. > On Dec 16, 2016, at 10:07 AM, Phil Sorber <notificati...@github.com> wrote: > > What was the underlying OS? > > â > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub, or mute the thread. > --- If your project is set 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 #981: TS-4821: Make hwloc a required dependency
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/981 The problem I saw was with hwloc crashing on startup. This was on a Chinese VM hosting service and was caused by hwloc doing a divide by zero. It was unclear what version of hwloc the bug was fixed in. Needless to say this was a bit of a showstopper for running Traffic Server. > On Dec 16, 2016, at 9:38 AM, Phil Sorber <notificati...@github.com> wrote: > > @jpeach can you elaborate on which platform it was that didn't have hwloc? If not, we are going to try to get this in still. > > â > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub, or mute the thread. > --- If your project is set 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 #1261: TS-5096: Lua metrics crashes if the prefix...
GitHub user jpeach opened a pull request: https://github.com/apache/trafficserver/pull/1261 TS-5096: Lua metrics crashes if the prefix is missing. Prevent a traffic_manager crash if someone registers a metric with no prefix. We can't make that metric available in Lua, but we ough not to crash. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jpeach/trafficserver TS-5096 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1261.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 #1261 commit 63dc89511f97b622a1126c8576c27606a58a Author: James Peach <jpe...@apache.org> Date: 2016-12-14T06:11:08Z TS-5096: Lua metrics crashes if the prefix is missing. Prevent a traffic_manager crash if someone registers a metric with no prefix. We can't make that metric available in Lua, but we ough not to crash. --- If your project is set 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 #1258: TS-5089: Add money_trace plugin.
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1258 Is anyone outside of Comcast using this? Can a configurable plugin supporting [OpenTracing](http://opentracing.io) be generalized to also support Comcast? --- If your project is set 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 #1254: TS-5085 `posix_fadvise` is incorrectly used in tr...
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1254 [approve ci] --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1235: HostDB memory fixes
Github user jpeach closed the pull request at: https://github.com/apache/trafficserver/pull/1235 --- If your project is set 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 #1233: TS-5062: Fix traffic_ctl to track plugin s...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1233#discussion_r89939136 --- Diff: doc/developer-guide/api/functions/TSMgmtSourceGet.en.rst --- @@ -0,0 +1,69 @@ +.. 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:: ../../../common.defs + +.. default-domain:: c + +TSMgmtSourceGet +*** + +Synopsis + + +`#include ` + +.. function:: TSReturnCode TSMgmtSourceGet(const char * var_name, TSMgmtSource * result) + +Description +=== + +Get the source of a value for a configuration variable. :arg:`var_name` is the name of the variable +as a nul terminated string. The source value is stored in :arg:`result`. The function can return +failure if :arg:`var_name` is not found. + +Types += + +.. type:: TSMgmtSource + + Source of the current value for a management (configuration) value. + + .. macro:: TS_MGMT_SOURCE_NULL --- End diff -- Why don't you just return this instead of `TS_ERROR`? --- If your project is set 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 #1233: TS-5062: Fix traffic_ctl to track plugin s...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1233#discussion_r89939168 --- Diff: doc/developer-guide/api/functions/TSMgmtSourceGet.en.rst --- @@ -0,0 +1,69 @@ +.. 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:: ../../../common.defs + +.. default-domain:: c + +TSMgmtSourceGet +*** + +Synopsis + + +`#include ` + +.. function:: TSReturnCode TSMgmtSourceGet(const char * var_name, TSMgmtSource * result) + +Description +=== + +Get the source of a value for a configuration variable. :arg:`var_name` is the name of the variable +as a nul terminated string. The source value is stored in :arg:`result`. The function can return +failure if :arg:`var_name` is not found. + +Types += + +.. type:: TSMgmtSource + + Source of the current value for a management (configuration) value. + + .. macro:: TS_MGMT_SOURCE_NULL + + Invalid value, no source available. This is primarily used as an initialization or error value + and should be returned only when an API call fails. + + .. macro:: TS_MGMT_SOURCE_DEFAULT + + The default value provided by the |TS| core. + + .. macro:: TS_MGMT_SOURCE_PLUGIN + + The default value provided by a plugin. This means the configuration variable itself was + created by a plugin and is not part of the |TS| core and the value has not been changed from + that default. + + .. macro:: TS_MGMT_SOURCE_EXPLICIT + + The value has been set in :file:`records.config` or via an explict API call (such as + :c:func:`TSMgmtIntSet`). Note this does not guarantee the value is not the default value, as + the variable could have been set to that value. This only means an administrator or plugin + explicit set the value. + + .. macro:: TS_MGMT_SOURCE_ENV + + The value was retrived from the process environment, overriding the default value. --- End diff -- Needs a `Return` section. --- If your project is set 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 #1225: Create a tool to list files changed since a tag
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1225 Sounds like a git alias, or [git shortlog](https://git-scm.com/docs/git-shortlog). --- If your project is set 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 #1227: TS-5051 Add a new log tag % AppVersionInfo.B...
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1227 Well it is trivial to make those constants available in the Lua API. I don't think these things are worth adding log tags on their own. --- If your project is set 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 #1235: HostDB memory fixes
GitHub user jpeach opened a pull request: https://github.com/apache/trafficserver/pull/1235 HostDB memory fixes [TS-5065](https://issues.apache.org/jira/browse/TS-5066) Use after free clearing HostDB. [TS-5066](https://issues.apache.org/jira/browse/TS-5065) HostDB serialization leaks on error path. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jpeach/trafficserver fix/hostdb Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1235.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 #1235 commit f49eed1fac6f0e0161039d0b27d05957de828730 Author: James Peach <jpe...@apache.org> Date: 2016-11-26T07:09:47Z TS-5065: Fix RefCountCache iterator invalidation. Removing items from the TSHashMap invalidates the iterator because the linked list pointers are embedded in the hash node, so we can't do that while clearing. Instead, deallocate and remove each entry explicitly. commit 223f28b58b495ee326ca5ceada7947170a7f270f Author: James Peach <jpe...@apache.org> Date: 2016-11-26T19:47:04Z TS-5066: Fix HostDB memory leaks on serialization failure. If the serializer fails to write the partition, the copied entries were being leaked. Clean up the cache entry allocation to centralize the pain of it and allow both the cache and the serializer to share the same allocation and free helpers. commit 966338d52896114c0d0136b6deb8f86e6d24e89d Author: James Peach <jpe...@apache.org> Date: 2016-11-26T20:41:37Z Improve HostDB serialization warnings. --- If your project is set 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 #1234: TS-5063: Fixes coverity warnings and clean...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1234#discussion_r89402683 --- Diff: mgmt/LocalManager.cc --- @@ -950,29 +949,27 @@ LocalManager::startProxy(const char *onetime_options) if (open_ports_p) { char portbuf[128]; bool need_comma_p = false; - real_proxy_options.append(" --httpport ", strlen(" --httpport ")); + + ink_strlcat(real_proxy_options, " --httpport ", strlen(" --httpport ")); for (int i = 0, limit = m_proxy_ports.length(); i < limit; ++i) { HttpProxyPort = m_proxy_ports[i]; if (ts::NO_FD != p.m_fd) { if (need_comma_p) { -real_proxy_options.append(','); +ink_strlcat(real_proxy_options, ",", 1); } need_comma_p = true; p.print(portbuf, sizeof(portbuf)); - real_proxy_options.append((const char *)portbuf, strlen(portbuf)); + ink_strlcat(real_proxy_options, (const char *)portbuf, strlen(portbuf)); --- End diff -- The size argument to [strlcat](https://www.freebsd.org/cgi/man.cgi?query=strlcat=3=0=FreeBSD+6.2-RELEASE) is the size of the destination buffer. All these usages are passing the wrong size. --- If your project is set 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 #1233: TS-5062: Fix traffic_ctl to track plugin s...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1233#discussion_r89399145 --- Diff: lib/ts/apidefs.h.in --- @@ -798,6 +798,16 @@ typedef int64_t TSMgmtCounter; typedef float TSMgmtFloat; typedef char *TSMgmtString; +/// The source of of management value. +enum TSMgmtSource { + TS_MGMT_SOURCE_NULL, ///< No source / value not found. + TS_MGMT_SOURCE_DEFAULT, ///< Built in core default. + TS_MGMT_SOURCE_PLUGIN, ///< Plugin supplied default. + TS_MGMT_SOURCE_EXPLICIT, ///< Set by administrator (config file, external API, cluster, etc.) + TS_MGMT_SOURCE_ENV ///< Process environment variable. +}; --- End diff -- This should be a typedef for consistency with all the other TSAPI enumerations. --- If your project is set 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 #1228: TS-5058: Fix CONNECT handling without pare...
Github user jpeach closed the pull request at: https://github.com/apache/trafficserver/pull/1228 --- If your project is set 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 #1229: TS-5061 TS use ats_malloc instead of malloc in lo...
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1229 [approve ci] --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1228: TS-5058: Fix CONNECT handling without pare...
GitHub user jpeach opened a pull request: https://github.com/apache/trafficserver/pull/1228 TS-5058: Fix CONNECT handling without parent proxying. The change in TS-5040 broke direct CONNECT method handling by always attempting to forward the CONNECT request. In fact, we should only be forwarding the request if there is a parent specified or we have explicit configuration to do so. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jpeach/trafficserver fix/5058 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1228.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 #1228 commit d5944f18d99c5ff27477bb7ff6b1c9226c1d3bc3 Author: James Peach <jpe...@apache.org> Date: 2016-11-20T00:16:24Z TS-5058: Fix CONNECT handling without parent proxying. The change in TS-5040 broke direct CONNECT method handling by always attempting to forward the CONNECT request. In fact, we should only be forwarding the request if there is a parent specified or we have explicit configuration to do so. --- If your project is set 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 #1227: TS-5051 Add a new log tag % AppVersionInfo.B...
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1227 I really don't think this is worth a log tag. It is trivial to add this information as Lua constants to be consumed by `logging.config`. Alternatively, a logging tag that can log an arbitrary metric would be interesting. This is just too specific IMHO. --- If your project is set 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 #1073: TS-4399 TS-4400 Management API messes up p...
Github user jpeach closed the pull request at: https://github.com/apache/trafficserver/pull/1073 --- If your project is set 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 #1218: TS-5050: The background_fetch plugin fails...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1218#discussion_r87948595 --- Diff: plugins/background_fetch/configs.cc --- @@ -44,8 +44,13 @@ BgFetchConfig::readConfig(const char *config_file) snprintf(file_path, sizeof(file_path), "%s/%s", TSInstallDirGet(), config_file); file = TSfopen(file_path, "r"); if (nullptr == file) { - TSError("[%s] invalid config file", PLUGIN_NAME); - return false; + TSDebug(PLUGIN_NAME, "Failed to open config file %s, trying config path", config_file); + snprintf(file_path, sizeof(file_path), "%s/%s", TSConfigDirGet(), config_file); + file = TSfopen(file_path, "r"); + if (NULL == file) { +TSError("[%s] invalid config file", PLUGIN_NAME); +return false; + } --- End diff -- This should be: ```C if (*config_file == '/') { file = TSfopen(config_file, "r"); } else { file = path.join(TSConfigDirGet(), config_file); } ``` By trying the given path without qualification you allow operators to accidentally depend on the current working directory. The pseudocode above is what all the other plugins do, and I think that it is worth being consistent with this. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this 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 #1218: TS-5050: The background_fetch plugin fails to che...
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1218 @jrushford Since you are fixing this, please make sure that the new behavior is consistent with other plugins that load files. IIRC absolute paths are consumes as is, relative paths are relative to the config directory. --- If your project is set 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 #1218: TS-5050: The background_fetch plugin fails to che...
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1218 Is this change consistent with other plugins are doing? > On Nov 11, 2016, at 1:07 PM, Leif Hedstrom <notificati...@github.com> wrote: > > @zwoop approved this pull request. > > I'm ok with this, but it feels a little clunky that we duplicate the codes / error checking nested like this. Since it's C++, couldn't we iterate over a vector of candidate directories? > > â > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub, or mute the thread. > --- If your project is set 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 #1202: TS-5040: Forward CONNECT without parent pr...
Github user jpeach closed the pull request at: https://github.com/apache/trafficserver/pull/1202 --- If your project is set 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 #1211: TS-5045: Add ws and wss scheme constants.
Github user jpeach closed the pull request at: https://github.com/apache/trafficserver/pull/1211 --- If your project is set 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 #1212: TS-1257: [ssl_cipher_table] replace TCL Ha...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1212#discussion_r87022320 --- Diff: iocore/net/SSLUtils.cc --- @@ -141,8 +142,9 @@ static int ssl_vc_index = -1; static ink_mutex *mutex_buf = nullptr; static bool open_ssl_initialized = false; -RecRawStatBlock *ssl_rsb = nullptr; -static InkHashTable *ssl_cipher_name_table = nullptr; +RecRawStatBlock *ssl_rsb = nullptr; +typedef std::unordered_map cipherTable; --- End diff -- This hashes on the pointer value, not on the string value. I'm not sure that OpenSSL guarantees to give the same pointer values when you fish them out of the SSL context. --- If your project is set 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 #1211: TS-5045: Add ws and wss scheme constants.
GitHub user jpeach opened a pull request: https://github.com/apache/trafficserver/pull/1211 TS-5045: Add ws and wss scheme constants. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jpeach/trafficserver fix/5045 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1211.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 #1211 commit ffef73252f202ca380bc747fd4811d087fcec052 Author: James Peach <jpe...@apache.org> Date: 2016-11-08T05:39:16Z TS-5045: Add ws and wss scheme constants. --- If your project is set 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 #1197: TS-1257_ssl_cipher_name_table: replace TCL_HashTa...
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1197 Please reformat the PR and commit subject to the convention: ``` TS-1257: Replace TCL hash table with unordered_map. ``` --- If your project is set 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 #1202: TS-5040: Forward CONNECT without parent proxying.
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1202 @zwoop Added 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 #1201: TS-4994: make state configuration in ts_lu...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1201#discussion_r86699466 --- Diff: plugins/experimental/ts_lua/ts_lua.c --- @@ -64,18 +65,46 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char *errbuf, int errbuf_s { int fn; int ret; + int states = TS_LUA_MAX_STATE_COUNT; --- End diff -- Newline below here so clang format doesn't make it ugly ;) --- If your project is set 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 #1201: TS-4994: make state configuration in ts_lu...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1201#discussion_r86699484 --- Diff: plugins/experimental/ts_lua/ts_lua.c --- @@ -64,18 +65,46 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char *errbuf, int errbuf_s { int fn; int ret; + int states = TS_LUA_MAX_STATE_COUNT; + static const struct option longopt[] = { +{"states", required_argument, 0, 's'}, {0, 0, 0, 0}, + }; + + argc--; + argv++; + + for (;;) { +int opt; + +opt = getopt_long(argc, (char *const *)argv, "", longopt, NULL); +switch (opt) { +case 's': + states = atoi(optarg); + // set state + break; +} - if (argc < 3) { +if (opt == -1) { + break; +} + } + + if (states > TS_LUA_MAX_STATE_COUNT || states < 1) { +snprintf(errbuf, errbuf_size, "[TSRemapNewInstance] - invalid state in option input"); --- End diff -- Include the max and given values? --- If your project is set 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 #1201: TS-4994: make state configuration in ts_lu...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1201#discussion_r86699557 --- Diff: plugins/experimental/ts_lua/ts_lua.c --- @@ -391,12 +422,38 @@ TSPluginInit(int argc, const char *argv[]) return; } - if (argc < 2) { + int states = TS_LUA_MAX_STATE_COUNT; + static const struct option longopt[] = { +{"states", required_argument, 0, 's'}, {0, 0, 0, 0}, + }; + + for (;;) { +int opt; + +opt = getopt_long(argc, (char *const *)argv, "", longopt, NULL); +switch (opt) { +case 's': + states = atoi(optarg); + // set state + break; +} + +if (opt == -1) { + break; +} + } + + if (states > TS_LUA_MAX_STATE_COUNT || states < 1) { +TSError("[ts_lua][%s] invalid # of states from option input", __FUNCTION__); --- End diff -- Print the value and the max. --- If your project is set 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 #1201: TS-4994: make state configuration in ts_lu...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1201#discussion_r86699540 --- Diff: plugins/experimental/ts_lua/ts_lua.c --- @@ -391,12 +422,38 @@ TSPluginInit(int argc, const char *argv[]) return; } - if (argc < 2) { + int states = TS_LUA_MAX_STATE_COUNT; --- End diff -- Newline. --- If your project is set 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 #1202: TS-5040: Forward CONNECT without parent proxying.
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1202 Sending early for review. Will add another commit with 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 #1202: TS-5040: Forward CONNECT without parent pr...
GitHub user jpeach opened a pull request: https://github.com/apache/trafficserver/pull/1202 TS-5040: Forward CONNECT without parent proxying. Unless parent proxy is enabled, CONNECT methods always result in setting up a direct tunnel to the origin server. Parent proxy, however, has the option of forwarding the CONNECT to the next hop and allowing it to set up the tunnel. This change adds the proxy.config.http.forward_connect_method configuration parameter that can be enabled to forward CONNECT methods even if we are not parent proxying. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jpeach/trafficserver jpeach/forward-connect Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1202.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 #1202 commit 3bc7627d2db5928a685f6e0857669fa8cd0e2e60 Author: James Peach <jpe...@apache.org> Date: 2016-11-06T23:07:13Z TS-5040: Forward CONNECT without parent proxying. Unless parent proxy is enabled, CONNECT methods always result in setting up a direct tunnel to the origin server. Parent proxy, however, has the option of forwarding the CONNECT to the next hop and allowing it to set up the tunnel. This change adds the proxy.config.http.forward_connect_method configuration parameter that can be enabled to forward CONNECT methods even if we are not parent proxying. --- If your project is set 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 #1193: TS-5037: Fix LogFormat object leak.
Github user jpeach closed the pull request at: https://github.com/apache/trafficserver/pull/1193 --- If your project is set 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 #1193: TS-5037: Fix LogFormat object leak.
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1193 The `LogObject` constructor copies the format. --- If your project is set 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 #1073: TS-4399 TS-4400 Management API messes up proxy op...
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1073 @danobi Since this covers 2 JIRAs, please designate one of them for the fix, and mark the other as a duplicate. --- If your project is set 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 #1184: TS-5030: Add API to get the unique client request...
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1184 @calavera [here](https://github.com/apache/trafficserver/blob/master/proxy/InkAPI.cc#L5743) is an arena example. --- If your project is set 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 #1175: socks.config - dest_ip rule issue
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/issues/1175 Which version of Traffic Server are you running? @oknet fixed the SOCKS proxy recently and those changes will be available in the forthcoming [7.0 release](https://lists.apache.org/thread.html/7b1ce4e5ce4156fb61a2756a7073477ed759f037ceae734afc874364@%3Cdev.trafficserver.apache.org%3E). --- If your project is set 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 #1192: TS-5036: use absolute path in autoconf/aut...
Github user jpeach closed the pull request at: https://github.com/apache/trafficserver/pull/1192 --- If your project is set 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 #1191: TS-5035: plugin 'background_fetch' can be built n...
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1191 Wow, that is embarrassing. Thanks @yunwen0111! --- If your project is set 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 #1191: TS-5035: plugin 'background_fetch' can be ...
Github user jpeach closed the pull request at: https://github.com/apache/trafficserver/pull/1191 --- If your project is set 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 #1193: TS-5037: Fix LogFormat object leak.
GitHub user jpeach opened a pull request: https://github.com/apache/trafficserver/pull/1193 TS-5037: Fix LogFormat object leak. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jpeach/trafficserver jpeach/log-fmt-leak Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1193.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 #1193 commit edf48ffcc46350908bef1b961790213c3132425e Author: James Peach <jpe...@apache.org> Date: 2016-11-04T03:29:24Z TS-5037: Fix LogFormat object leak. --- If your project is set 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 #1192: TS-5036: use absolute path in autoconf/automake f...
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1192 [approve ci] --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1191: TS-5035: plugin 'background_fetch' can be built n...
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1191 [approve ci] --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1187: TS-5034: Add a --enable-tsan configure opt...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1187#discussion_r86480536 --- Diff: configure.ac --- @@ -197,6 +197,15 @@ AC_ARG_ENABLE([asan], ) AC_MSG_RESULT([$enable_asan]) +# Enable TSAN for the builds +AC_MSG_CHECKING([whether to enable tsan]) +AC_ARG_ENABLE([tsan], + [AS_HELP_STRING([--enable-tsan],[turn on tsan])], --- End diff -- "enable Thread Sanitizer" --- If your project is set 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 #1181: TS-5027: Replace readdir_r with readdir.
Github user jpeach closed the pull request at: https://github.com/apache/trafficserver/pull/1181 --- If your project is set 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 #1185: TS-5031: Use readdir() instead of readdir_r()
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1185 Dupe of #1181 :) --- If your project is set 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 #1073: TS-4399 TS-4400 Management API messes up proxy op...
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1073 [approve ci] --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1180: TS-4991: jtest should handle Range requests.
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1180 [approve ci] --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1165: TS-5023 Allow diags.log and traffic.out to be rot...
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1165 [approve ci] --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #1168: Tsqa-- switch to using build that exist
Github user jpeach commented on the issue: https://github.com/apache/trafficserver/pull/1168 /cc @dragon512 --- If your project is set 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 #1127: TS-4990: Support new apis in ts_lua.
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1127#discussion_r86285707 --- Diff: doc/admin-guide/plugins/ts_lua.en.rst --- @@ -1424,6 +1460,38 @@ We will get the output: `TOP <#ts-lua-plugin>`_ +ts.server_request.server_addr.set_addr +-- +**syntax:** *ts.server_request.server_addr.set_addr()* + +**context:** no later than function @ TS_LUA_HOOK_OS_DNS hook point + +**description**: This function can be used to set socket address of the origin server. + +The ts.server_request.server_addr.set_addr function requires three inputs, ip is a string, port and family is number. + +Here is an example: + +:: + +function do_global_read_request() +ts.server_request.server_addr.set_addr("192,168,231,17", 80, TS_LUA_AF_INET) --- End diff -- Commas? --- If your project is set 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 #1181: TS-5027: Replace readdir_r with readdir.
GitHub user jpeach opened a pull request: https://github.com/apache/trafficserver/pull/1181 TS-5027: Replace readdir_r with readdir. Glibc deprecated readdir_r(3), so replace it with readdir(3). We were already using readdir(3) in many places so this is just accepting the inevitable. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jpeach/trafficserver fix/readdir Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1181.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 #1181 commit c886cffa126c756e9729b450ffd86e444c2260d7 Author: James Peach <jpe...@apache.org> Date: 2016-11-03T04:30:47Z TS-5027: Replace readdir_r with readdir. Glibc deprecated readdir_r(3), so replace it with readdir(3). We were already using readdir(3) in many places so this is just accepting the inevitable. --- If your project is set 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 #1073: TS-4399 TS-4400 Management API messes up p...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1073#discussion_r85870712 --- Diff: mgmt/LocalManager.cc --- @@ -189,6 +189,7 @@ LocalManager::LocalManager(bool proxy_on) : BaseManager(), run_proxy(proxy_on), proxy_launch_outstanding = false; mgmt_shutdown_outstanding = MGMT_PENDING_NONE; proxy_running = 0; + coreapi_sleep = true; --- End diff -- As mentioned in the other comment, I think we can get away without this. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this 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 #1073: TS-4399 TS-4400 Management API messes up p...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1073#discussion_r85870674 --- Diff: mgmt/LocalManager.cc --- @@ -902,8 +907,13 @@ LocalManager::startProxy() Vec real_proxy_options; real_proxy_options.append(proxy_options, strlen(proxy_options)); +if (onetime_options && *onetime_options) { + real_proxy_options.append(" ", strlen(" ")); + real_proxy_options.append(onetime_options, strlen(onetime_options)); +} -if (!strstr(proxy_options, MGMT_OPT)) { // Make sure we're starting the proxy in mgmt mode +// Make sure we're starting the proxy in mgmt mode +if (!strstr(proxy_options, MGMT_OPT) && !strstr(onetime_options, MGMT_OPT)) { --- End diff -- Prefer ``strstr(...) == 0``. It's just that little bit more readable. --- If your project is set 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 #1073: TS-4399 TS-4400 Management API messes up p...
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1073#discussion_r85870603 --- Diff: mgmt/api/CoreAPI.cc --- @@ -184,21 +184,19 @@ ProxyStateSet(TSProxyStateT state, TSCacheClearT clear) ink_strlcat(tsArgs, " -k", sizeof(tsArgs)); } -if (strlen(tsArgs) > 0) { /* Passed command line args for proxy */ - ats_free(lmgmt->proxy_options); - lmgmt->proxy_options = ats_strdup(tsArgs); - mgmt_log("[ProxyStateSet] Traffic Server Args: '%s'\n", lmgmt->proxy_options); -} +mgmt_log("[ProxyStateSet] Traffic Server Args: '%s %s'\n", lmgmt->proxy_options ? lmgmt->proxy_options : "", tsArgs); lmgmt->run_proxy = true; lmgmt->listenForProxy(); +lmgmt->startProxy(tsArgs); --- End diff -- Do you actually need to do the sleeping stuff here? Since you now call ``LocalManager:: startProxy()`` directly, you have the return value to know that it succeeded. I don't think that the contract for this API needs to include waiting for a message. ```C return lmgmt->startProxy(tsArgs) ? TS_ERR_OKAY : TS_ERR_FAIL; ``` --- If your project is set 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 #1112: TS-4977: Prefer nullptr to NULL.
Github user jpeach closed the pull request at: https://github.com/apache/trafficserver/pull/1112 --- If your project is set 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 #1147: TS-4855: Make `const char` consistent acro...
Github user jpeach closed the pull request at: https://github.com/apache/trafficserver/pull/1147 --- If your project is set 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 #1127: [TS-4990] support new apis in ts_lua
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1127#discussion_r85235598 --- Diff: plugins/experimental/ts_lua/ts_lua_server_request.c --- @@ -755,3 +765,46 @@ ts_lua_server_request_server_addr_get_addr(lua_State *L) return 3; } + +static int +ts_lua_server_request_server_addr_set_addr(lua_State *L) +{ + struct sockaddr_in addr4; + struct sockaddr_in6 addr6; --- End diff -- ```C union { struct sockaddr_in sin4; struct sockaddr_in6 sin6; struct sockaddr sa; } addr; ... TSHttpTxnServerAddrSet(http_ctx->txnp, ); --- If your project is set 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 #1127: [TS-4990] support new apis in ts_lua
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1127#discussion_r85234779 --- Diff: plugins/experimental/ts_lua/ts_lua_http.c --- @@ -387,6 +411,52 @@ ts_lua_http_set_cache_lookup_url(lua_State *L) } static int +ts_lua_http_get_parent_proxy(lua_State *L) +{ + const char *hostname = NULL; + int port = 0; + ts_lua_http_ctx *http_ctx; + + GET_HTTP_CONTEXT(http_ctx, L); + + TSHttpTxnParentProxyGet(http_ctx->txnp, , ); + + if (hostname == NULL) { +lua_pushnil(L); + } else { +lua_pushstring(L, hostname); + } + lua_pushnumber(L, port); + + return 2; +} + +static int +ts_lua_http_set_parent_proxy(lua_State *L) +{ + int n = 0; + const char *hostname; + size_t hostname_len; + int port = 0; + const char *target; + + ts_lua_http_ctx *http_ctx; + + GET_HTTP_CONTEXT(http_ctx, L); + + n = lua_gettop(L); + + if (n == 2) { --- End diff -- Throw an error if `n != 2`? --- If your project is set 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. ---