elsloo commented on a change in pull request #8088:
URL: https://github.com/apache/trafficserver/pull/8088#discussion_r675763105
##########
File path: src/traffic_server/InkAPI.cc
##########
@@ -6880,14 +6880,28 @@ TSHttpTxnPluginTagGet(TSHttpTxn txnp)
TSVConn
TSHttpConnectWithPluginId(sockaddr const *addr, const char *tag, int64_t id)
+{
+ return TSHttpConnectPlugin(addr, tag, id, BUFFER_SIZE_INDEX_32K,
DEFAULT_PLUGIN_VC_BUFFER_WATER_MARK);
+}
+
+TSVConn
+TSHttpConnectPlugin(sockaddr const *addr, const char *tag, int64_t id, int64_t
buffer_index, int64_t buffer_water_mark)
{
sdk_assert(addr);
sdk_assert(ats_is_ip(addr));
sdk_assert(ats_ip_port_cast(addr));
+ if (buffer_index < BUFFER_SIZE_INDEX_128 || buffer_index >
MAX_BUFFER_SIZE_INDEX) {
+ buffer_index = BUFFER_SIZE_INDEX_32K; // out of range, set to the default
for safety
+ }
+
+ if (buffer_water_mark < 0) {
+ buffer_water_mark = DEFAULT_PLUGIN_VC_BUFFER_WATER_MARK;
+ }
Review comment:
I believe this is the correct behavior because there is no assert on
these inputs. I originally had asserts in place of the if statements with
similar logic but removed them due to the risk of crashing ATS if/when a new
overridden parameter comes in. Ignoring invalid parameters and avoiding a crash
seemed like the lesser of two evils, but I'm open to alternate suggestions.
This is not the only place where I do this sort of check, and I would prefer to
keep the behavior consistent, so if we change it here, I'll want to change it
elsewhere in this PR.
--
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]
