ChristopherSchultz commented on pull request #428: URL: https://github.com/apache/tomcat/pull/428#issuecomment-876511953
When considering defensive copying, one must remember that no other code can be trusted, but our own code _can_ be trusted. So structures passed-into the realm must be copied to protect our code from a malicious (or incompetent) client breaking things. When returning structures, there is more flexibility depending upon how much you trust the JVM and whether or not you want to protect against truly malicious code. For example, for most cases, it's reasonable to return Collections.unmodifiable[Collection](foo) to ensure that foreign code doesn't mess with your structure. But, reflection is a possibility and therefore it's not _entirely_ safe. If it's reasonable to return `Collections.unmodifiable[Collection](foo)` for a certain `getFoo()` operation, and the strcuture needs to be unmodifiable locally, anyway, then making a defensive *and* unmodifiable copy during `setFoo` (or similar) is reasonable. Then we can avoid additional allocations on every `getFoo()` call. But again, it depends upon hos security-sensitive these items are. Unfortunately, it's impossible to know how security-sensitive these things can be, because they are application-defined. Having a user-attribute "locale" is unlikely to be highly security-sensitive. But maybe "totp-seed" may be. Or "user-is-unrestricted-administrator" or something similar. Security features are always a trade-off, and most applications don't need this kind of uber-paranoid data-handling. But some environments may require it. Since nobody has been beating-down the door asking for user-attributes on realms in the first place, perhaps engineering a solution to include uber-paranoia is over-engineering. Anything that can be written can be re-written, improved, etc. I suggest we do the minimum possible to build the useful feature and ship it. If "casual" damage to structures would be Bad, let's prevent that from happening. But let's not worry too much about an application using native code to modify the internal char array of a `java.lang.String` value at this point. -- 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: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
