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..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 @@ -71,6 +71,11 @@ 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, 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 testEjbCanCreateSubContextByDefault() throws Exception { String originalValue = System.getProperty(javax.naming.Context.INITIAL_CONTEXT_FACTORY); @@ -79,11 +84,11 @@ public void testEjbCanCreateSubContextByDefault() throws Exception { } try { String result = ejb.createSubContext(); - assertEquals("Cannot create sub 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/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 50be16d782f..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 @@ -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; @@ -282,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}; @@ -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(allDeployments)) { - logger.info("createApplication.naming", appInfo.path); - } - return appContext; } catch (final ValidationException | DeploymentException ve) { throw ve; @@ -1117,20 +1118,65 @@ private AppContext createApplication(final AppInfo appInfo, ClassLoader classLoa } } + /** + * 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) { - if("true".equals(SystemInstance.get().getProperty(FORCE_READ_ONLY_APP_NAMING, "false"))) { - 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(); - } - } - return true; + final AppContext appContext = appContextOf(allDeployments); + if (appContext == null || !appContext.isReadOnlyAppNamingContext()) { + return false; + } + + for (final BeanContext beanContext : allDeployments) { + markReadOnly(beanContext.getJndiContext()); + } + + if (appContext.lastModuleDeployed()) { + markReadOnly(appContext.getAppJndiContext()); + } + return true; + } + + /** + * 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, "false"); + 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)) { + IvmContext.class.cast(ctx).setReadOnly(true); + } else if (ContextHandler.class.isInstance(ctx)) { + ContextHandler.class.cast(ctx).setReadOnly(); } - return false; } private List getDuplicates(final AppInfo appInfo) { @@ -1709,7 +1755,23 @@ 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") 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..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 @@ -36,62 +36,120 @@ 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(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(); + } + + public void testReadOnlyAppNamingContext() throws SystemException, URISyntaxException { + final List beanContexts = getMockBeanContextsList(); + appContext.setReadOnlyAppNamingContext(true); + + final Assembler assembler = new Assembler(); + assertTrue(assembler.setAppNamingContextReadOnly(beanContexts)); + + 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()); } - - //check TOMEE behavior is backward compatible + + // 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()); + } + + // 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 + appContext.setReadOnlyAppNamingContext(true); + + final Assembler assembler = new Assembler(); + assertTrue(assembler.setAppNamingContextReadOnly(beanContexts)); + + assertWriteRefused(beanContexts.get(0).getJndiContext()); + } + + // the naming context stays writable unless the application opts in public void testAppNamingContextWritableByDefault() throws SystemException, URISyntaxException, NamingException { + final List beanContexts = getMockBeanContextsList(); + appContext.setReadOnlyAppNamingContext(false); + + final Assembler assembler = new Assembler(); + assertFalse(assembler.setAppNamingContextReadOnly(beanContexts)); - List mockBeanContextsList = getMockBeanContextsList(); - - Assembler assembler = new Assembler(); - assembler.setAppNamingContextReadOnly(mockBeanContextsList); - - Context beanNamingContext = mockBeanContextsList.get(0).getJndiContext(); - Context subContext = beanNamingContext.createSubcontext("sub"); - - assertNotNull(subContext); + 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 { + 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 new file mode 100644 index 00000000000..c140a67c1f4 --- /dev/null +++ b/container/openejb-core/src/test/java/org/apache/openejb/core/ivm/naming/JavaCompReadOnlyTest.java @@ -0,0 +1,128 @@ +/** + * 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.NameNotFoundException; +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. 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(); + + 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 { + try { + final AppContext app = deploy(); + 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")); + assertWriteRefused(comp, "createSubcontext", () -> comp.createSubcontext("newName")); + + // none of the attempted writes may be observable afterwards + assertNotBound(comp, "newName"); + assertNotBound(comp, "renameTo"); + + // 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 { + 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("app must still be bound", appCtx.lookup("app") instanceof Context); + } finally { + SystemInstance.reset(); + } + } + + 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(); + } catch (final OperationNotSupportedException expected) { + // ok, the context refused the write outright + } 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 NameNotFoundException expected) { + // ok + } + } + + public static class Bean { + } +}