SolidWallOfCode commented on code in PR #9358:
URL: https://github.com/apache/trafficserver/pull/9358#discussion_r1103408480
##########
iocore/net/P_SNIActionPerformer.h:
##########
@@ -89,12 +90,73 @@ class HTTP2BufferWaterMark : public ActionItem
class TunnelDestination : public ActionItem
{
+ // ID of the configured variable. This will be used to know which function
+ // should be called when processing the tunnel destination.
+ enum OpId : int32_t {
+ DEFAULT = -1, // No specific variable set.
+ MATCH_GROUPS, // Deal with configured groups.
+ MAP_WITH_RECV_PORT, // Use port from inbound local
+ MAP_WITH_PROXY_PROTOCOL_PORT, // Use port from the proxy protocol
+ MAX // Always at the end and do not change the
value of the above items.
+ };
+ const std::string MAP_WITH_RECV_PORT_STR = "{inbound_local_port}";
+ const std::string MAP_WITH_PROXY_PROTOCOL_PORT_STR = "{proxy_protocol_port}";
+
public:
TunnelDestination(const std::string_view &dest, SNIRoutingType type,
YamlSNIConfig::TunnelPreWarm prewarm,
const std::vector<int> &alpn)
: destination(dest), type(type), tunnel_prewarm(prewarm), alpn_ids(alpn)
{
- need_fix = (destination.find_first_of('$') != std::string::npos);
+ std::size_t var_start_pos{0}; // keep the starting position of the
variable.
+ if (destination.find_first_of('$') != std::string::npos) {
+ fnArrIndex = OpId::MATCH_GROUPS;
+ } else if (var_start_pos = destination.find(MAP_WITH_RECV_PORT_STR);
var_start_pos != std::string::npos) {
+ fnArrIndex = OpId::MAP_WITH_RECV_PORT;
+ } else if (var_start_pos =
destination.find(MAP_WITH_PROXY_PROTOCOL_PORT_STR); var_start_pos !=
std::string::npos) {
+ fnArrIndex = OpId::MAP_WITH_PROXY_PROTOCOL_PORT;
+ }
+
+ // Replace wildcards with matched groups.
+ fix_destination[OpId::MATCH_GROUPS] = [&](std::string_view destination,
const Context &ctx, SSLNetVConnection *,
+ bool &port_is_dynamic) ->
std::string {
+ port_is_dynamic = false;
+ if ((destination.find_first_of('$') != std::string::npos) &&
ctx._fqdn_wildcard_captured_groups) {
+ const auto &fixed_dst = replace_match_groups(destination,
*ctx._fqdn_wildcard_captured_groups, port_is_dynamic);
+ return fixed_dst;
Review Comment:
Isn't `fixed_dst` a reference? Is it safe to return?
##########
proxy/http/HttpTransact.cc:
##########
@@ -6366,8 +6367,16 @@ HttpTransact::is_request_valid(State *s, HTTPHdr
*incoming_request)
RequestError_t incoming_error;
URL *url = nullptr;
- // If we are blind tunneling the header is just a synthesized placeholder
anyway
+ // If we are blind tunneling the header is just a synthesized placeholder
anyway.
+ // But we do have to check that we are not tunneling to a dynamic port that
is
+ // not in the connect_ports list.
if (s->client_info.port_attribute == HttpProxyPort::TRANSPORT_BLIND_TUNNEL) {
+ if (!is_port_in_range(incoming_request->url_get()->port_get(),
s->http_config_param->connect_ports)) {
Review Comment:
Does this break anything else? In what circumstances would this code have
executed?
##########
iocore/net/P_SNIActionPerformer.h:
##########
@@ -168,16 +236,19 @@ class TunnelDestination : public ActionItem
to = (port - pos) - 1;
}
}
- const auto &number_str = dst.substr(pos + 1, to);
+ std::string_view number_str{dst.substr(pos + 1, to)};
if (!is_number(number_str)) {
// it may be some issue on the configured string, place the char and
keep going.
real_dst += *c;
continue;
}
- const std::size_t group_index = std::stoi(number_str);
+ const std::size_t group_index = swoc::svtoi(std::string{number_str});
Review Comment:
The point of using `svtoi` is to avoid converting to `std::string`. E.g.
`swoc::svtoi(number_str)`
##########
iocore/net/P_SNIActionPerformer.h:
##########
@@ -89,12 +90,73 @@ class HTTP2BufferWaterMark : public ActionItem
class TunnelDestination : public ActionItem
{
+ // ID of the configured variable. This will be used to know which function
+ // should be called when processing the tunnel destination.
+ enum OpId : int32_t {
+ DEFAULT = -1, // No specific variable set.
+ MATCH_GROUPS, // Deal with configured groups.
+ MAP_WITH_RECV_PORT, // Use port from inbound local
+ MAP_WITH_PROXY_PROTOCOL_PORT, // Use port from the proxy protocol
+ MAX // Always at the end and do not change the
value of the above items.
+ };
+ const std::string MAP_WITH_RECV_PORT_STR = "{inbound_local_port}";
+ const std::string MAP_WITH_PROXY_PROTOCOL_PORT_STR = "{proxy_protocol_port}";
+
public:
TunnelDestination(const std::string_view &dest, SNIRoutingType type,
YamlSNIConfig::TunnelPreWarm prewarm,
const std::vector<int> &alpn)
: destination(dest), type(type), tunnel_prewarm(prewarm), alpn_ids(alpn)
{
- need_fix = (destination.find_first_of('$') != std::string::npos);
+ std::size_t var_start_pos{0}; // keep the starting position of the
variable.
+ if (destination.find_first_of('$') != std::string::npos) {
+ fnArrIndex = OpId::MATCH_GROUPS;
+ } else if (var_start_pos = destination.find(MAP_WITH_RECV_PORT_STR);
var_start_pos != std::string::npos) {
+ fnArrIndex = OpId::MAP_WITH_RECV_PORT;
+ } else if (var_start_pos =
destination.find(MAP_WITH_PROXY_PROTOCOL_PORT_STR); var_start_pos !=
std::string::npos) {
+ fnArrIndex = OpId::MAP_WITH_PROXY_PROTOCOL_PORT;
+ }
+
+ // Replace wildcards with matched groups.
+ fix_destination[OpId::MATCH_GROUPS] = [&](std::string_view destination,
const Context &ctx, SSLNetVConnection *,
+ bool &port_is_dynamic) ->
std::string {
+ port_is_dynamic = false;
+ if ((destination.find_first_of('$') != std::string::npos) &&
ctx._fqdn_wildcard_captured_groups) {
+ const auto &fixed_dst = replace_match_groups(destination,
*ctx._fqdn_wildcard_captured_groups, port_is_dynamic);
+ return fixed_dst;
+ }
+ return {};
+ };
+
+ // Use local port for the tunnel.
+ fix_destination[OpId::MAP_WITH_RECV_PORT] = [&,
var_start_pos](std::string_view destination, const Context &ctx,
Review Comment:
Does this need to be done every constructor? Isn't this all process static?
--
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]
