This is an automated email from the ASF dual-hosted git repository. jungm pushed a commit to branch claude/tomee-4654-fix-6ad09c in repository https://gitbox.apache.org/repos/asf/tomee.git
commit d9b620dda5fd5c251cdcf366920273d3827883c6 Author: Markus Jung <[email protected]> AuthorDate: Thu Jul 23 20:44:30 2026 +0200 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 +++-- .../assembler/classic/AppNamingReadOnlyTest.java | 55 ++++++--- .../core/ivm/naming/JavaCompReadOnlyTest.java | 123 +++++++++++++++++++++ 3 files changed, 184 insertions(+), 23 deletions(-) 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 50be16d782..0a8dcd0a74 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 @@ public class Assembler extends AssemblerTool implements org.apache.openejb.spi.A 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 @@ public class Assembler extends AssemblerTool implements org.apache.openejb.spi.A } } - boolean setAppNamingContextReadOnly(final List<BeanContext> allDeployments) { - if("true".equals(SystemInstance.get().getProperty(FORCE_READ_ONLY_APP_NAMING, "false"))) { + boolean setAppNamingContextReadOnly(final AppContext appContext, final List<BeanContext> 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<String> getDuplicates(final AppInfo appInfo) { final List<String> 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 8262ddd0da..6b6cf06a98 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 class AppNamingReadOnlyTest extends TestCase { System.setProperty(Assembler.FORCE_READ_ONLY_APP_NAMING, Boolean.TRUE.toString()); try { List<BeanContext> 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 class AppNamingReadOnlyTest extends TestCase { } } - //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<BeanContext> mockBeanContextsList = getMockBeanContextsList(); - - Assembler assembler = new Assembler(); - assembler.setAppNamingContextReadOnly(mockBeanContextsList); - - Context beanNamingContext = mockBeanContextsList.get(0).getJndiContext(); - Context subContext = beanNamingContext.createSubcontext("sub"); - - assertNotNull(subContext); + List<BeanContext> 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<BeanContext> 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<BeanContext> 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 0000000000..4e3b5807bf --- /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 { + } +}
