You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "bneradt (via GitHub)" <gi...@apache.org> on 2023/02/01 20:10:52 UTC

[GitHub] [trafficserver] bneradt opened a new pull request, #9358: tunnel_route: Support local inbound and proxy protocol ports

bneradt opened a new pull request, #9358:
URL: https://github.com/apache/trafficserver/pull/9358

   This patch adds the ability to specify that ATS connect to the tunnel_route host using the port that was either:
   
   1. The destination port on the client-side (inbound) connection that instigated the tunnel_route. This is specified as {inbound_local_port} for the tunnel_route host port target.
   2. The destination port specified via the inbound Proxy Protocol string that instigated the tunnel_route. This is specified as {proxy_protocol_port} for the tunnel_route host port target.
   
   
   ---
   
   Marking this as a draft while this is reviewed in the dev mailing list.


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] bneradt commented on a diff in pull request #9358: tunnel_route: Support local inbound and proxy protocol ports

Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #9358:
URL: https://github.com/apache/trafficserver/pull/9358#discussion_r1103410108


##########
iocore/net/P_SNIActionPerformer.h:
##########
@@ -89,12 +89,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;
+    }
+

Review Comment:
   It's initialized to `OpId::DEFAULT` at line 273, which is -1. At line 170 this is checked.



-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] SolidWallOfCode commented on a diff in pull request #9358: tunnel_route: Support local inbound and proxy protocol ports

Posted by "SolidWallOfCode (via GitHub)" <gi...@apache.org>.
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] bneradt merged pull request #9358: tunnel_route: Support local inbound and proxy protocol ports

Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt merged PR #9358:
URL: https://github.com/apache/trafficserver/pull/9358


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] bneradt commented on a diff in pull request #9358: tunnel_route: Support local inbound and proxy protocol ports

Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #9358:
URL: https://github.com/apache/trafficserver/pull/9358#discussion_r1103699003


##########
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:
   That makes sense. Without much tweaking we can make it 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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] SolidWallOfCode commented on a diff in pull request #9358: tunnel_route: Support local inbound and proxy protocol ports

Posted by "SolidWallOfCode (via GitHub)" <gi...@apache.org>.
SolidWallOfCode commented on code in PR #9358:
URL: https://github.com/apache/trafficserver/pull/9358#discussion_r1103297182


##########
iocore/net/P_SNIActionPerformer.h:
##########
@@ -168,13 +225,13 @@ 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 = std::stoi(std::string{number_str});

Review Comment:
   Oh no! Use `ts::svtoi` or `swoc::svtoi`. 



-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] bneradt commented on a diff in pull request #9358: tunnel_route: Support local inbound and proxy protocol ports

Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #9358:
URL: https://github.com/apache/trafficserver/pull/9358#discussion_r1103696108


##########
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:
   No, it is not safe to return the reference. Thanks for catching this.



-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] brbzull0 commented on a diff in pull request #9358: tunnel_route: Support local inbound and proxy protocol ports

Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 commented on code in PR #9358:
URL: https://github.com/apache/trafficserver/pull/9358#discussion_r1103849795


##########
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?
   
   It should be safe, the function returns by value, the caller should get this stored safely.



-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] bneradt commented on a diff in pull request #9358: tunnel_route: Support local inbound and proxy protocol ports

Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #9358:
URL: https://github.com/apache/trafficserver/pull/9358#discussion_r1103695940


##########
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:
   Right, that's nice.



-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] SolidWallOfCode commented on a diff in pull request #9358: tunnel_route: Support local inbound and proxy protocol ports

Posted by "SolidWallOfCode (via GitHub)" <gi...@apache.org>.
SolidWallOfCode commented on code in PR #9358:
URL: https://github.com/apache/trafficserver/pull/9358#discussion_r1103406276


##########
iocore/net/P_SNIActionPerformer.h:
##########
@@ -89,12 +89,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;
+    }
+

Review Comment:
   What is `fnArrIndex` if none of the checks match?



-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] bneradt commented on a diff in pull request #9358: tunnel_route: Support local inbound and proxy protocol ports

Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #9358:
URL: https://github.com/apache/trafficserver/pull/9358#discussion_r1103696487


##########
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:
   We should also check `tunnel_port_is_dynamic` actually.



-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] bneradt commented on pull request #9358: tunnel_route: Support local inbound and proxy protocol ports

Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on PR #9358:
URL: https://github.com/apache/trafficserver/pull/9358#issuecomment-1412746950

   [approve ci autest]


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] bneradt commented on pull request #9358: tunnel_route: Support local inbound and proxy protocol ports

Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on PR #9358:
URL: https://github.com/apache/trafficserver/pull/9358#issuecomment-1412882389

   [approve ci autest]


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org