glassfish icon indicating copy to clipboard operation
glassfish copied to clipboard

Fix GF Embedded runtime detection and unify the working approaches to detect runtime

Open OndroMih opened this issue 10 months ago • 1 comments

What doesn't work:

In SecurityLifecycle class, Util.isEmbeddedServer() method is used to detect whether the runtime is GF Embedded. But the method always returns false, hence system properties for login.conf and server.policy are not set.

The Util.isEmbeddedServer() method uses the Server class and weird logic that mode is embedded if there are some Server instances. It seems to me that the Server class and related classes that implement EmbeddedContainer are dead code. The only way to create Server instances is using the Builder class, which is used only in tests, which probably test this unused container functionality: https://github.com/search?q=repo%3Aeclipse-ee4j%2Fglassfish++Server.Builder&type=code

Fix

Change the Util.isEmbeddedServer() method to rely on one of the two ways that work:

  • EmbeddedSecurityUtil.isEmbedded(), as in https://github.com/eclipse-ee4j/glassfish/blob/9d16cbab386829705f938f2037594ae6a171113d/nucleus/security/core/src/main/java/com/sun/enterprise/security/embedded/EmbeddedSecurityUtil.java#L65
  • EjbContainerUtilImpl.isEmbeddedServer(), which seems to use a similar code, but completely distinct and duplicated

Additional refactor

Check whether it makes sense to

  • Unify the above 2 working approaches to detecting the runtime and replace them with a single approach
  • Remove the dead code of Server and related classes

OndroMih avatar Feb 28 '25 13:02 OndroMih

I am not sure, but the term "embedded" can mean 2-3 different things in GlassFish terminology, as it evolved over years. In our usual usage the ServerEnvironment.isEmbedded is the right one; for 7.1.0 I already created commit which pushed this method to the interface; maybe I should split it from my PR #25517

dmatej avatar May 24 '25 23:05 dmatej