bzp2010 commented on code in PR #12187:
URL: https://github.com/apache/apisix/pull/12187#discussion_r2118840015
##########
apisix/core/utils.lua:
##########
@@ -461,5 +461,35 @@ function _M.check_tls_bool(fields, conf, plugin_name)
end
end
+---
+-- Checks if a string is an nginx variable.
+-- An nginx variable starts with the '$' character.
+--
+-- @function core.utils.is_nginx_variable
+-- @tparam string str The string to check
+-- @treturn boolean true if the string starts with '$', false otherwise
+-- @usage
+-- local utils = require("apisix.core.utils")
+--
+-- -- Usage examples:
+-- local is_var = utils.is_nginx_variable("$host") -- true
+-- local is_var = utils.is_nginx_variable("host") -- false
+-- local is_var = utils.is_nginx_variable("${host}") -- true
+-- local is_var = utils.is_nginx_variable("\\$host") -- false
+--
+-- -- Usage in APISIX context:
+-- if utils.is_nginx_variable(up_conf.service_name) then
+-- -- Handle as nginx variable
+-- else
+-- -- Handle as regular service name
+-- end
+function _M.is_nginx_variable(str)
+ if not str or type(str) ~= "string" then
+ return false
+ end
+
+ -- Check if the string starts with '$' and it's not escaped
+ return str:sub(1, 1) == "$" and (str:sub(1, 2) ~= "\\$")
+end
Review Comment:
Does it really work? That's part of what the `resolve_var` function does.
You can just use `resolve_var` directly, and it resolves the variable as
best it can. When any variable is not found or does not exist, the variable is
replaced with an empty string.
Also, `resolve_var` returns the number of variables it replaces; if it
returns 0, you can choose to leave the template string as it is (i.e., no
replacement).
Using a separate function to determine if it is a variable is neither useful
nor necessary. Your implementation assumes that variable strings must begin
with `$`, but `resolve_var` can handle more clearances than you assume, such as
`TMP_${VAR1}_${VAR2}`.
Other than that, your function just checks if the function is a string
starting with `$` (by the way `core.string` provides `has_prefix` and
`has_suffix` functions directly), it doesn't check if the variable key is a
real nginx variable at all, and in fact it can't be checked, a variable that
doesn't exist is indistinguishable from a variable with a value of nil.
Therefore the naming of `is_nginx_variable` is inappropriate.
--
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: notifications-unsubscr...@apisix.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
