Copilot commented on code in PR #12650: URL: https://github.com/apache/trafficserver/pull/12650#discussion_r2507598849
########## tests/gold_tests/pluginTest/lua/verified_addr.lua: ########## @@ -0,0 +1,87 @@ +-- 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. + +function do_remap() + local result = "" + + -- Test 1: Check if verified address is initially nil + local ip1, family1 = ts.client_request.client_addr.get_verified_addr() + if not ip1 then + result = result .. "initial:nil;" + end + + -- Test 2: Set an IPv4 verified address from X-Real-IP header + local real_ip = ts.client_request.header["X-Real-IP"] + if real_ip then + local success, err = pcall(function() + ts.client_request.client_addr.set_verified_addr(real_ip, 2) Review Comment: Use the global constant `TS_LUA_AF_INET` instead of the hardcoded value `2` for better portability and consistency with the rest of the codebase. The constant is already defined and exported to Lua (see doc/admin-guide/plugins/lua.en.rst lines 1923-1924). ########## tests/gold_tests/pluginTest/lua/verified_addr.lua: ########## @@ -0,0 +1,87 @@ +-- 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. + +function do_remap() + local result = "" + + -- Test 1: Check if verified address is initially nil + local ip1, family1 = ts.client_request.client_addr.get_verified_addr() + if not ip1 then + result = result .. "initial:nil;" + end + + -- Test 2: Set an IPv4 verified address from X-Real-IP header + local real_ip = ts.client_request.header["X-Real-IP"] + if real_ip then + local success, err = pcall(function() + ts.client_request.client_addr.set_verified_addr(real_ip, 2) + end) + + if success then + result = result .. "set:success;" + + -- Test 3: Get the verified address we just set + local ip2, family2 = ts.client_request.client_addr.get_verified_addr() + if ip2 then + result = result .. "get:" .. ip2 .. ":" .. tostring(family2) .. ";" + else + result = result .. "get:failed;" + end + else + result = result .. "set:failed;" + end + end + + -- Test 4: Try setting an IPv6 address from X-Real-IP-V6 header + local real_ipv6 = ts.client_request.header["X-Real-IP-V6"] + if real_ipv6 then + local success, err = pcall(function() + ts.client_request.client_addr.set_verified_addr(real_ipv6, 10) + end) + + if success then + result = result .. "setv6:success;" + + -- Get the IPv6 verified address + local ip3, family3 = ts.client_request.client_addr.get_verified_addr() + if ip3 then + result = result .. "getv6:" .. ip3 .. ":" .. tostring(family3) .. ";" + else + result = result .. "getv6:failed;" + end + else + result = result .. "setv6:failed;" + end + end + + -- Test 5: Try setting an invalid address (should fail) + local invalid_ip = ts.client_request.header["X-Invalid-IP"] + if invalid_ip then + local success, err = pcall(function() + ts.client_request.client_addr.set_verified_addr(invalid_ip, 2) Review Comment: Use the global constant `TS_LUA_AF_INET` instead of the hardcoded value `2` for better portability and consistency with the rest of the codebase. The constant is already defined and exported to Lua (see doc/admin-guide/plugins/lua.en.rst lines 1923-1924). ```suggestion ts.client_request.client_addr.set_verified_addr(invalid_ip, TS_LUA_AF_INET) ``` ########## doc/admin-guide/plugins/lua.en.rst: ########## @@ -1053,6 +1053,90 @@ Here is an example: :ref:`TOP <admin-plugins-ts-lua>` +ts.client_request.client_addr.get_verified_addr +----------------------------------------------- +**syntax:** *ip, family = ts.client_request.client_addr.get_verified_addr()* + +**context:** do_remap/do_os_response or do_global_* or later + +**description**: This function can be used to get the verified client IP address for the current transaction. + +The verified address is set by plugins (typically earlier in the transaction) to provide a reliable client IP address. +This is useful when Traffic Server is behind a proxy or load balancer that provides the real client IP through +mechanisms like PROXY protocol, X-Forwarded-For headers, or X-Real-IP headers. + +The ts.client_request.client_addr.get_verified_addr function returns two values: ip is a string and family is a number. +If no verified address has been set, both return values will be nil. + +Here is an example: + +:: + + function do_remap() + ip, family = ts.client_request.client_addr.get_verified_addr() + if ip then + ts.debug(ip) -- 192.168.1.100 + ts.debug(family) -- 2(AF_INET) + else + ts.debug("No verified address set") + end + return 0 + end + +When ``proxy.config.acl.subjects`` is set to ``PLUGIN``, Traffic Server will use the verified address (if set) +for ACL evaluation instead of the actual client connection address. + +:ref:`TOP <admin-plugins-ts-lua>` + +ts.client_request.client_addr.set_verified_addr +----------------------------------------------- +**syntax:** *ts.client_request.client_addr.set_verified_addr(ip, family)* + +**context:** do_remap/do_os_response or do_global_* or later + +**description**: This function can be used to set a verified client IP address for the current transaction. + +This function enables plugins to provide a reliable client IP address for Traffic Server and other plugins. +Plugins that call this function are expected to validate the IP address before setting it. + +**Parameters:** + +* ``ip`` - string: The IP address to set (e.g., "192.168.1.100" or "2001:db8::1") +* ``family`` - number: The address family (2 for AF_INET/IPv4, 10 for AF_INET6/IPv6) Review Comment: Replace hardcoded family values with the defined constants for consistency and better documentation. Change "2 for AF_INET/IPv4" to "TS_LUA_AF_INET for IPv4" and "10 for AF_INET6/IPv6" to "TS_LUA_AF_INET6 for IPv6". These constants are already documented at lines 1923-1924 and used in similar functions like `set_addr` (line 1912). ```suggestion * ``family`` - number: The address family (`TS_LUA_AF_INET` for IPv4, `TS_LUA_AF_INET6` for IPv6) ``` ########## plugins/lua/ts_lua_client_request.cc: ########## @@ -1139,3 +1147,78 @@ ts_lua_client_request_get_ssl_curve(lua_State *L) return 1; } + +static int +ts_lua_client_request_client_addr_get_verified_addr(lua_State *L) +{ + struct sockaddr const *verified_addr; + ts_lua_http_ctx *http_ctx; + int family = AF_UNSPEC; + char vip[128] = ""; + + GET_HTTP_CONTEXT(http_ctx, L); + + if (TSHttpTxnVerifiedAddrGet(http_ctx->txnp, &verified_addr) == TS_SUCCESS) { + if (verified_addr->sa_family == AF_INET) { + inet_ntop(AF_INET, (const void *)&((struct sockaddr_in *)verified_addr)->sin_addr, vip, sizeof(vip)); + family = AF_INET; + } else if (verified_addr->sa_family == AF_INET6) { + inet_ntop(AF_INET6, (const void *)&((struct sockaddr_in6 *)verified_addr)->sin6_addr, vip, sizeof(vip)); + family = AF_INET6; + } + + lua_pushstring(L, vip); + lua_pushnumber(L, family); Review Comment: When `TSHttpTxnVerifiedAddrGet` succeeds but returns an address with an unsupported family (neither AF_INET nor AF_INET6), the function returns an empty string for IP and AF_UNSPEC for family. This is inconsistent with the nil/nil return when the function fails. Consider returning nil/nil in this case as well for consistency, or handling other address families if they're expected. ```suggestion lua_pushstring(L, vip); lua_pushnumber(L, family); } else if (verified_addr->sa_family == AF_INET6) { inet_ntop(AF_INET6, (const void *)&((struct sockaddr_in6 *)verified_addr)->sin6_addr, vip, sizeof(vip)); family = AF_INET6; lua_pushstring(L, vip); lua_pushnumber(L, family); } else { lua_pushnil(L); lua_pushnil(L); } ``` ########## tests/gold_tests/pluginTest/lua/verified_addr.lua: ########## @@ -0,0 +1,87 @@ +-- 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. + +function do_remap() + local result = "" + + -- Test 1: Check if verified address is initially nil + local ip1, family1 = ts.client_request.client_addr.get_verified_addr() + if not ip1 then + result = result .. "initial:nil;" + end + + -- Test 2: Set an IPv4 verified address from X-Real-IP header + local real_ip = ts.client_request.header["X-Real-IP"] + if real_ip then + local success, err = pcall(function() + ts.client_request.client_addr.set_verified_addr(real_ip, 2) + end) + + if success then + result = result .. "set:success;" + + -- Test 3: Get the verified address we just set + local ip2, family2 = ts.client_request.client_addr.get_verified_addr() + if ip2 then + result = result .. "get:" .. ip2 .. ":" .. tostring(family2) .. ";" + else + result = result .. "get:failed;" + end + else + result = result .. "set:failed;" + end + end + + -- Test 4: Try setting an IPv6 address from X-Real-IP-V6 header + local real_ipv6 = ts.client_request.header["X-Real-IP-V6"] + if real_ipv6 then + local success, err = pcall(function() + ts.client_request.client_addr.set_verified_addr(real_ipv6, 10) Review Comment: Use the global constant `TS_LUA_AF_INET6` instead of the hardcoded value `10` for better portability and consistency with the rest of the codebase. The constant is already defined and exported to Lua (see doc/admin-guide/plugins/lua.en.rst lines 1923-1924). ```suggestion ts.client_request.client_addr.set_verified_addr(real_ipv6, TS_LUA_AF_INET6) ``` ########## plugins/lua/ts_lua_client_request.cc: ########## @@ -1139,3 +1147,78 @@ ts_lua_client_request_get_ssl_curve(lua_State *L) return 1; } + +static int +ts_lua_client_request_client_addr_get_verified_addr(lua_State *L) +{ + struct sockaddr const *verified_addr; + ts_lua_http_ctx *http_ctx; + int family = AF_UNSPEC; + char vip[128] = ""; + + GET_HTTP_CONTEXT(http_ctx, L); + + if (TSHttpTxnVerifiedAddrGet(http_ctx->txnp, &verified_addr) == TS_SUCCESS) { + if (verified_addr->sa_family == AF_INET) { + inet_ntop(AF_INET, (const void *)&((struct sockaddr_in *)verified_addr)->sin_addr, vip, sizeof(vip)); + family = AF_INET; + } else if (verified_addr->sa_family == AF_INET6) { + inet_ntop(AF_INET6, (const void *)&((struct sockaddr_in6 *)verified_addr)->sin6_addr, vip, sizeof(vip)); + family = AF_INET6; + } + + lua_pushstring(L, vip); + lua_pushnumber(L, family); + } else { + lua_pushnil(L); + lua_pushnil(L); + } + + return 2; +} + +static int +ts_lua_client_request_client_addr_set_verified_addr(lua_State *L) +{ + union { + struct sockaddr_in sin4; + struct sockaddr_in6 sin6; + struct sockaddr sa; + } addr; Review Comment: The union should be zero-initialized to ensure all fields (like sin6_flowinfo and sin6_scope_id for IPv6) are set to known values. Consider initializing with `= {0}` or using `memset(&addr, 0, sizeof(addr))` before setting the specific fields. This prevents potential issues with uninitialized padding bytes or extra fields. ```suggestion } addr; memset(&addr, 0, sizeof(addr)); ``` ########## doc/admin-guide/plugins/lua.en.rst: ########## @@ -1053,6 +1053,90 @@ Here is an example: :ref:`TOP <admin-plugins-ts-lua>` +ts.client_request.client_addr.get_verified_addr +----------------------------------------------- +**syntax:** *ip, family = ts.client_request.client_addr.get_verified_addr()* + +**context:** do_remap/do_os_response or do_global_* or later + +**description**: This function can be used to get the verified client IP address for the current transaction. + +The verified address is set by plugins (typically earlier in the transaction) to provide a reliable client IP address. +This is useful when Traffic Server is behind a proxy or load balancer that provides the real client IP through +mechanisms like PROXY protocol, X-Forwarded-For headers, or X-Real-IP headers. + +The ts.client_request.client_addr.get_verified_addr function returns two values: ip is a string and family is a number. +If no verified address has been set, both return values will be nil. + +Here is an example: + +:: + + function do_remap() + ip, family = ts.client_request.client_addr.get_verified_addr() + if ip then + ts.debug(ip) -- 192.168.1.100 + ts.debug(family) -- 2(AF_INET) + else + ts.debug("No verified address set") + end + return 0 + end + +When ``proxy.config.acl.subjects`` is set to ``PLUGIN``, Traffic Server will use the verified address (if set) +for ACL evaluation instead of the actual client connection address. + +:ref:`TOP <admin-plugins-ts-lua>` + +ts.client_request.client_addr.set_verified_addr +----------------------------------------------- +**syntax:** *ts.client_request.client_addr.set_verified_addr(ip, family)* + +**context:** do_remap/do_os_response or do_global_* or later + +**description**: This function can be used to set a verified client IP address for the current transaction. + +This function enables plugins to provide a reliable client IP address for Traffic Server and other plugins. +Plugins that call this function are expected to validate the IP address before setting it. + +**Parameters:** + +* ``ip`` - string: The IP address to set (e.g., "192.168.1.100" or "2001:db8::1") +* ``family`` - number: The address family (2 for AF_INET/IPv4, 10 for AF_INET6/IPv6) + +Here is an example: + +:: + + function do_remap() + -- Get real client IP from X-Forwarded-For header + local xff = ts.client_request.header["X-Forwarded-For"] + + if xff then + -- Parse the first IP from X-Forwarded-For + local real_ip = string.match(xff, "([^,]+)") + + if real_ip then + -- Trim whitespace + real_ip = real_ip:match("^%s*(.-)%s*$") + + -- Set as verified address (IPv4 example, family=2) + ts.client_request.client_addr.set_verified_addr(real_ip, 2) Review Comment: Use the global constant `TS_LUA_AF_INET` instead of the hardcoded value `2` for better portability and consistency. Replace `2` with `TS_LUA_AF_INET` in the example. The constant is already documented at line 1923. ```suggestion -- Set as verified address (IPv4 example, family=TS_LUA_AF_INET) ts.client_request.client_addr.set_verified_addr(real_ip, TS_LUA_AF_INET) ``` ########## doc/admin-guide/plugins/lua.en.rst: ########## @@ -1053,6 +1053,90 @@ Here is an example: :ref:`TOP <admin-plugins-ts-lua>` +ts.client_request.client_addr.get_verified_addr +----------------------------------------------- +**syntax:** *ip, family = ts.client_request.client_addr.get_verified_addr()* + +**context:** do_remap/do_os_response or do_global_* or later + +**description**: This function can be used to get the verified client IP address for the current transaction. + +The verified address is set by plugins (typically earlier in the transaction) to provide a reliable client IP address. +This is useful when Traffic Server is behind a proxy or load balancer that provides the real client IP through +mechanisms like PROXY protocol, X-Forwarded-For headers, or X-Real-IP headers. + +The ts.client_request.client_addr.get_verified_addr function returns two values: ip is a string and family is a number. +If no verified address has been set, both return values will be nil. + +Here is an example: + +:: + + function do_remap() + ip, family = ts.client_request.client_addr.get_verified_addr() + if ip then + ts.debug(ip) -- 192.168.1.100 + ts.debug(family) -- 2(AF_INET) + else + ts.debug("No verified address set") + end + return 0 + end + +When ``proxy.config.acl.subjects`` is set to ``PLUGIN``, Traffic Server will use the verified address (if set) +for ACL evaluation instead of the actual client connection address. + +:ref:`TOP <admin-plugins-ts-lua>` + +ts.client_request.client_addr.set_verified_addr +----------------------------------------------- +**syntax:** *ts.client_request.client_addr.set_verified_addr(ip, family)* + +**context:** do_remap/do_os_response or do_global_* or later + +**description**: This function can be used to set a verified client IP address for the current transaction. + +This function enables plugins to provide a reliable client IP address for Traffic Server and other plugins. +Plugins that call this function are expected to validate the IP address before setting it. + +**Parameters:** + +* ``ip`` - string: The IP address to set (e.g., "192.168.1.100" or "2001:db8::1") +* ``family`` - number: The address family (2 for AF_INET/IPv4, 10 for AF_INET6/IPv6) + +Here is an example: + +:: + + function do_remap() + -- Get real client IP from X-Forwarded-For header + local xff = ts.client_request.header["X-Forwarded-For"] + + if xff then + -- Parse the first IP from X-Forwarded-For + local real_ip = string.match(xff, "([^,]+)") + + if real_ip then + -- Trim whitespace + real_ip = real_ip:match("^%s*(.-)%s*$") + + -- Set as verified address (IPv4 example, family=2) + ts.client_request.client_addr.set_verified_addr(real_ip, 2) + ts.debug("Set verified address to: " .. real_ip) + end + end + + return 0 + end + +**Important Notes:** + +* For IPv6 addresses, use family value 10 (AF_INET6). Review Comment: Replace the hardcoded value with the constant name for consistency. Change "For IPv6 addresses, use family value 10 (AF_INET6)" to "For IPv6 addresses, use TS_LUA_AF_INET6" to match the documentation style used for other similar functions. ```suggestion * For IPv6 addresses, use TS_LUA_AF_INET6. ``` -- 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]
