LinkinStars commented on PR #11172: URL: https://github.com/apache/apisix/pull/11172#issuecomment-2079225639
@shreemaan-abhishek First, we discuss the issue of `env`. After my testing, **both the uppercase and lowercase should be supported** after changes. There are two reasons for this 1. unit test 18 passed https://github.com/shreemaan-abhishek/apisix/blob/07b3cc3f63db8fc4ae3706baa324a76ee3049338/t/router/radixtree-sni2.t#L700-L725 2. user can use it after changing to lowercase https://github.com/apache/apisix/issues/11141#issuecomment-2051028533 After reading the code I found out why. For env, both checking and parsing converted the target to uppercase preferentially. `if string.has_prefix(upper(uri), core.env.PREFIX) then` Because when characters are cut, the character length is used. Both uppercase and lowercase lengths are the same, so there's no problem. `local path = sub(env_uri, #ENV_PREFIX + 1)` ---- Secondly, let's discuss the `secret`. Unfortunately, as you said, using the uppercase 'SECRET' is problematic. **I tried adding unit tests and found that they could not pass.** The reason is quite simple: 'secret' is not converted to uppercase like 'env' before comparison and parsing. Dig more. I find the git history. The 'secret' was previously modified by KMS, and there was uppercase conversion before.  However, `$SECRET` has never been used. It's not even mentioned in the documentation. So, in my opinion, I would **not recommend supporting uppercase SECRET**. I think the author who wrote the code at that time must have also considered that. Of course, these are just my personal thoughts, if there is anything incorrect, please point it out. --- All in all, there are two options. 1. not support `$SECRET` 2. suport `$SECRET`, just like the `ENV`. such as`string.has_prefix(upper(uri), secret.PREFIX)` -- 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]
