SolidWallOfCode commented on a change in pull request #8335:
URL: https://github.com/apache/trafficserver/pull/8335#discussion_r744176083
##########
File path: plugins/stats_over_http/stats_over_http.cc
##########
@@ -55,8 +57,8 @@
#define SYSTEM_RECORD_TYPE (0x100)
#define DEFAULT_RECORD_TYPES (SYSTEM_RECORD_TYPE | TS_RECORDTYPE_PROCESS |
TS_RECORDTYPE_PLUGIN)
-#define DEFAULT_IP "0.0.0.0"
-#define DEFAULT_IP6 "::"
+#define DEFAULT_IP "0.0.0.0/0"
Review comment:
Use C++ constructs. Something like
```
constexpr std::string_view DEFAULT_IP = "0.0.0.0/0";
std::string const DEFAULT_IP = "0.0.0.0/0";
```
##########
File path: plugins/stats_over_http/stats_over_http.cc
##########
@@ -705,250 +718,209 @@ TSPluginInit(int argc, const char *argv[])
/* Path was not set during load, so the param was not a config file, we also
have an argument so it must be the path, set it here. Otherwise if no
argument
then use the default _stats path */
- if ((config_holder->config != NULL) && (config_holder->config->stats_path ==
0) && (argc > 0) &&
+ if ((config_holder->config != NULL) &&
(config_holder->config->stats_path.empty()) && (argc > 0) &&
(config_holder->config_path == NULL)) {
- config_holder->config->stats_path = TSstrdup(argv[0] + ('/' ==
argv[0][0] ? 1 : 0));
- config_holder->config->stats_path_len =
strlen(config_holder->config->stats_path);
- } else if ((config_holder->config != NULL) &&
(config_holder->config->stats_path == 0)) {
- config_holder->config->stats_path = nstr(DEFAULT_URL_PATH);
- config_holder->config->stats_path_len =
strlen(config_holder->config->stats_path);
+ config_holder->config->stats_path = TSstrdup(argv[0] + ('/' == argv[0][0]
? 1 : 0));
+ } else if ((config_holder->config != NULL) &&
(config_holder->config->stats_path.empty())) {
+ config_holder->config->stats_path = DEFAULT_URL_PATH;
}
/* Create a continuation with a mutex as there is a shared global structure
containing the headers to add */
- main_cont = TSContCreate(stats_origin, NULL);
+ main_cont = TSContCreate(global_stats_origin_wrapper, NULL);
TSContDataSet(main_cont, (void *)config_holder);
TSHttpHookAdd(TS_HTTP_READ_REQUEST_HDR_HOOK, main_cont);
/* Create continuation for management updates to re-read config file */
config_cont = TSContCreate(config_handler, TSMutexCreate());
TSContDataSet(config_cont, (void *)config_holder);
TSMgmtUpdateRegister(config_cont, PLUGIN_NAME);
- TSDebug(PLUGIN_NAME, "stats module registered with path %s",
config_holder->config->stats_path);
+ TSDebug(PLUGIN_NAME, "stats module registered with path %s",
config_holder->config->stats_path.c_str());
done:
return;
}
-static bool
-is_ip_match(const char *ip, char *ipmask, char mask)
+///////////////////////////////////////////////////////////////////////////////
+// Initialize the plugin as a remap plugin.
+//
+TSReturnCode
+TSRemapInit(TSRemapInterface *api_info, char *errbuf, int errbuf_size)
{
- unsigned int j, i, k;
- char cm;
- // to be able to set mask to 128
- unsigned int umask = 0xff & mask;
-
- for (j = 0, i = 0; ((i + 1) * 8) <= umask; i++) {
- if (ip[i] != ipmask[i]) {
- return false;
- }
- j += 8;
- }
- cm = 0;
- for (k = 0; j < umask; j++, k++) {
- cm |= 1 << (7 - k);
+ if (api_info->size < sizeof(TSRemapInterface)) {
+ strncpy(errbuf, "[tsremap_init] - Incorrect size of TSRemapInterface
structure", errbuf_size - 1);
+ return TS_ERROR;
}
- if ((ip[i] & cm) != (ipmask[i] & cm)) {
- return false;
+ if (api_info->tsremap_version < TSREMAP_VERSION) {
+ snprintf(errbuf, errbuf_size, "[tsremap_init] - Incorrect API version
%ld.%ld", api_info->tsremap_version >> 16,
+ (api_info->tsremap_version & 0xffff));
+ return TS_ERROR;
}
- return true;
+
+ TSDebug(PLUGIN_NAME, "remap plugin is successfully initialized");
+ return TS_SUCCESS;
}
-static bool
-is_ip_allowed(const config_t *config, const struct sockaddr *addr)
+TSReturnCode
+TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf */, int
/* errbuf_size */)
{
- char ip_port_text_buffer[INET6_ADDRSTRLEN];
- int i;
- char *ipmask;
- if (!addr) {
- return true;
+ config_holder_t *config_holder;
+ config_holder = new_config_holder(argc > 2 ? argv[2] : NULL);
+ *ih = static_cast<void *>(config_holder);
+ /* Path was not set during load, so the param was not a config file, we also
+ have an argument so it must be the path, set it here. Otherwise if no
argument
+ then use the default _stats path */
+ if ((config_holder->config != NULL) &&
(config_holder->config->stats_path.empty()) && (argc > 2) &&
+ (config_holder->config_path == NULL)) {
+ config_holder->config->stats_path = TSstrdup(argv[2] + ('/' == argv[2][0]
? 1 : 0));
Review comment:
Why not make `stats_path` a `std::string` and avoid calling not only
`TSstrdup` but `TSfree`?
##########
File path: plugins/stats_over_http/stats_over_http.cc
##########
@@ -705,250 +718,209 @@ TSPluginInit(int argc, const char *argv[])
/* Path was not set during load, so the param was not a config file, we also
have an argument so it must be the path, set it here. Otherwise if no
argument
then use the default _stats path */
- if ((config_holder->config != NULL) && (config_holder->config->stats_path ==
0) && (argc > 0) &&
+ if ((config_holder->config != NULL) &&
(config_holder->config->stats_path.empty()) && (argc > 0) &&
(config_holder->config_path == NULL)) {
- config_holder->config->stats_path = TSstrdup(argv[0] + ('/' ==
argv[0][0] ? 1 : 0));
- config_holder->config->stats_path_len =
strlen(config_holder->config->stats_path);
- } else if ((config_holder->config != NULL) &&
(config_holder->config->stats_path == 0)) {
- config_holder->config->stats_path = nstr(DEFAULT_URL_PATH);
- config_holder->config->stats_path_len =
strlen(config_holder->config->stats_path);
+ config_holder->config->stats_path = TSstrdup(argv[0] + ('/' == argv[0][0]
? 1 : 0));
+ } else if ((config_holder->config != NULL) &&
(config_holder->config->stats_path.empty())) {
+ config_holder->config->stats_path = DEFAULT_URL_PATH;
}
/* Create a continuation with a mutex as there is a shared global structure
containing the headers to add */
- main_cont = TSContCreate(stats_origin, NULL);
+ main_cont = TSContCreate(global_stats_origin_wrapper, NULL);
TSContDataSet(main_cont, (void *)config_holder);
TSHttpHookAdd(TS_HTTP_READ_REQUEST_HDR_HOOK, main_cont);
/* Create continuation for management updates to re-read config file */
config_cont = TSContCreate(config_handler, TSMutexCreate());
TSContDataSet(config_cont, (void *)config_holder);
TSMgmtUpdateRegister(config_cont, PLUGIN_NAME);
- TSDebug(PLUGIN_NAME, "stats module registered with path %s",
config_holder->config->stats_path);
+ TSDebug(PLUGIN_NAME, "stats module registered with path %s",
config_holder->config->stats_path.c_str());
done:
return;
}
-static bool
-is_ip_match(const char *ip, char *ipmask, char mask)
+///////////////////////////////////////////////////////////////////////////////
+// Initialize the plugin as a remap plugin.
+//
+TSReturnCode
+TSRemapInit(TSRemapInterface *api_info, char *errbuf, int errbuf_size)
{
- unsigned int j, i, k;
- char cm;
- // to be able to set mask to 128
- unsigned int umask = 0xff & mask;
-
- for (j = 0, i = 0; ((i + 1) * 8) <= umask; i++) {
- if (ip[i] != ipmask[i]) {
- return false;
- }
- j += 8;
- }
- cm = 0;
- for (k = 0; j < umask; j++, k++) {
- cm |= 1 << (7 - k);
+ if (api_info->size < sizeof(TSRemapInterface)) {
+ strncpy(errbuf, "[tsremap_init] - Incorrect size of TSRemapInterface
structure", errbuf_size - 1);
+ return TS_ERROR;
}
- if ((ip[i] & cm) != (ipmask[i] & cm)) {
- return false;
+ if (api_info->tsremap_version < TSREMAP_VERSION) {
+ snprintf(errbuf, errbuf_size, "[tsremap_init] - Incorrect API version
%ld.%ld", api_info->tsremap_version >> 16,
+ (api_info->tsremap_version & 0xffff));
+ return TS_ERROR;
}
- return true;
+
+ TSDebug(PLUGIN_NAME, "remap plugin is successfully initialized");
+ return TS_SUCCESS;
}
-static bool
-is_ip_allowed(const config_t *config, const struct sockaddr *addr)
+TSReturnCode
+TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf */, int
/* errbuf_size */)
{
- char ip_port_text_buffer[INET6_ADDRSTRLEN];
- int i;
- char *ipmask;
- if (!addr) {
- return true;
+ config_holder_t *config_holder;
+ config_holder = new_config_holder(argc > 2 ? argv[2] : NULL);
+ *ih = static_cast<void *>(config_holder);
+ /* Path was not set during load, so the param was not a config file, we also
+ have an argument so it must be the path, set it here. Otherwise if no
argument
+ then use the default _stats path */
+ if ((config_holder->config != NULL) &&
(config_holder->config->stats_path.empty()) && (argc > 2) &&
+ (config_holder->config_path == NULL)) {
+ config_holder->config->stats_path = TSstrdup(argv[2] + ('/' == argv[2][0]
? 1 : 0));
+ } else if ((config_holder->config != NULL) &&
(config_holder->config->stats_path.empty())) {
+ config_holder->config->stats_path = DEFAULT_URL_PATH;
}
- if (addr->sa_family == AF_INET && config->allowIps) {
- const struct sockaddr_in *addr_in = (struct sockaddr_in *)addr;
- const char *ip = (char *)&addr_in->sin_addr;
+ // Associate our config file with remap.config to be able to initiate reloads
+ if (config_holder->config_path != NULL) {
+ TSMgmtString result;
+ const char *var_name = "proxy.config.url_remap.filename";
+ TSMgmtStringGet(var_name, &result);
+ TSMgmtConfigFileAdd(result, config_holder->config_path);
+ TSDebug(PLUGIN_NAME, "Registered config file %s with remap reload",
config_holder->config_path);
+ }
- for (i = 0; i < config->ipCount; i++) {
- ipmask = config->allowIps + (i * (sizeof(struct in_addr) + 1));
- if (is_ip_match(ip, ipmask, ipmask[4])) {
- TSDebug(PLUGIN_NAME, "clientip is %s--> ALLOW", inet_ntop(AF_INET, ip,
ip_port_text_buffer, INET6_ADDRSTRLEN));
- return true;
- }
- }
- TSDebug(PLUGIN_NAME, "clientip is %s--> DENY", inet_ntop(AF_INET, ip,
ip_port_text_buffer, INET6_ADDRSTRLEN));
- return false;
-
- } else if (addr->sa_family == AF_INET6 && config->allowIps6) {
- const struct sockaddr_in6 *addr_in6 = (struct sockaddr_in6 *)addr;
- const char *ip = (char *)&addr_in6->sin6_addr;
-
- for (i = 0; i < config->ip6Count; i++) {
- ipmask = config->allowIps6 + (i * (sizeof(struct in6_addr) + 1));
- if (is_ip_match(ip, ipmask, ipmask[sizeof(struct in6_addr)])) {
- TSDebug(PLUGIN_NAME, "clientip6 is %s--> ALLOW", inet_ntop(AF_INET6,
ip, ip_port_text_buffer, INET6_ADDRSTRLEN));
- return true;
- }
- }
- TSDebug(PLUGIN_NAME, "clientip6 is %s--> DENY", inet_ntop(AF_INET6, ip,
ip_port_text_buffer, INET6_ADDRSTRLEN));
- return false;
+ if (config_holder->config != nullptr) {
+ TSDebug(PLUGIN_NAME, "New remap instance for %s on path %s", PLUGIN_NAME,
config_holder->config->stats_path.c_str());
+ return TS_SUCCESS;
+ } else {
+ TSDebug(PLUGIN_NAME, "Failed to create stats_over_http instance, config is
null");
+ return TS_ERROR;
}
- return true;
}
-static void
-parseIps(config_t *config, char *ipStr)
+void
+TSRemapDeleteInstance(void *ih)
{
- char buffer[STR_BUFFER_SIZE];
- char *p, *tok1, *tok2, *ip;
- int i, mask;
- char ip_port_text_buffer[INET_ADDRSTRLEN];
-
- if (!ipStr) {
- config->ipCount = 1;
- ip = config->allowIps = TSmalloc(sizeof(struct in_addr) + 1);
- inet_pton(AF_INET, DEFAULT_IP, ip);
- ip[4] = 0;
- return;
+ if (nullptr != ih) {
+ config_holder_t *const config = static_cast<config_holder_t *>(ih);
+ TSDebug(PLUGIN_NAME, "Freeing config");
+ TSfree(config);
}
+}
- strcpy(buffer, ipStr);
- p = buffer;
- while (strtok_r(p, ", \n", &p)) {
- config->ipCount++;
+///////////////////////////////////////////////////////////////////////////////
+// Main entry point when used as a remap plugin.
+//
+TSRemapStatus
+TSRemapDoRemap(void *ih, TSHttpTxn rh, TSRemapRequestInfo *rri)
+{
+ if (nullptr != ih) {
+ config_holder_t *const config = static_cast<config_holder_t *>(ih);
+ TSEvent event = TS_EVENT_NONE;
+ stats_origin(config->config, event, (void *)rh);
}
- if (!config->ipCount) {
- return;
+ return TSREMAP_NO_REMAP;
+}
+
+static bool
+is_ipmap_allowed(const config_t *config, const struct sockaddr *addr)
+{
+ if (!addr) {
+ return true;
}
- config->allowIps = TSmalloc(5 * config->ipCount); // 4 bytes for ip + 1 for
bit mask
- strcpy(buffer, ipStr);
- p = buffer;
- i = 0;
- while ((tok1 = strtok_r(p, ", \n", &p))) {
- TSDebug(PLUGIN_NAME, "%d) parsing: %s", i + 1, tok1);
- tok2 = strtok_r(tok1, "/", &tok1);
- ip = config->allowIps + ((sizeof(struct in_addr) + 1) * i);
- if (!inet_pton(AF_INET, tok2, ip)) {
- TSDebug(PLUGIN_NAME, "%d) skipping: %s", i + 1, tok1);
- continue;
- }
- if (tok1 != NULL) {
- tok2 = strtok_r(tok1, "/", &tok1);
- }
- if (!tok2) {
- mask = 32;
- } else {
- mask = atoi(tok2);
- }
- ip[4] = mask;
- TSDebug(PLUGIN_NAME, "%d) adding netmask: %s/%d", i + 1,
inet_ntop(AF_INET, ip, ip_port_text_buffer, INET_ADDRSTRLEN), ip[4]);
- i++;
+ if (config->allow_ip_map.contains(addr, nullptr)) {
+ return true;
}
+
+ return false;
}
static void
-parseIps6(config_t *config, char *ipStr)
+parseIpMap(config_t *config, const char *ipStr)
{
- char buffer[STR_BUFFER_SIZE];
- char *p, *tok1, *tok2, *ip;
- int i, mask;
- char ip_port_text_buffer[INET6_ADDRSTRLEN];
-
- if (!ipStr) {
- config->ip6Count = 1;
- ip = config->allowIps6 = TSmalloc(sizeof(struct in6_addr) + 1);
- inet_pton(AF_INET6, DEFAULT_IP6, ip);
- ip[sizeof(struct in6_addr)] = 0;
- return;
+ IpAddr min, max;
+ std::string delimiter = ",";
+ size_t pos_start = 0, pos_end, delimiter_len = delimiter.length();
+ std::string token;
Review comment:
`ats_ip_range_parse` takes a `string_view` - why aren't these
`string_view`?
##########
File path: plugins/stats_over_http/stats_over_http.cc
##########
@@ -705,250 +718,209 @@ TSPluginInit(int argc, const char *argv[])
/* Path was not set during load, so the param was not a config file, we also
have an argument so it must be the path, set it here. Otherwise if no
argument
then use the default _stats path */
- if ((config_holder->config != NULL) && (config_holder->config->stats_path ==
0) && (argc > 0) &&
+ if ((config_holder->config != NULL) &&
(config_holder->config->stats_path.empty()) && (argc > 0) &&
Review comment:
`NULL` -> `nullptr`
##########
File path: plugins/stats_over_http/stats_over_http.cc
##########
@@ -656,7 +659,17 @@ stats_origin(TSCont contp ATS_UNUSED, TSEvent event
ATS_UNUSED, void *edata)
if (accept_encoding_field) {
TSHandleMLocRelease(reqp, TS_NULL_MLOC, accept_encoding_field);
}
- TSHttpTxnReenable(txnp, reenable);
+
+ return reenable;
+}
+
+static int
+global_stats_origin_wrapper(TSCont contp ATS_UNUSED, TSEvent event ATS_UNUSED,
void *edata)
Review comment:
Don't use `ATS_UNUSED`, just omit the name entirely.
##########
File path: doc/admin-guide/plugins/stats_over_http.en.rst
##########
@@ -41,9 +41,16 @@ default URL::
where host and port is the hostname/IP and port number of the server.
+This plugin can also run as a :file:`remap.config` plugin::
+
+ map http://incoming/ http://outgoing/ @plugin=stats_over_http.so
@pparam=(empty/undefined || desired stats URL path || location of config file)
Review comment:
Is `@pparam` required in the empty case?
##########
File path: plugins/stats_over_http/stats_over_http.cc
##########
@@ -89,12 +91,9 @@ static bool wrap_counters = false;
typedef struct {
Review comment:
No `typedef`!
```
struct config_t { ... };
```
##########
File path: plugins/stats_over_http/stats_over_http.cc
##########
@@ -705,250 +718,209 @@ TSPluginInit(int argc, const char *argv[])
/* Path was not set during load, so the param was not a config file, we also
have an argument so it must be the path, set it here. Otherwise if no
argument
then use the default _stats path */
- if ((config_holder->config != NULL) && (config_holder->config->stats_path ==
0) && (argc > 0) &&
+ if ((config_holder->config != NULL) &&
(config_holder->config->stats_path.empty()) && (argc > 0) &&
(config_holder->config_path == NULL)) {
- config_holder->config->stats_path = TSstrdup(argv[0] + ('/' ==
argv[0][0] ? 1 : 0));
- config_holder->config->stats_path_len =
strlen(config_holder->config->stats_path);
- } else if ((config_holder->config != NULL) &&
(config_holder->config->stats_path == 0)) {
- config_holder->config->stats_path = nstr(DEFAULT_URL_PATH);
- config_holder->config->stats_path_len =
strlen(config_holder->config->stats_path);
+ config_holder->config->stats_path = TSstrdup(argv[0] + ('/' == argv[0][0]
? 1 : 0));
+ } else if ((config_holder->config != NULL) &&
(config_holder->config->stats_path.empty())) {
+ config_holder->config->stats_path = DEFAULT_URL_PATH;
}
/* Create a continuation with a mutex as there is a shared global structure
containing the headers to add */
- main_cont = TSContCreate(stats_origin, NULL);
+ main_cont = TSContCreate(global_stats_origin_wrapper, NULL);
TSContDataSet(main_cont, (void *)config_holder);
TSHttpHookAdd(TS_HTTP_READ_REQUEST_HDR_HOOK, main_cont);
/* Create continuation for management updates to re-read config file */
config_cont = TSContCreate(config_handler, TSMutexCreate());
TSContDataSet(config_cont, (void *)config_holder);
TSMgmtUpdateRegister(config_cont, PLUGIN_NAME);
- TSDebug(PLUGIN_NAME, "stats module registered with path %s",
config_holder->config->stats_path);
+ TSDebug(PLUGIN_NAME, "stats module registered with path %s",
config_holder->config->stats_path.c_str());
done:
return;
}
-static bool
-is_ip_match(const char *ip, char *ipmask, char mask)
+///////////////////////////////////////////////////////////////////////////////
+// Initialize the plugin as a remap plugin.
+//
+TSReturnCode
+TSRemapInit(TSRemapInterface *api_info, char *errbuf, int errbuf_size)
{
- unsigned int j, i, k;
- char cm;
- // to be able to set mask to 128
- unsigned int umask = 0xff & mask;
-
- for (j = 0, i = 0; ((i + 1) * 8) <= umask; i++) {
- if (ip[i] != ipmask[i]) {
- return false;
- }
- j += 8;
- }
- cm = 0;
- for (k = 0; j < umask; j++, k++) {
- cm |= 1 << (7 - k);
+ if (api_info->size < sizeof(TSRemapInterface)) {
+ strncpy(errbuf, "[tsremap_init] - Incorrect size of TSRemapInterface
structure", errbuf_size - 1);
+ return TS_ERROR;
}
- if ((ip[i] & cm) != (ipmask[i] & cm)) {
- return false;
+ if (api_info->tsremap_version < TSREMAP_VERSION) {
+ snprintf(errbuf, errbuf_size, "[tsremap_init] - Incorrect API version
%ld.%ld", api_info->tsremap_version >> 16,
+ (api_info->tsremap_version & 0xffff));
+ return TS_ERROR;
}
- return true;
+
+ TSDebug(PLUGIN_NAME, "remap plugin is successfully initialized");
+ return TS_SUCCESS;
}
-static bool
-is_ip_allowed(const config_t *config, const struct sockaddr *addr)
+TSReturnCode
+TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf */, int
/* errbuf_size */)
{
- char ip_port_text_buffer[INET6_ADDRSTRLEN];
- int i;
- char *ipmask;
- if (!addr) {
- return true;
+ config_holder_t *config_holder;
+ config_holder = new_config_holder(argc > 2 ? argv[2] : NULL);
+ *ih = static_cast<void *>(config_holder);
+ /* Path was not set during load, so the param was not a config file, we also
+ have an argument so it must be the path, set it here. Otherwise if no
argument
+ then use the default _stats path */
+ if ((config_holder->config != NULL) &&
(config_holder->config->stats_path.empty()) && (argc > 2) &&
+ (config_holder->config_path == NULL)) {
+ config_holder->config->stats_path = TSstrdup(argv[2] + ('/' == argv[2][0]
? 1 : 0));
+ } else if ((config_holder->config != NULL) &&
(config_holder->config->stats_path.empty())) {
+ config_holder->config->stats_path = DEFAULT_URL_PATH;
}
- if (addr->sa_family == AF_INET && config->allowIps) {
- const struct sockaddr_in *addr_in = (struct sockaddr_in *)addr;
- const char *ip = (char *)&addr_in->sin_addr;
+ // Associate our config file with remap.config to be able to initiate reloads
+ if (config_holder->config_path != NULL) {
+ TSMgmtString result;
+ const char *var_name = "proxy.config.url_remap.filename";
+ TSMgmtStringGet(var_name, &result);
+ TSMgmtConfigFileAdd(result, config_holder->config_path);
+ TSDebug(PLUGIN_NAME, "Registered config file %s with remap reload",
config_holder->config_path);
+ }
- for (i = 0; i < config->ipCount; i++) {
- ipmask = config->allowIps + (i * (sizeof(struct in_addr) + 1));
- if (is_ip_match(ip, ipmask, ipmask[4])) {
- TSDebug(PLUGIN_NAME, "clientip is %s--> ALLOW", inet_ntop(AF_INET, ip,
ip_port_text_buffer, INET6_ADDRSTRLEN));
- return true;
- }
- }
- TSDebug(PLUGIN_NAME, "clientip is %s--> DENY", inet_ntop(AF_INET, ip,
ip_port_text_buffer, INET6_ADDRSTRLEN));
- return false;
-
- } else if (addr->sa_family == AF_INET6 && config->allowIps6) {
- const struct sockaddr_in6 *addr_in6 = (struct sockaddr_in6 *)addr;
- const char *ip = (char *)&addr_in6->sin6_addr;
-
- for (i = 0; i < config->ip6Count; i++) {
- ipmask = config->allowIps6 + (i * (sizeof(struct in6_addr) + 1));
- if (is_ip_match(ip, ipmask, ipmask[sizeof(struct in6_addr)])) {
- TSDebug(PLUGIN_NAME, "clientip6 is %s--> ALLOW", inet_ntop(AF_INET6,
ip, ip_port_text_buffer, INET6_ADDRSTRLEN));
- return true;
- }
- }
- TSDebug(PLUGIN_NAME, "clientip6 is %s--> DENY", inet_ntop(AF_INET6, ip,
ip_port_text_buffer, INET6_ADDRSTRLEN));
- return false;
+ if (config_holder->config != nullptr) {
+ TSDebug(PLUGIN_NAME, "New remap instance for %s on path %s", PLUGIN_NAME,
config_holder->config->stats_path.c_str());
+ return TS_SUCCESS;
+ } else {
+ TSDebug(PLUGIN_NAME, "Failed to create stats_over_http instance, config is
null");
+ return TS_ERROR;
}
- return true;
}
-static void
-parseIps(config_t *config, char *ipStr)
+void
+TSRemapDeleteInstance(void *ih)
{
- char buffer[STR_BUFFER_SIZE];
- char *p, *tok1, *tok2, *ip;
- int i, mask;
- char ip_port_text_buffer[INET_ADDRSTRLEN];
-
- if (!ipStr) {
- config->ipCount = 1;
- ip = config->allowIps = TSmalloc(sizeof(struct in_addr) + 1);
- inet_pton(AF_INET, DEFAULT_IP, ip);
- ip[4] = 0;
- return;
+ if (nullptr != ih) {
+ config_holder_t *const config = static_cast<config_holder_t *>(ih);
+ TSDebug(PLUGIN_NAME, "Freeing config");
+ TSfree(config);
}
+}
- strcpy(buffer, ipStr);
- p = buffer;
- while (strtok_r(p, ", \n", &p)) {
- config->ipCount++;
+///////////////////////////////////////////////////////////////////////////////
+// Main entry point when used as a remap plugin.
+//
+TSRemapStatus
+TSRemapDoRemap(void *ih, TSHttpTxn rh, TSRemapRequestInfo *rri)
+{
+ if (nullptr != ih) {
+ config_holder_t *const config = static_cast<config_holder_t *>(ih);
+ TSEvent event = TS_EVENT_NONE;
+ stats_origin(config->config, event, (void *)rh);
}
- if (!config->ipCount) {
- return;
+ return TSREMAP_NO_REMAP;
+}
+
+static bool
+is_ipmap_allowed(const config_t *config, const struct sockaddr *addr)
+{
+ if (!addr) {
+ return true;
}
- config->allowIps = TSmalloc(5 * config->ipCount); // 4 bytes for ip + 1 for
bit mask
- strcpy(buffer, ipStr);
- p = buffer;
- i = 0;
- while ((tok1 = strtok_r(p, ", \n", &p))) {
- TSDebug(PLUGIN_NAME, "%d) parsing: %s", i + 1, tok1);
- tok2 = strtok_r(tok1, "/", &tok1);
- ip = config->allowIps + ((sizeof(struct in_addr) + 1) * i);
- if (!inet_pton(AF_INET, tok2, ip)) {
- TSDebug(PLUGIN_NAME, "%d) skipping: %s", i + 1, tok1);
- continue;
- }
- if (tok1 != NULL) {
- tok2 = strtok_r(tok1, "/", &tok1);
- }
- if (!tok2) {
- mask = 32;
- } else {
- mask = atoi(tok2);
- }
- ip[4] = mask;
- TSDebug(PLUGIN_NAME, "%d) adding netmask: %s/%d", i + 1,
inet_ntop(AF_INET, ip, ip_port_text_buffer, INET_ADDRSTRLEN), ip[4]);
- i++;
+ if (config->allow_ip_map.contains(addr, nullptr)) {
+ return true;
}
+
+ return false;
}
static void
-parseIps6(config_t *config, char *ipStr)
+parseIpMap(config_t *config, const char *ipStr)
{
- char buffer[STR_BUFFER_SIZE];
- char *p, *tok1, *tok2, *ip;
- int i, mask;
- char ip_port_text_buffer[INET6_ADDRSTRLEN];
-
- if (!ipStr) {
- config->ip6Count = 1;
- ip = config->allowIps6 = TSmalloc(sizeof(struct in6_addr) + 1);
- inet_pton(AF_INET6, DEFAULT_IP6, ip);
- ip[sizeof(struct in6_addr)] = 0;
- return;
+ IpAddr min, max;
+ std::string delimiter = ",";
+ size_t pos_start = 0, pos_end, delimiter_len = delimiter.length();
+ std::string token;
+ std::string ipstring("");
+
+ if (ipStr != 0) {
+ ipstring.assign(ipStr);
}
- strcpy(buffer, ipStr);
- p = buffer;
- while (strtok_r(p, ", \n", &p)) {
- config->ip6Count++;
- }
- if (!config->ip6Count) {
+ // sent null ipstring, fill with default open IPs
+ if (ipStr == nullptr) {
+ ats_ip_range_parse(std::string_view{DEFAULT_IP6}, min, max);
+ config->allow_ip_map.fill(min, max, nullptr);
+ ats_ip_range_parse(std::string_view{DEFAULT_IP}, min, max);
+ config->allow_ip_map.fill(min, max, nullptr);
+ TSDebug(PLUGIN_NAME, "Empty allow settings, setting all IPs in allow
list");
return;
}
- config->allowIps6 = TSmalloc((sizeof(struct in6_addr) + 1) *
config->ip6Count); // 16 bytes for ip + 1 for bit mask
- strcpy(buffer, ipStr);
- p = buffer;
- i = 0;
- while ((tok1 = strtok_r(p, ", \n", &p))) {
- TSDebug(PLUGIN_NAME, "%d) parsing: %s", i + 1, tok1);
- tok2 = strtok_r(tok1, "/", &tok1);
- ip = config->allowIps6 + ((sizeof(struct in6_addr) + 1) * i);
- if (!inet_pton(AF_INET6, tok2, ip)) {
- TSDebug(PLUGIN_NAME, "%d) skipping: %s", i + 1, tok1);
- continue;
- }
-
- if (tok1 != NULL) {
- tok2 = strtok_r(tok1, "/", &tok1);
- }
-
- if (!tok2) {
- mask = 128;
- } else {
- mask = atoi(tok2);
- }
- ip[sizeof(struct in6_addr)] = mask;
- TSDebug(PLUGIN_NAME, "%d) adding netmask: %s/%d", i + 1,
inet_ntop(AF_INET6, ip, ip_port_text_buffer, INET6_ADDRSTRLEN),
- ip[sizeof(struct in6_addr)]);
- i++;
+ while ((pos_end = ipstring.find(delimiter, pos_start)) != std::string::npos)
{
Review comment:
`TextView`?
```
while (ipstring) {
token = ipstring.take_prefix_at(',');
```
##########
File path: plugins/stats_over_http/stats_over_http.cc
##########
@@ -705,250 +718,209 @@ TSPluginInit(int argc, const char *argv[])
/* Path was not set during load, so the param was not a config file, we also
have an argument so it must be the path, set it here. Otherwise if no
argument
then use the default _stats path */
- if ((config_holder->config != NULL) && (config_holder->config->stats_path ==
0) && (argc > 0) &&
+ if ((config_holder->config != NULL) &&
(config_holder->config->stats_path.empty()) && (argc > 0) &&
(config_holder->config_path == NULL)) {
- config_holder->config->stats_path = TSstrdup(argv[0] + ('/' ==
argv[0][0] ? 1 : 0));
- config_holder->config->stats_path_len =
strlen(config_holder->config->stats_path);
- } else if ((config_holder->config != NULL) &&
(config_holder->config->stats_path == 0)) {
- config_holder->config->stats_path = nstr(DEFAULT_URL_PATH);
- config_holder->config->stats_path_len =
strlen(config_holder->config->stats_path);
+ config_holder->config->stats_path = TSstrdup(argv[0] + ('/' == argv[0][0]
? 1 : 0));
+ } else if ((config_holder->config != NULL) &&
(config_holder->config->stats_path.empty())) {
+ config_holder->config->stats_path = DEFAULT_URL_PATH;
}
/* Create a continuation with a mutex as there is a shared global structure
containing the headers to add */
- main_cont = TSContCreate(stats_origin, NULL);
+ main_cont = TSContCreate(global_stats_origin_wrapper, NULL);
TSContDataSet(main_cont, (void *)config_holder);
TSHttpHookAdd(TS_HTTP_READ_REQUEST_HDR_HOOK, main_cont);
/* Create continuation for management updates to re-read config file */
config_cont = TSContCreate(config_handler, TSMutexCreate());
TSContDataSet(config_cont, (void *)config_holder);
TSMgmtUpdateRegister(config_cont, PLUGIN_NAME);
- TSDebug(PLUGIN_NAME, "stats module registered with path %s",
config_holder->config->stats_path);
+ TSDebug(PLUGIN_NAME, "stats module registered with path %s",
config_holder->config->stats_path.c_str());
done:
return;
}
-static bool
-is_ip_match(const char *ip, char *ipmask, char mask)
+///////////////////////////////////////////////////////////////////////////////
+// Initialize the plugin as a remap plugin.
+//
+TSReturnCode
+TSRemapInit(TSRemapInterface *api_info, char *errbuf, int errbuf_size)
{
- unsigned int j, i, k;
- char cm;
- // to be able to set mask to 128
- unsigned int umask = 0xff & mask;
-
- for (j = 0, i = 0; ((i + 1) * 8) <= umask; i++) {
- if (ip[i] != ipmask[i]) {
- return false;
- }
- j += 8;
- }
- cm = 0;
- for (k = 0; j < umask; j++, k++) {
- cm |= 1 << (7 - k);
+ if (api_info->size < sizeof(TSRemapInterface)) {
+ strncpy(errbuf, "[tsremap_init] - Incorrect size of TSRemapInterface
structure", errbuf_size - 1);
+ return TS_ERROR;
}
- if ((ip[i] & cm) != (ipmask[i] & cm)) {
- return false;
+ if (api_info->tsremap_version < TSREMAP_VERSION) {
+ snprintf(errbuf, errbuf_size, "[tsremap_init] - Incorrect API version
%ld.%ld", api_info->tsremap_version >> 16,
+ (api_info->tsremap_version & 0xffff));
+ return TS_ERROR;
}
- return true;
+
+ TSDebug(PLUGIN_NAME, "remap plugin is successfully initialized");
+ return TS_SUCCESS;
}
-static bool
-is_ip_allowed(const config_t *config, const struct sockaddr *addr)
+TSReturnCode
+TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf */, int
/* errbuf_size */)
{
- char ip_port_text_buffer[INET6_ADDRSTRLEN];
- int i;
- char *ipmask;
- if (!addr) {
- return true;
+ config_holder_t *config_holder;
+ config_holder = new_config_holder(argc > 2 ? argv[2] : NULL);
+ *ih = static_cast<void *>(config_holder);
+ /* Path was not set during load, so the param was not a config file, we also
+ have an argument so it must be the path, set it here. Otherwise if no
argument
+ then use the default _stats path */
+ if ((config_holder->config != NULL) &&
(config_holder->config->stats_path.empty()) && (argc > 2) &&
+ (config_holder->config_path == NULL)) {
+ config_holder->config->stats_path = TSstrdup(argv[2] + ('/' == argv[2][0]
? 1 : 0));
+ } else if ((config_holder->config != NULL) &&
(config_holder->config->stats_path.empty())) {
+ config_holder->config->stats_path = DEFAULT_URL_PATH;
}
- if (addr->sa_family == AF_INET && config->allowIps) {
- const struct sockaddr_in *addr_in = (struct sockaddr_in *)addr;
- const char *ip = (char *)&addr_in->sin_addr;
+ // Associate our config file with remap.config to be able to initiate reloads
+ if (config_holder->config_path != NULL) {
+ TSMgmtString result;
+ const char *var_name = "proxy.config.url_remap.filename";
+ TSMgmtStringGet(var_name, &result);
+ TSMgmtConfigFileAdd(result, config_holder->config_path);
+ TSDebug(PLUGIN_NAME, "Registered config file %s with remap reload",
config_holder->config_path);
+ }
- for (i = 0; i < config->ipCount; i++) {
- ipmask = config->allowIps + (i * (sizeof(struct in_addr) + 1));
- if (is_ip_match(ip, ipmask, ipmask[4])) {
- TSDebug(PLUGIN_NAME, "clientip is %s--> ALLOW", inet_ntop(AF_INET, ip,
ip_port_text_buffer, INET6_ADDRSTRLEN));
- return true;
- }
- }
- TSDebug(PLUGIN_NAME, "clientip is %s--> DENY", inet_ntop(AF_INET, ip,
ip_port_text_buffer, INET6_ADDRSTRLEN));
- return false;
-
- } else if (addr->sa_family == AF_INET6 && config->allowIps6) {
- const struct sockaddr_in6 *addr_in6 = (struct sockaddr_in6 *)addr;
- const char *ip = (char *)&addr_in6->sin6_addr;
-
- for (i = 0; i < config->ip6Count; i++) {
- ipmask = config->allowIps6 + (i * (sizeof(struct in6_addr) + 1));
- if (is_ip_match(ip, ipmask, ipmask[sizeof(struct in6_addr)])) {
- TSDebug(PLUGIN_NAME, "clientip6 is %s--> ALLOW", inet_ntop(AF_INET6,
ip, ip_port_text_buffer, INET6_ADDRSTRLEN));
- return true;
- }
- }
- TSDebug(PLUGIN_NAME, "clientip6 is %s--> DENY", inet_ntop(AF_INET6, ip,
ip_port_text_buffer, INET6_ADDRSTRLEN));
- return false;
+ if (config_holder->config != nullptr) {
+ TSDebug(PLUGIN_NAME, "New remap instance for %s on path %s", PLUGIN_NAME,
config_holder->config->stats_path.c_str());
+ return TS_SUCCESS;
+ } else {
+ TSDebug(PLUGIN_NAME, "Failed to create stats_over_http instance, config is
null");
+ return TS_ERROR;
}
- return true;
}
-static void
-parseIps(config_t *config, char *ipStr)
+void
+TSRemapDeleteInstance(void *ih)
{
- char buffer[STR_BUFFER_SIZE];
- char *p, *tok1, *tok2, *ip;
- int i, mask;
- char ip_port_text_buffer[INET_ADDRSTRLEN];
-
- if (!ipStr) {
- config->ipCount = 1;
- ip = config->allowIps = TSmalloc(sizeof(struct in_addr) + 1);
- inet_pton(AF_INET, DEFAULT_IP, ip);
- ip[4] = 0;
- return;
+ if (nullptr != ih) {
+ config_holder_t *const config = static_cast<config_holder_t *>(ih);
+ TSDebug(PLUGIN_NAME, "Freeing config");
+ TSfree(config);
}
+}
- strcpy(buffer, ipStr);
- p = buffer;
- while (strtok_r(p, ", \n", &p)) {
- config->ipCount++;
+///////////////////////////////////////////////////////////////////////////////
+// Main entry point when used as a remap plugin.
+//
+TSRemapStatus
+TSRemapDoRemap(void *ih, TSHttpTxn rh, TSRemapRequestInfo *rri)
+{
+ if (nullptr != ih) {
+ config_holder_t *const config = static_cast<config_holder_t *>(ih);
+ TSEvent event = TS_EVENT_NONE;
+ stats_origin(config->config, event, (void *)rh);
}
- if (!config->ipCount) {
- return;
+ return TSREMAP_NO_REMAP;
+}
+
+static bool
+is_ipmap_allowed(const config_t *config, const struct sockaddr *addr)
+{
+ if (!addr) {
+ return true;
}
- config->allowIps = TSmalloc(5 * config->ipCount); // 4 bytes for ip + 1 for
bit mask
- strcpy(buffer, ipStr);
- p = buffer;
- i = 0;
- while ((tok1 = strtok_r(p, ", \n", &p))) {
- TSDebug(PLUGIN_NAME, "%d) parsing: %s", i + 1, tok1);
- tok2 = strtok_r(tok1, "/", &tok1);
- ip = config->allowIps + ((sizeof(struct in_addr) + 1) * i);
- if (!inet_pton(AF_INET, tok2, ip)) {
- TSDebug(PLUGIN_NAME, "%d) skipping: %s", i + 1, tok1);
- continue;
- }
- if (tok1 != NULL) {
- tok2 = strtok_r(tok1, "/", &tok1);
- }
- if (!tok2) {
- mask = 32;
- } else {
- mask = atoi(tok2);
- }
- ip[4] = mask;
- TSDebug(PLUGIN_NAME, "%d) adding netmask: %s/%d", i + 1,
inet_ntop(AF_INET, ip, ip_port_text_buffer, INET_ADDRSTRLEN), ip[4]);
- i++;
+ if (config->allow_ip_map.contains(addr, nullptr)) {
+ return true;
}
+
+ return false;
}
static void
-parseIps6(config_t *config, char *ipStr)
+parseIpMap(config_t *config, const char *ipStr)
{
- char buffer[STR_BUFFER_SIZE];
- char *p, *tok1, *tok2, *ip;
- int i, mask;
- char ip_port_text_buffer[INET6_ADDRSTRLEN];
-
- if (!ipStr) {
- config->ip6Count = 1;
- ip = config->allowIps6 = TSmalloc(sizeof(struct in6_addr) + 1);
- inet_pton(AF_INET6, DEFAULT_IP6, ip);
- ip[sizeof(struct in6_addr)] = 0;
- return;
+ IpAddr min, max;
+ std::string delimiter = ",";
Review comment:
No, it's allocating that every time!
```
constexpr std::string_view delimiter = ",";
```
##########
File path: plugins/stats_over_http/stats_over_http.cc
##########
@@ -574,22 +574,25 @@ stats_origin(TSCont contp ATS_UNUSED, TSEvent event
ATS_UNUSED, void *edata)
goto cleanup;
}
- int path_len = 0;
- const char *path = TSUrlPathGet(reqp, url_loc, &path_len);
+ path = TSUrlPathGet(reqp, url_loc, &path_len);
TSDebug(PLUGIN_NAME, "Path: %.*s", path_len, path);
- if (!(path_len != 0 && path_len == config->stats_path_len && !memcmp(path,
config->stats_path, config->stats_path_len))) {
+ if (!(path_len != 0 && path_len == int(config->stats_path.length()) &&
+ !memcmp(path, config->stats_path.c_str(),
config->stats_path.length()))) {
+ TSDebug(PLUGIN_NAME, "not this plugins path, saw: %.*s, looking for: %s",
path_len, path, config->stats_path.c_str());
goto notforme;
}
- const struct sockaddr *addr = TSHttpTxnClientAddrGet(txnp);
- if (!is_ip_allowed(config, addr)) {
+ TSSkipRemappingSet(txnp, 1); // not strictly necessary, but speed is
everything these days
+
+ addr = TSHttpTxnClientAddrGet(txnp);
Review comment:
For real C++ style, don't declare this above with an initializer,
declare it here initialized by `TSHttpTxnClientAddrGet`.
```
auto addr = TSHttpTxnClienAddrGet(txnp);
```
Not sure why you changed that from the original. If you wanted to be really
cutting edge, you'd do
```
if ( auto addr = TSHttpTxnClientAddrGet(txnp) ; !is _ip_allowed(config,
addr)) {
// ....
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
