From d9b620dda5fd5c251cdcf366920273d3827883c6 Mon Sep 17 00:00:00 2001 From: Markus Jung Date: Thu, 23 Jul 2026 20:44:30 +0200 Subject: [PATCH 1/3] TOMEE-4654 - Make the component naming context read-only by default The Enterprise Beans spec (10.4.4) and EE.5.3.4 require java:comp and its subcontexts to be read-only inside a deployed application: write attempts must not take effect. The IvmContext read-only machinery already existed but was gated behind openejb.forceReadOnlyAppNamingContext, which defaulted to false, so every deployed application received a fully writable ENC. Flip that default to true (retained as an explicit opt-out for backward compatibility) and extend the enforcement to the web and app contexts. Servlets and JSF managed beans resolve java:comp/java:module/java:app through the WebContext and AppContext rather than a BeanContext, so marking only the BeanContexts read-only left the web tier writable. Adds JavaCompReadOnlyTest, which deploys a real application and asserts that bind/rebind/rename/unbind/createSubcontext/destroySubcontext are all refused on java:comp and java:app, and inverts AppNamingReadOnlyTest, whose former testAppNamingContextWritableByDefault asserted the exact behaviour being fixed. --- .../openejb/assembler/classic/Assembler.java | 29 +++-- .../classic/AppNamingReadOnlyTest.java | 55 ++++++-- .../core/ivm/naming/JavaCompReadOnlyTest.java | 123 ++++++++++++++++++ 3 files changed, 184 insertions(+), 23 deletions(-) create mode 100644 container/openejb-core/src/test/java/org/apache/openejb/core/ivm/naming/JavaCompReadOnlyTest.java diff --git a/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java b/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java index 50be16d782f..0a8dcd0a74c 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java @@ -1094,7 +1094,7 @@ private AppContext createApplication(final AppInfo appInfo, ClassLoader classLoa logger.info("createApplication.success", appInfo.path); //required by spec EE.5.3.4 - if(setAppNamingContextReadOnly(allDeployments)) { + if(setAppNamingContextReadOnly(appContext, allDeployments)) { logger.info("createApplication.naming", appInfo.path); } @@ -1117,22 +1117,33 @@ private AppContext createApplication(final AppInfo appInfo, ClassLoader classLoa } } - boolean setAppNamingContextReadOnly(final List allDeployments) { - if("true".equals(SystemInstance.get().getProperty(FORCE_READ_ONLY_APP_NAMING, "false"))) { + boolean setAppNamingContextReadOnly(final AppContext appContext, final List allDeployments) { + if("true".equals(SystemInstance.get().getProperty(FORCE_READ_ONLY_APP_NAMING, "true"))) { for(BeanContext beanContext : allDeployments) { - Context ctx = beanContext.getJndiContext(); - - if(IvmContext.class.isInstance(ctx)) { - IvmContext.class.cast(ctx).setReadOnly(true); - } else if(ContextHandler.class.isInstance(ctx)) { - ContextHandler.class.cast(ctx).setReadOnly(); + markReadOnly(beanContext.getJndiContext()); + } + + // servlets, JSF beans and other web components resolve java:comp/java:module/java:app through the + // web and app contexts rather than through a BeanContext, so they need the same treatment + if(appContext != null) { + for(final WebContext webContext : appContext.getWebContexts()) { + markReadOnly(webContext.getJndiEnc()); } + markReadOnly(appContext.getAppJndiContext()); } return true; } return false; } + private static void markReadOnly(final Context ctx) { + if(IvmContext.class.isInstance(ctx)) { + IvmContext.class.cast(ctx).setReadOnly(true); + } else if(ContextHandler.class.isInstance(ctx)) { + ContextHandler.class.cast(ctx).setReadOnly(); + } + } + private List getDuplicates(final AppInfo appInfo) { final List used = new ArrayList<>(); for (final EjbJarInfo ejbJarInfo : appInfo.ejbJars) { diff --git a/container/openejb-core/src/test/java/org/apache/openejb/assembler/classic/AppNamingReadOnlyTest.java b/container/openejb-core/src/test/java/org/apache/openejb/assembler/classic/AppNamingReadOnlyTest.java index 8262ddd0da4..6b6cf06a982 100644 --- a/container/openejb-core/src/test/java/org/apache/openejb/assembler/classic/AppNamingReadOnlyTest.java +++ b/container/openejb-core/src/test/java/org/apache/openejb/assembler/classic/AppNamingReadOnlyTest.java @@ -43,10 +43,10 @@ public void testReadOnlyAppNamingContext() throws SystemException, URISyntaxExce System.setProperty(Assembler.FORCE_READ_ONLY_APP_NAMING, Boolean.TRUE.toString()); try { List mockBeanContextsList = getMockBeanContextsList(); - + Assembler assembler = new Assembler(); - assembler.setAppNamingContextReadOnly(mockBeanContextsList); - + assembler.setAppNamingContextReadOnly(null, mockBeanContextsList); + Context beanNamingContext = mockBeanContextsList.get(0).getJndiContext(); //may return null or throw exception depending on openejb.jndiExceptionOnFailedWrite value; //this test is not intended to test read-only behavior (null/exception); it should check whether naming context is marked as read only @@ -68,18 +68,45 @@ public void testReadOnlyAppNamingContext() throws SystemException, URISyntaxExce } } - //check TOMEE behavior is backward compatible - public void testAppNamingContextWritableByDefault() throws SystemException, URISyntaxException, NamingException { + //read-only is the spec-mandated default (EE.5.3.4, Enterprise Beans 10.4.4) + public void testAppNamingContextReadOnlyByDefault() throws SystemException, URISyntaxException { - List mockBeanContextsList = getMockBeanContextsList(); - - Assembler assembler = new Assembler(); - assembler.setAppNamingContextReadOnly(mockBeanContextsList); - - Context beanNamingContext = mockBeanContextsList.get(0).getJndiContext(); - Context subContext = beanNamingContext.createSubcontext("sub"); - - assertNotNull(subContext); + List mockBeanContextsList = getMockBeanContextsList(); + + Assembler assembler = new Assembler(); + assertTrue(assembler.setAppNamingContextReadOnly(null, mockBeanContextsList)); + + Context beanNamingContext = mockBeanContextsList.get(0).getJndiContext(); + try { + assertNull(beanNamingContext.createSubcontext("sub")); + } catch (OperationNotSupportedException e) { + //ok + } catch (NamingException e) { + throw new AssertionError(e); + } + } + + //the legacy writable behavior is still available as an explicit opt-out + public void testAppNamingContextWritableWhenDisabled() throws SystemException, URISyntaxException, NamingException { + + String originalValue = System.getProperty(Assembler.FORCE_READ_ONLY_APP_NAMING); + System.setProperty(Assembler.FORCE_READ_ONLY_APP_NAMING, Boolean.FALSE.toString()); + try { + List mockBeanContextsList = getMockBeanContextsList(); + + Assembler assembler = new Assembler(); + assertFalse(assembler.setAppNamingContextReadOnly(null, mockBeanContextsList)); + + Context beanNamingContext = mockBeanContextsList.get(0).getJndiContext(); + assertNotNull(beanNamingContext.createSubcontext("sub")); + } finally { + if(originalValue == null) { + System.clearProperty(Assembler.FORCE_READ_ONLY_APP_NAMING); + } else { + System.setProperty(Assembler.FORCE_READ_ONLY_APP_NAMING, originalValue); + } + SystemInstance.reset(); + } } private List getMockBeanContextsList() throws SystemException, URISyntaxException { diff --git a/container/openejb-core/src/test/java/org/apache/openejb/core/ivm/naming/JavaCompReadOnlyTest.java b/container/openejb-core/src/test/java/org/apache/openejb/core/ivm/naming/JavaCompReadOnlyTest.java new file mode 100644 index 00000000000..4e3b5807bff --- /dev/null +++ b/container/openejb-core/src/test/java/org/apache/openejb/core/ivm/naming/JavaCompReadOnlyTest.java @@ -0,0 +1,123 @@ +/** + * 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. + */ +package org.apache.openejb.core.ivm.naming; + +import junit.framework.TestCase; +import org.apache.openejb.AppContext; +import org.apache.openejb.BeanContext; +import org.apache.openejb.assembler.classic.Assembler; +import org.apache.openejb.assembler.classic.SecurityServiceInfo; +import org.apache.openejb.assembler.classic.TransactionServiceInfo; +import org.apache.openejb.config.AppModule; +import org.apache.openejb.config.ConfigurationFactory; +import org.apache.openejb.config.EjbModule; +import org.apache.openejb.jee.EjbJar; +import org.apache.openejb.jee.SingletonBean; +import org.apache.openejb.loader.SystemInstance; + +import javax.naming.Context; +import javax.naming.OperationNotSupportedException; + +/** + * The Enterprise Beans spec (10.4.4) and EE.5.3.4 require the component naming context to be read-only: + * writes against java:comp and friends must not take effect. + */ +public class JavaCompReadOnlyTest extends TestCase { + + private AppContext deploy() throws Exception { + final ConfigurationFactory config = new ConfigurationFactory(); + final Assembler assembler = new Assembler(); + + assembler.createTransactionManager(config.configureService(TransactionServiceInfo.class)); + assembler.createSecurityService(config.configureService(SecurityServiceInfo.class)); + + final EjbJar ejbJar = new EjbJar("testmodule"); + ejbJar.addEnterpriseBean(new SingletonBean(Bean.class)); + + final AppModule module = new AppModule(new EjbModule(ejbJar)); + return assembler.createApplication(config.configureApplication(module)); + } + + public void testCompContextRefusesWrites() throws Exception { + final AppContext app = deploy(); + try { + final BeanContext bean = app.getBeanContexts().get(0); + final Context comp = bean.getJndiContext(); + + assertWriteRefused(comp, "bind", () -> comp.bind("newName", "newValue")); + assertWriteRefused(comp, "rebind", () -> comp.rebind("newName", "newValue")); + assertWriteRefused(comp, "rename", () -> comp.rename("comp", "renameTo")); + assertWriteRefused(comp, "unbind", () -> comp.unbind("comp")); + assertWriteRefused(comp, "destroySubcontext", () -> comp.destroySubcontext("comp")); + + // createSubcontext either throws or returns null, depending on jndiExceptionOnFailedWrite + try { + assertNull(comp.createSubcontext("newName")); + } catch (final OperationNotSupportedException expected) { + // ok + } + + // nothing the writes attempted may be observable afterwards + assertNotBound(comp, "newName"); + assertNotBound(comp, "renameTo"); + + // and the pre-existing binding must have survived unbind/rename/destroySubcontext + assertTrue(comp.lookup("comp") instanceof Context); + } finally { + SystemInstance.reset(); + } + } + + public void testAppContextRefusesWrites() throws Exception { + final AppContext app = deploy(); + try { + final Context appCtx = app.getAppJndiContext(); + + assertWriteRefused(appCtx, "bind", () -> appCtx.bind("newName", "newValue")); + assertNotBound(appCtx, "newName"); + assertTrue(appCtx.lookup("app") instanceof Context); + } finally { + SystemInstance.reset(); + } + } + + private interface Write { + void run() throws Exception; + } + + private void assertWriteRefused(final Context ctx, final String operation, final Write write) { + try { + write.run(); + fail(operation + " should have been refused on a read-only naming context"); + } catch (final OperationNotSupportedException expected) { + // ok + } catch (final Exception e) { + throw new AssertionError("unexpected exception from " + operation, e); + } + } + + private void assertNotBound(final Context ctx, final String name) throws Exception { + try { + assertNull(name + " must not be bound", ctx.lookup(name)); + } catch (final javax.naming.NameNotFoundException expected) { + // ok + } + } + + public static class Bean { + } +} From 5cf5d33d71ce0d6c1d644b1c290cbd7c13013160 Mon Sep 17 00:00:00 2001 From: Markus Jung Date: Tue, 28 Jul 2026 21:47:05 +0200 Subject: [PATCH 2/3] TOMEE-4654 - Mark the naming contexts read only after the deployments are started Review feedback on the first commit: marking the contexts at the end of createApplication is the wrong point in the lifecycle, and it cuts both ways. isSkip defers the ejb modules of an ear's web modules to the web app builders, which call initEjbs and startEjbs after createApplication has returned. Closing the application context in createApplication therefore made JndiBuilder's app// bindings fail with OperationNotSupportedException on that later pass, while the java:comp of exactly those modules was never closed at all - the spec violation survived for the beans living in an ear's web modules. Moving the marking to the end of startEjbs fixes both: the containers bind comp/EJBContext, comp/WebServiceContext and comp/TimerService into each bean enc while deploying, which happens there and not in initEjbs. The intent is recorded on the AppContext when the application is configured and applied once the deployments exist, so late modules are covered too. The shared application context is only closed once no further module can bind into it, tracked by a count of the late modules still to come. Also from the review: - Drop the WebContext loop. Neither TomcatWebAppBuilder nor LightweightWebAppBuilder stores an IvmContext or ContextHandler in WebContext.jndiEnc, so it never matched and was dead code. - Read the opt-out from appInfo.properties before the system property, as the other Assembler switches do, so one legacy application cannot force the whole container off the behaviour the specification requires. - Parse the opt-out with Boolean.parseBoolean so -D...=FALSE is honoured. - Invert EmbeddedTomEEContainerTest.testEjbCanCreateSubContextByDefault, which still asserted the pre-fix semantic, into a test that the write is refused. - Accept both refusal modes in the new tests instead of requiring OperationNotSupportedException, run deploy() inside the try/finally so a failure there cannot skip SystemInstance.reset(), and replace the tautological rename/destroySubcontext assertions. --- .../embedded/EmbeddedTomEEContainerTest.java | 14 +- .../java/org/apache/openejb/AppContext.java | 38 ++++ .../openejb/assembler/classic/Assembler.java | 91 ++++++--- .../classic/AppNamingReadOnlyTest.java | 174 ++++++++++-------- .../core/ivm/naming/JavaCompReadOnlyTest.java | 34 ++-- 5 files changed, 238 insertions(+), 113 deletions(-) diff --git a/arquillian/arquillian-tomee-embedded/src/test/java/org/apache/openejb/arquillian/embedded/EmbeddedTomEEContainerTest.java b/arquillian/arquillian-tomee-embedded/src/test/java/org/apache/openejb/arquillian/embedded/EmbeddedTomEEContainerTest.java index ed652dc0254..25e0787a204 100644 --- a/arquillian/arquillian-tomee-embedded/src/test/java/org/apache/openejb/arquillian/embedded/EmbeddedTomEEContainerTest.java +++ b/arquillian/arquillian-tomee-embedded/src/test/java/org/apache/openejb/arquillian/embedded/EmbeddedTomEEContainerTest.java @@ -35,6 +35,7 @@ import java.net.URL; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; @RunWith(Arquillian.class) @@ -71,19 +72,26 @@ public void restServiceIsDeployed() throws Exception { assertEquals("foo", read); } + /** + * EE.5.3.4 and Enterprise Beans 10.4.4 require the component naming context to be read only, so + * createSubcontext must not succeed from within a bean. Depending on + * openejb.jndiExceptionOnFailedWrite the container either throws + * OperationNotSupportedException or ignores the write and returns null; both are refusals, what + * must not happen is the subcontext being created. + */ @Test - public void testEjbCanCreateSubContextByDefault() throws Exception { + public void testEjbCannotCreateSubContextByDefault() throws Exception { String originalValue = System.getProperty(javax.naming.Context.INITIAL_CONTEXT_FACTORY); if(originalValue == null) { System.setProperty(javax.naming.Context.INITIAL_CONTEXT_FACTORY, InitContextFactory.class.getName()); } try { String result = ejb.createSubContext(); - assertEquals("Cannot create sub context", "created", result); + assertNotEquals("Sub context creation must be refused on a read-only naming context", "created", result); } finally { if(originalValue == null) { System.clearProperty(javax.naming.Context.INITIAL_CONTEXT_FACTORY); - } + } } } } diff --git a/container/openejb-core/src/main/java/org/apache/openejb/AppContext.java b/container/openejb-core/src/main/java/org/apache/openejb/AppContext.java index 4d8bb7001cb..bc459e3241a 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/AppContext.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/AppContext.java @@ -30,6 +30,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; /** * @version $Rev$ $Date$ @@ -42,6 +43,8 @@ public class AppContext extends DeploymentContext { private final Context appJndiContext; private final boolean standaloneModule; private boolean cdiEnabled; + private boolean readOnlyAppNamingContext; + private final AtomicInteger pendingLateModules = new AtomicInteger(); private WebBeansContext webBeansContext; private final Collection injections = new HashSet<>(); private final Map bindings = new HashMap<>(); @@ -59,6 +62,41 @@ public AppContext(final String id, final SystemInstance systemInstance, final Cl this.standaloneModule = standaloneModule; } + /** + * Whether the component naming contexts of this application must be marked read only once its + * deployments have been created, as required by EE.5.3.4 and Enterprise Beans 10.4.4. The flag is + * recorded when the application is configured but only applied after the container's own bindings + * are done, so that modules deployed later - the web modules of an ear, for instance - are covered + * too. + */ + public boolean isReadOnlyAppNamingContext() { + return readOnlyAppNamingContext; + } + + public void setReadOnlyAppNamingContext(final boolean readOnlyAppNamingContext) { + this.readOnlyAppNamingContext = readOnlyAppNamingContext; + } + + /** + * How many modules are still to be deployed after createApplication returns - the web modules of + * an ear, which TomcatWebAppBuilder starts one by one. The application scoped naming context stays + * writable until the last of them is in, so the container can keep binding into it. + */ + public void setPendingLateModules(final int count) { + pendingLateModules.set(count); + } + + /** + * Consumes one deployment pass. + * + * @return true when no further module can bind into the application naming context + */ + public boolean lastModuleDeployed() { + // the count is how many passes are still to come after this one, so a pass only closes the + // application context when there is nothing left to consume + return pendingLateModules.get() <= 0 || pendingLateModules.getAndDecrement() <= 0; + } + public Collection getInjections() { return injections; } diff --git a/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java b/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java index 0a8dcd0a74c..0ab66f9f34e 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java @@ -35,6 +35,7 @@ import org.apache.openejb.Injection; import org.apache.openejb.JndiConstants; import org.apache.openejb.MethodContext; +import org.apache.openejb.ModuleContext; import org.apache.openejb.NoSuchApplicationException; import org.apache.openejb.OpenEJBException; import org.apache.openejb.OpenEJBRuntimeException; @@ -835,6 +836,11 @@ private AppContext createApplication(final AppInfo appInfo, ClassLoader classLoa } final AppContext appContext = new AppContext(appInfo.appId, SystemInstance.get(), classLoader, globalJndiContext, appJndiContext, appInfo.standaloneModule); + //required by spec EE.5.3.4, applied once the deployments exist - see setAppNamingContextReadOnly + appContext.setReadOnlyAppNamingContext(isReadOnlyAppNaming(appInfo)); + // isSkip defers the web modules of an ear to the web app builders, so the application + // naming context has to stay writable until the last of them has been started + appContext.setPendingLateModules(appInfo.webAppAlone ? 0 : appInfo.webApps.size()); for (final Entry entry : appInfo.properties.entrySet()) { if (! Module.class.isInstance(entry.getValue())) { appContext.getProperties().put(entry.getKey(), entry.getValue()); @@ -1093,11 +1099,6 @@ private AppContext createApplication(final AppInfo appInfo, ClassLoader classLoa systemInstance.fireEvent(new AssemblerAfterApplicationCreated(appInfo, appContext, allDeployments)); logger.info("createApplication.success", appInfo.path); - //required by spec EE.5.3.4 - if(setAppNamingContextReadOnly(appContext, allDeployments)) { - logger.info("createApplication.naming", appInfo.path); - } - return appContext; } catch (final ValidationException | DeploymentException ve) { throw ve; @@ -1117,29 +1118,59 @@ private AppContext createApplication(final AppInfo appInfo, ClassLoader classLoa } } - boolean setAppNamingContextReadOnly(final AppContext appContext, final List allDeployments) { - if("true".equals(SystemInstance.get().getProperty(FORCE_READ_ONLY_APP_NAMING, "true"))) { - for(BeanContext beanContext : allDeployments) { - markReadOnly(beanContext.getJndiContext()); - } + /** + * Marks the component naming contexts of the deployments that were just created read only, as + * required by EE.5.3.4 and Enterprise Beans 10.4.4. + * + * This runs at the end of {@link #startEjbs} rather than at the end of {@link #createApplication} + * on purpose, for two reasons. + * + * The web modules of an ear are skipped by {@link #isSkip} and deployed later, from + * TomcatWebAppBuilder, so marking in createApplication would both miss their java:comp and make + * the container's own bindings - JndiBuilder binds app/<module>/<bean> into the app context - + * fail with OperationNotSupportedException on that later pass. And the containers themselves bind + * comp/EJBContext, comp/WebServiceContext and comp/TimerService into each bean enc while + * deploying, which happens in startEjbs, after initEjbs has returned. + * + * The bean contexts are closed as soon as their own deployment pass is done. The shared + * application context has to wait until no further module can bind into it, which for an ear + * means its last web module. + * + * @return true if anything was marked read only + */ + boolean setAppNamingContextReadOnly(final List allDeployments) { + final AppContext appContext = appContextOf(allDeployments); + if (appContext == null || !appContext.isReadOnlyAppNamingContext()) { + return false; + } - // servlets, JSF beans and other web components resolve java:comp/java:module/java:app through the - // web and app contexts rather than through a BeanContext, so they need the same treatment - if(appContext != null) { - for(final WebContext webContext : appContext.getWebContexts()) { - markReadOnly(webContext.getJndiEnc()); - } - markReadOnly(appContext.getAppJndiContext()); - } - return true; + for (final BeanContext beanContext : allDeployments) { + markReadOnly(beanContext.getJndiContext()); } - return false; + + if (appContext.lastModuleDeployed()) { + markReadOnly(appContext.getAppJndiContext()); + } + return true; + } + + /** + * The application scoped opt-out wins over the container wide one so that a single legacy + * application writing into its own naming context does not force the whole container off the + * behaviour the specification requires. + */ + private static boolean isReadOnlyAppNaming(final AppInfo appInfo) { + final String globalDefault = SystemInstance.get().getProperty(FORCE_READ_ONLY_APP_NAMING, "true"); + final String value = appInfo == null + ? globalDefault + : appInfo.properties.getProperty(FORCE_READ_ONLY_APP_NAMING, globalDefault); + return Boolean.parseBoolean(value); } private static void markReadOnly(final Context ctx) { - if(IvmContext.class.isInstance(ctx)) { + if (IvmContext.class.isInstance(ctx)) { IvmContext.class.cast(ctx).setReadOnly(true); - } else if(ContextHandler.class.isInstance(ctx)) { + } else if (ContextHandler.class.isInstance(ctx)) { ContextHandler.class.cast(ctx).setReadOnly(); } } @@ -1720,9 +1751,25 @@ public void startEjbs(final boolean start, final List allDeployment throw new OpenEJBException("Error starting '" + deployment.getEjbName() + "'. Exception: " + t.getClass() + ": " + t.getMessage(), t); } } + + // the containers bind comp/EJBContext and friends into the bean encs while deploying, so + // the naming contexts can only be closed for writing once all of that is done + if (setAppNamingContextReadOnly(allDeployments)) { + logger.info("createApplication.naming", appContextOf(allDeployments).getId()); + } } } + private static AppContext appContextOf(final List allDeployments) { + for (final BeanContext beanContext : allDeployments) { + final ModuleContext moduleContext = beanContext.getModuleContext(); + if (moduleContext != null && moduleContext.getAppContext() != null) { + return moduleContext.getAppContext(); + } + } + return null; + } + @SuppressWarnings("unchecked") private void deployMBean(final WebBeansContext wc, final ClassLoader cl, final String mbeanClass, final Properties appMbeans, final String id) { if (LocalMBeanServer.isJMXActive()) { diff --git a/container/openejb-core/src/test/java/org/apache/openejb/assembler/classic/AppNamingReadOnlyTest.java b/container/openejb-core/src/test/java/org/apache/openejb/assembler/classic/AppNamingReadOnlyTest.java index 6b6cf06a982..91c80e716d1 100644 --- a/container/openejb-core/src/test/java/org/apache/openejb/assembler/classic/AppNamingReadOnlyTest.java +++ b/container/openejb-core/src/test/java/org/apache/openejb/assembler/classic/AppNamingReadOnlyTest.java @@ -36,89 +36,119 @@ import junit.framework.TestCase; public class AppNamingReadOnlyTest extends TestCase { - - public void testReadOnlyAppNamingContext() throws SystemException, URISyntaxException { - - String originalValue = System.getProperty(Assembler.FORCE_READ_ONLY_APP_NAMING); - System.setProperty(Assembler.FORCE_READ_ONLY_APP_NAMING, Boolean.TRUE.toString()); - try { - List mockBeanContextsList = getMockBeanContextsList(); - - Assembler assembler = new Assembler(); - assembler.setAppNamingContextReadOnly(null, mockBeanContextsList); - - Context beanNamingContext = mockBeanContextsList.get(0).getJndiContext(); - //may return null or throw exception depending on openejb.jndiExceptionOnFailedWrite value; - //this test is not intended to test read-only behavior (null/exception); it should check whether naming context is marked as read only - try { - Context subContext = beanNamingContext.createSubcontext("sub"); - assertNull(subContext); - } catch (OperationNotSupportedException e) { - //ok - } catch (NamingException e) { - throw new AssertionError(); - } - } finally { - if(originalValue == null) { - System.clearProperty(Assembler.FORCE_READ_ONLY_APP_NAMING); - } else { - System.setProperty(Assembler.FORCE_READ_ONLY_APP_NAMING, originalValue); - } - SystemInstance.reset(); - } + + private AppContext appContext; + + @Override + protected void tearDown() throws Exception { + SystemInstance.reset(); + super.tearDown(); } - - //read-only is the spec-mandated default (EE.5.3.4, Enterprise Beans 10.4.4) - public void testAppNamingContextReadOnlyByDefault() throws SystemException, URISyntaxException { - List mockBeanContextsList = getMockBeanContextsList(); + public void testReadOnlyAppNamingContext() throws SystemException, URISyntaxException { + final List beanContexts = getMockBeanContextsList(); + appContext.setReadOnlyAppNamingContext(true); - Assembler assembler = new Assembler(); - assertTrue(assembler.setAppNamingContextReadOnly(null, mockBeanContextsList)); + final Assembler assembler = new Assembler(); + assertTrue(assembler.setAppNamingContextReadOnly(beanContexts)); - Context beanNamingContext = mockBeanContextsList.get(0).getJndiContext(); - try { - assertNull(beanNamingContext.createSubcontext("sub")); - } catch (OperationNotSupportedException e) { - //ok - } catch (NamingException e) { - throw new AssertionError(e); - } + assertWriteRefused(beanContexts.get(0).getJndiContext()); + } + + // the shared application context must stay writable while further modules can still bind into it + public void testAppContextStaysWritableUntilTheLastModule() throws Exception { + final List beanContexts = getMockBeanContextsList(); + appContext.setReadOnlyAppNamingContext(true); + // one web module still to come, as for an ear whose war is started by TomcatWebAppBuilder + appContext.setPendingLateModules(1); + + final Assembler assembler = new Assembler(); + assertTrue(assembler.setAppNamingContextReadOnly(beanContexts)); + + // the bean context is closed straight away, it can no longer receive bindings + assertWriteRefused(beanContexts.get(0).getJndiContext()); + + // but the application context still accepts the container's own bindings, as JndiBuilder + // does for the ejb modules of an ear's web modules, which deploy later + assertNotNull(appContext.getAppJndiContext().createSubcontext("app/late")); + + // and once the last module is in, it closes too + assertTrue(assembler.setAppNamingContextReadOnly(beanContexts)); + assertWriteRefused(appContext.getAppJndiContext()); + } + + // an ear with two web modules only closes its application context on the third pass: the one + // createApplication does, then one per web module started by the web app builder + public void testAppContextWaitsForEveryLateModule() throws Exception { + getMockBeanContextsList(); + appContext.setPendingLateModules(2); + + assertFalse("createApplication pass must not close the app context", appContext.lastModuleDeployed()); + assertFalse("first web module must not close the app context", appContext.lastModuleDeployed()); + assertTrue("last web module closes the app context", appContext.lastModuleDeployed()); + // and it stays closed for any further call + assertTrue(appContext.lastModuleDeployed()); } - //the legacy writable behavior is still available as an explicit opt-out + // a standalone module is fully deployed in one pass, so it closes immediately + public void testStandaloneModuleClosesOnTheFirstPass() throws Exception { + getMockBeanContextsList(); + appContext.setPendingLateModules(0); + + assertTrue(appContext.lastModuleDeployed()); + } + + // read-only is the spec-mandated default (EE.5.3.4, Enterprise Beans 10.4.4) + public void testAppNamingContextReadOnlyByDefault() throws SystemException, URISyntaxException { + final List beanContexts = getMockBeanContextsList(); + // the flag is what createApplication derives from the properties, defaulting to true + appContext.setReadOnlyAppNamingContext(true); + + final Assembler assembler = new Assembler(); + assertTrue(assembler.setAppNamingContextReadOnly(beanContexts)); + + assertWriteRefused(beanContexts.get(0).getJndiContext()); + } + + // the legacy writable behavior is still available as an explicit opt-out public void testAppNamingContextWritableWhenDisabled() throws SystemException, URISyntaxException, NamingException { + final List beanContexts = getMockBeanContextsList(); + appContext.setReadOnlyAppNamingContext(false); - String originalValue = System.getProperty(Assembler.FORCE_READ_ONLY_APP_NAMING); - System.setProperty(Assembler.FORCE_READ_ONLY_APP_NAMING, Boolean.FALSE.toString()); + final Assembler assembler = new Assembler(); + assertFalse(assembler.setAppNamingContextReadOnly(beanContexts)); + + assertNotNull(beanContexts.get(0).getJndiContext().createSubcontext("sub")); + } + + /** + * A read only context either throws OperationNotSupportedException or silently ignores the write, + * depending on openejb.jndiExceptionOnFailedWrite. This only checks that the context is marked. + */ + private void assertWriteRefused(final Context context) { try { - List mockBeanContextsList = getMockBeanContextsList(); - - Assembler assembler = new Assembler(); - assertFalse(assembler.setAppNamingContextReadOnly(null, mockBeanContextsList)); - - Context beanNamingContext = mockBeanContextsList.get(0).getJndiContext(); - assertNotNull(beanNamingContext.createSubcontext("sub")); - } finally { - if(originalValue == null) { - System.clearProperty(Assembler.FORCE_READ_ONLY_APP_NAMING); - } else { - System.setProperty(Assembler.FORCE_READ_ONLY_APP_NAMING, originalValue); - } - SystemInstance.reset(); + assertNull(context.createSubcontext("sub")); + } catch (final OperationNotSupportedException e) { + // ok + } catch (final NamingException e) { + throw new AssertionError(e); } } - + private List getMockBeanContextsList() throws SystemException, URISyntaxException { - IvmContext context = new IvmContext(); - - AppContext mockAppContext = new AppContext("appId", SystemInstance.get(), this.getClass().getClassLoader(), context, context, false); - ModuleContext mockModuleContext = new ModuleContext("moduleId", new URI(""), "uniqueId", mockAppContext, context, this.getClass().getClassLoader()); - BeanContext mockBeanContext = new BeanContext("test", context, mockModuleContext, this.getClass(), this.getClass(), new HashMap<>()); - - List beanContextsList = new ArrayList<>(); - beanContextsList.add(mockBeanContext); - - return beanContextsList; + final IvmContext beanJndiContext = new IvmContext(); + final IvmContext appJndiContext = new IvmContext(); + + appContext = new AppContext("appId", SystemInstance.get(), this.getClass().getClassLoader(), + appJndiContext, appJndiContext, false); + final ModuleContext mockModuleContext = new ModuleContext("moduleId", new URI(""), "uniqueId", appContext, + beanJndiContext, this.getClass().getClassLoader()); + final BeanContext mockBeanContext = new BeanContext("test", beanJndiContext, mockModuleContext, + this.getClass(), this.getClass(), new HashMap<>()); + + final List beanContextsList = new ArrayList<>(); + beanContextsList.add(mockBeanContext); + + return beanContextsList; } } diff --git a/container/openejb-core/src/test/java/org/apache/openejb/core/ivm/naming/JavaCompReadOnlyTest.java b/container/openejb-core/src/test/java/org/apache/openejb/core/ivm/naming/JavaCompReadOnlyTest.java index 4e3b5807bff..462f950846b 100644 --- a/container/openejb-core/src/test/java/org/apache/openejb/core/ivm/naming/JavaCompReadOnlyTest.java +++ b/container/openejb-core/src/test/java/org/apache/openejb/core/ivm/naming/JavaCompReadOnlyTest.java @@ -30,6 +30,7 @@ import org.apache.openejb.loader.SystemInstance; import javax.naming.Context; +import javax.naming.NameNotFoundException; import javax.naming.OperationNotSupportedException; /** @@ -53,8 +54,8 @@ private AppContext deploy() throws Exception { } public void testCompContextRefusesWrites() throws Exception { - final AppContext app = deploy(); try { + final AppContext app = deploy(); final BeanContext bean = app.getBeanContexts().get(0); final Context comp = bean.getJndiContext(); @@ -63,33 +64,30 @@ public void testCompContextRefusesWrites() throws Exception { assertWriteRefused(comp, "rename", () -> comp.rename("comp", "renameTo")); assertWriteRefused(comp, "unbind", () -> comp.unbind("comp")); assertWriteRefused(comp, "destroySubcontext", () -> comp.destroySubcontext("comp")); + assertWriteRefused(comp, "createSubcontext", () -> comp.createSubcontext("newName")); - // createSubcontext either throws or returns null, depending on jndiExceptionOnFailedWrite - try { - assertNull(comp.createSubcontext("newName")); - } catch (final OperationNotSupportedException expected) { - // ok - } - - // nothing the writes attempted may be observable afterwards + // none of the attempted writes may be observable afterwards assertNotBound(comp, "newName"); assertNotBound(comp, "renameTo"); - // and the pre-existing binding must have survived unbind/rename/destroySubcontext - assertTrue(comp.lookup("comp") instanceof Context); + // and what was already bound must have survived rename, unbind and destroySubcontext + assertTrue("comp must still be bound", comp.lookup("comp") instanceof Context); } finally { SystemInstance.reset(); } } public void testAppContextRefusesWrites() throws Exception { - final AppContext app = deploy(); try { + final AppContext app = deploy(); final Context appCtx = app.getAppJndiContext(); assertWriteRefused(appCtx, "bind", () -> appCtx.bind("newName", "newValue")); + assertWriteRefused(appCtx, "rebind", () -> appCtx.rebind("newName", "newValue")); + assertWriteRefused(appCtx, "createSubcontext", () -> appCtx.createSubcontext("newName")); + assertNotBound(appCtx, "newName"); - assertTrue(appCtx.lookup("app") instanceof Context); + assertTrue("app must still be bound", appCtx.lookup("app") instanceof Context); } finally { SystemInstance.reset(); } @@ -99,12 +97,16 @@ private interface Write { void run() throws Exception; } + /** + * A read only context may either throw OperationNotSupportedException or silently ignore the + * write, depending on openejb.jndiExceptionOnFailedWrite. Both are accepted here; that the write + * did not take effect is asserted separately by {@link #assertNotBound}. + */ private void assertWriteRefused(final Context ctx, final String operation, final Write write) { try { write.run(); - fail(operation + " should have been refused on a read-only naming context"); } catch (final OperationNotSupportedException expected) { - // ok + // ok, the context refused the write outright } catch (final Exception e) { throw new AssertionError("unexpected exception from " + operation, e); } @@ -113,7 +115,7 @@ private void assertWriteRefused(final Context ctx, final String operation, final private void assertNotBound(final Context ctx, final String name) throws Exception { try { assertNull(name + " must not be bound", ctx.lookup(name)); - } catch (final javax.naming.NameNotFoundException expected) { + } catch (final NameNotFoundException expected) { // ok } } From 3c75c91de12bd8615fdfafd8ce2815e20830de04 Mon Sep 17 00:00:00 2001 From: Markus Jung Date: Wed, 29 Jul 2026 21:10:58 +0200 Subject: [PATCH 3/3] TOMEE-4654 - Keep the writable naming context as the default Review feedback: enabling the read only component naming context for everyone is a behaviour change that would break the applications writing into their own naming context after deployment, so openejb.forceReadOnlyAppNamingContext goes back to being off by default and the TCK turns it on for its runs. The lifecycle work stays as it is: when the flag is set the contexts are still marked at the end of startEjbs, so the ear web modules deployed later by the web app builders are covered and the container's own binds all run first. Makes the constant public so the harness and tests outside assembler.classic can set it, opts JavaCompReadOnlyTest into the flag explicitly, and restores EmbeddedTomEEContainerTest.testEjbCanCreateSubContextByDefault to asserting the write succeeds. --- .../embedded/EmbeddedTomEEContainerTest.java | 13 +++++-------- .../openejb/assembler/classic/Assembler.java | 14 +++++++++----- .../assembler/classic/AppNamingReadOnlyTest.java | 11 ++++++----- .../core/ivm/naming/JavaCompReadOnlyTest.java | 5 ++++- 4 files changed, 24 insertions(+), 19 deletions(-) diff --git a/arquillian/arquillian-tomee-embedded/src/test/java/org/apache/openejb/arquillian/embedded/EmbeddedTomEEContainerTest.java b/arquillian/arquillian-tomee-embedded/src/test/java/org/apache/openejb/arquillian/embedded/EmbeddedTomEEContainerTest.java index 25e0787a204..72501a321b5 100644 --- a/arquillian/arquillian-tomee-embedded/src/test/java/org/apache/openejb/arquillian/embedded/EmbeddedTomEEContainerTest.java +++ b/arquillian/arquillian-tomee-embedded/src/test/java/org/apache/openejb/arquillian/embedded/EmbeddedTomEEContainerTest.java @@ -35,7 +35,6 @@ import java.net.URL; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; @RunWith(Arquillian.class) @@ -73,21 +72,19 @@ public void restServiceIsDeployed() throws Exception { } /** - * EE.5.3.4 and Enterprise Beans 10.4.4 require the component naming context to be read only, so - * createSubcontext must not succeed from within a bean. Depending on - * openejb.jndiExceptionOnFailedWrite the container either throws - * OperationNotSupportedException or ignores the write and returns null; both are refusals, what - * must not happen is the subcontext being created. + * EE.5.3.4 and Enterprise Beans 10.4.4 require the component naming context to be read only, but + * TomEE only enforces that when openejb.forceReadOnlyAppNamingContext is turned on, so that + * applications writing into their own naming context keep working. Without it the write succeeds. */ @Test - public void testEjbCannotCreateSubContextByDefault() throws Exception { + public void testEjbCanCreateSubContextByDefault() throws Exception { String originalValue = System.getProperty(javax.naming.Context.INITIAL_CONTEXT_FACTORY); if(originalValue == null) { System.setProperty(javax.naming.Context.INITIAL_CONTEXT_FACTORY, InitContextFactory.class.getName()); } try { String result = ejb.createSubContext(); - assertNotEquals("Sub context creation must be refused on a read-only naming context", "created", result); + assertEquals("created", result); } finally { if(originalValue == null) { System.clearProperty(javax.naming.Context.INITIAL_CONTEXT_FACTORY); diff --git a/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java b/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java index 0ab66f9f34e..2cb499419e9 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java @@ -283,7 +283,7 @@ public class Assembler extends AssemblerTool implements org.apache.openejb.spi.A public static final String TIMER_STORE_CLASS = "timerStore.class"; private static final ReentrantLock lock = new ReentrantLock(true); public static final String OPENEJB_TIMERS_ON = "openejb.timers.on"; - static final String FORCE_READ_ONLY_APP_NAMING = "openejb.forceReadOnlyAppNamingContext"; + public static final String FORCE_READ_ONLY_APP_NAMING = "openejb.forceReadOnlyAppNamingContext"; public static final Class[] VALIDATOR_FACTORY_INTERFACES = new Class[]{ValidatorFactory.class, Serializable.class}; public static final Class[] VALIDATOR_INTERFACES = new Class[]{Validator.class}; @@ -1155,12 +1155,16 @@ boolean setAppNamingContextReadOnly(final List allDeployments) { } /** - * The application scoped opt-out wins over the container wide one so that a single legacy - * application writing into its own naming context does not force the whole container off the - * behaviour the specification requires. + * Off by default: making the component naming context read only is what the specification + * requires, but turning it on for everyone would break the applications that write into their + * own naming context after deployment. It stays opt-in until that can be a release wide + * behaviour change. + * + * The application scoped setting wins over the container wide one so a single application can + * opt in or out without the whole container following it. */ private static boolean isReadOnlyAppNaming(final AppInfo appInfo) { - final String globalDefault = SystemInstance.get().getProperty(FORCE_READ_ONLY_APP_NAMING, "true"); + final String globalDefault = SystemInstance.get().getProperty(FORCE_READ_ONLY_APP_NAMING, "false"); final String value = appInfo == null ? globalDefault : appInfo.properties.getProperty(FORCE_READ_ONLY_APP_NAMING, globalDefault); diff --git a/container/openejb-core/src/test/java/org/apache/openejb/assembler/classic/AppNamingReadOnlyTest.java b/container/openejb-core/src/test/java/org/apache/openejb/assembler/classic/AppNamingReadOnlyTest.java index 91c80e716d1..cd1f62fd8e0 100644 --- a/container/openejb-core/src/test/java/org/apache/openejb/assembler/classic/AppNamingReadOnlyTest.java +++ b/container/openejb-core/src/test/java/org/apache/openejb/assembler/classic/AppNamingReadOnlyTest.java @@ -98,10 +98,11 @@ public void testStandaloneModuleClosesOnTheFirstPass() throws Exception { assertTrue(appContext.lastModuleDeployed()); } - // read-only is the spec-mandated default (EE.5.3.4, Enterprise Beans 10.4.4) - public void testAppNamingContextReadOnlyByDefault() throws SystemException, URISyntaxException { + // opting in gives the read only component naming context the specification requires + // (EE.5.3.4, Enterprise Beans 10.4.4) + public void testAppNamingContextReadOnlyWhenEnabled() throws SystemException, URISyntaxException { final List beanContexts = getMockBeanContextsList(); - // the flag is what createApplication derives from the properties, defaulting to true + // the flag is what createApplication derives from the properties appContext.setReadOnlyAppNamingContext(true); final Assembler assembler = new Assembler(); @@ -110,8 +111,8 @@ public void testAppNamingContextReadOnlyByDefault() throws SystemException, URIS assertWriteRefused(beanContexts.get(0).getJndiContext()); } - // the legacy writable behavior is still available as an explicit opt-out - public void testAppNamingContextWritableWhenDisabled() throws SystemException, URISyntaxException, NamingException { + // the naming context stays writable unless the application opts in + public void testAppNamingContextWritableByDefault() throws SystemException, URISyntaxException, NamingException { final List beanContexts = getMockBeanContextsList(); appContext.setReadOnlyAppNamingContext(false); diff --git a/container/openejb-core/src/test/java/org/apache/openejb/core/ivm/naming/JavaCompReadOnlyTest.java b/container/openejb-core/src/test/java/org/apache/openejb/core/ivm/naming/JavaCompReadOnlyTest.java index 462f950846b..c140a67c1f4 100644 --- a/container/openejb-core/src/test/java/org/apache/openejb/core/ivm/naming/JavaCompReadOnlyTest.java +++ b/container/openejb-core/src/test/java/org/apache/openejb/core/ivm/naming/JavaCompReadOnlyTest.java @@ -35,11 +35,14 @@ /** * The Enterprise Beans spec (10.4.4) and EE.5.3.4 require the component naming context to be read-only: - * writes against java:comp and friends must not take effect. + * writes against java:comp and friends must not take effect. TomEE only does this when + * openejb.forceReadOnlyAppNamingContext is turned on, so the deployments here opt in explicitly. */ public class JavaCompReadOnlyTest extends TestCase { private AppContext deploy() throws Exception { + SystemInstance.get().setProperty(Assembler.FORCE_READ_ONLY_APP_NAMING, Boolean.TRUE.toString()); + final ConfigurationFactory config = new ConfigurationFactory(); final Assembler assembler = new Assembler();