SLING-7805 - NPE in Oak SessionImpl when starting up
This reverts commit 335d0aec54d8674b275b51286a18fff92e0eaccc, reversing
changes made to bf18d14584cacbadcaec443821433e78669a43a6.
It also reverts commits 5a36b33e05b1b05827993ad09bd4e389674cd8a1 and
dde168f8e1c205a04a972b8b1997c002b41055d3.
diff --git a/pom.xml b/pom.xml
index b1059c9..1d18f59 100644
--- a/pom.xml
+++ b/pom.xml
@@ -54,7 +54,6 @@
<plugin>
<groupId>org.apache.sling</groupId>
<artifactId>maven-sling-plugin</artifactId>
- <version>2.3.2</version>
<executions>
<execution>
<id>generate-adapter-metadata</id>
@@ -177,7 +176,7 @@
<dependency>
<groupId>org.apache.sling</groupId>
<artifactId>org.apache.sling.api</artifactId>
- <version>2.18.2</version>
+ <version>2.16.4</version>
<scope>provided</scope>
</dependency>
<dependency>
diff --git a/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrProviderStateFactory.java b/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrProviderStateFactory.java
index 2f43526..8767fa9 100644
--- a/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrProviderStateFactory.java
+++ b/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrProviderStateFactory.java
@@ -145,12 +145,7 @@
@Nonnull final Map<String, Object> authenticationInfo,
@Nullable final BundleContext ctx
) throws LoginException {
- boolean explicitSessionUsed = (getSession(authenticationInfo) != null);
- final Session impersonatedSession = handleImpersonation(session, authenticationInfo, logoutSession, explicitSessionUsed);
- if (impersonatedSession != session && explicitSessionUsed) {
- // update the session in the auth info map in case the resolver gets cloned in the future
- authenticationInfo.put(JcrResourceConstants.AUTHENTICATION_INFO_SESSION, impersonatedSession);
- }
+ final Session impersonatedSession = handleImpersonation(session, authenticationInfo, logoutSession);
// if we're actually impersonating, we're responsible for closing the session we've created, regardless
// of what the original logoutSession value was.
boolean doLogoutSession = logoutSession || (impersonatedSession != session);
@@ -172,35 +167,25 @@
* @param logoutSession
* whether to logout the <code>session</code> after impersonation
* or not.
- * @param explicitSessionUsed
- * whether the JCR session was explicitly given in the auth info or not.
* @return The original session or impersonated session.
* @throws LoginException
* If something goes wrong.
*/
private static Session handleImpersonation(final Session session, final Map<String, Object> authenticationInfo,
- final boolean logoutSession, boolean explicitSessionUsed) throws LoginException {
+ final boolean logoutSession) throws LoginException {
final String sudoUser = getSudoUser(authenticationInfo);
- // Do we need session.impersonate() because we are asked to impersonate another user?
- boolean needsSudo = (sudoUser != null) && !session.getUserID().equals(sudoUser);
- // Do we need session.impersonate() to get an independent copy of the session we were given in the auth info?
- boolean needsCloning = !needsSudo && explicitSessionUsed && authenticationInfo.containsKey(ResourceProvider.AUTH_CLONE);
- try {
- if (needsSudo) {
- SimpleCredentials creds = new SimpleCredentials(sudoUser, new char[0]);
+ if (sudoUser != null && !session.getUserID().equals(sudoUser)) {
+ try {
+ final SimpleCredentials creds = new SimpleCredentials(sudoUser, new char[0]);
copyAttributes(creds, authenticationInfo);
creds.setAttribute(ResourceResolver.USER_IMPERSONATOR, session.getUserID());
return session.impersonate(creds);
- } else if (needsCloning) {
- SimpleCredentials creds = new SimpleCredentials(session.getUserID(), new char[0]);
- copyAttributes(creds, authenticationInfo);
- return session.impersonate(creds);
- }
- } catch (final RepositoryException re) {
- throw getLoginException(re);
- } finally {
- if (logoutSession) {
- session.logout();
+ } catch (final RepositoryException re) {
+ throw getLoginException(re);
+ } finally {
+ if (logoutSession) {
+ session.logout();
+ }
}
}
return session;
diff --git a/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProviderSessionHandlingTest.java b/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProviderSessionHandlingTest.java
deleted file mode 100644
index a471446..0000000
--- a/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProviderSessionHandlingTest.java
+++ /dev/null
@@ -1,199 +0,0 @@
-/*
- * 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.sling.jcr.resource.internal.helper.jcr;
-
-import static org.hamcrest.Matchers.is;
-import static org.hamcrest.Matchers.not;
-import static org.hamcrest.Matchers.sameInstance;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotSame;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assume.assumeThat;
-import static org.junit.Assume.assumeTrue;
-import static org.mockito.Matchers.anyString;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
-import javax.jcr.Session;
-
-import org.apache.sling.api.resource.ResourceResolver;
-import org.apache.sling.api.resource.ResourceResolverFactory;
-import org.apache.sling.commons.testing.jcr.RepositoryProvider;
-import org.apache.sling.jcr.api.SlingRepository;
-import org.apache.sling.jcr.resource.api.JcrResourceConstants;
-import org.apache.sling.spi.resource.provider.ResolveContext;
-import org.apache.sling.spi.resource.provider.ResourceProvider;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-import org.junit.runners.Parameterized.Parameter;
-import org.junit.runners.Parameterized.Parameters;
-import org.mockito.Mockito;
-import org.osgi.framework.ServiceReference;
-import org.osgi.service.component.ComponentContext;
-
-@RunWith(Parameterized.class)
-public class JcrResourceProviderSessionHandlingTest {
-
- private enum LoginStyle {USER, SESSION};
-
- private static final String AUTH_USER = "admin";
- private static final char[] AUTH_PASSWORD = "admin".toCharArray();
- private static final String SUDO_USER = "anonymous";
-
- @Parameters(name = "loginStyle= {0}, sudo = {1}, clone = {2}")
- public static List<Object[]> data() {
-
- LoginStyle[] loginStyles = LoginStyle.values();
- boolean[] sudoOptions = new boolean[] {false, true};
- boolean[] cloneOptions = new boolean[] {false, true};
-
- // Generate all possible combinations into data.
- List<Object[]> data = new ArrayList<>();
- Object[] dataPoint = new Object[3];
- for (LoginStyle loginStyle : loginStyles) {
- dataPoint[0] = loginStyle;
- for (boolean sudo : sudoOptions) {
- dataPoint[1] = sudo;
- for (boolean clone : cloneOptions) {
- dataPoint[2] = clone;
- data.add(dataPoint.clone());
- }
- }
- }
- return data;
- }
-
- @Parameter(0)
- public LoginStyle loginStyle;
-
- @Parameter(1)
- public boolean useSudo;
-
- @Parameter(2)
- public boolean doClone;
-
- // Session we're using when loginStyle == SESSION, null otherwise.
- private Session explicitSession;
-
- private JcrResourceProvider jcrResourceProvider;
- private JcrProviderState jcrProviderState;
-
- @Before
- public void setUp() throws Exception {
- SlingRepository repo = RepositoryProvider.instance().getRepository();
- Map<String, Object> authInfo = new HashMap<>();
- switch (loginStyle) {
- case USER:
- authInfo.put(ResourceResolverFactory.USER, AUTH_USER);
- authInfo.put(ResourceResolverFactory.PASSWORD, AUTH_PASSWORD);
- break;
- case SESSION:
- explicitSession = repo.loginAdministrative(null);
- authInfo.put(JcrResourceConstants.AUTHENTICATION_INFO_SESSION, explicitSession);
- break;
- }
-
- if (useSudo) {
- authInfo.put(ResourceResolverFactory.USER_IMPERSONATION, SUDO_USER);
- }
-
- if (doClone) {
- authInfo.put(ResourceProvider.AUTH_CLONE, true);
- }
-
- ComponentContext ctx = mock(ComponentContext.class);
- when(ctx.locateService(anyString(), Mockito.<ServiceReference<Object>>any())).thenReturn(repo);
-
- jcrResourceProvider = new JcrResourceProvider();
- jcrResourceProvider.activate(ctx);
-
- jcrProviderState = jcrResourceProvider.authenticate(authInfo);
- }
-
- @After
- public void tearDown() throws Exception {
-
- // Some tests do a logout, so check for liveness before trying to log out.
- if (jcrProviderState.getSession().isLive()) {
- jcrResourceProvider.logout(jcrProviderState);
- }
-
- jcrResourceProvider.deactivate();
-
- if (explicitSession != null) {
- explicitSession.logout();
- }
- }
-
- @Test
- public void sessionUsesCorrectUser() {
- String expectedUser = useSudo ? SUDO_USER : AUTH_USER;
- assertEquals(expectedUser, jcrProviderState.getSession().getUserID());
- }
-
- @Test
- public void explicitSessionNotClosedOnLogout() {
- assumeTrue(loginStyle == LoginStyle.SESSION);
-
- jcrResourceProvider.logout(jcrProviderState);
-
- assertTrue(explicitSession.isLive());
- }
-
- @Test
- public void sessionsDoNotLeak() {
- // This test is only valid if we either didn't pass an explicit session,
- // or the provider had to clone it. Sessions created by the provider
- // must be closed by the provider, or we have a session leak.
- assumeThat(jcrProviderState.getSession(), is(not(sameInstance(explicitSession))));
-
- jcrResourceProvider.logout(jcrProviderState);
-
- assertFalse(jcrProviderState.getSession().isLive());
- }
-
- @Test
- public void impersonatorIsReportedCorrectly() {
- assumeTrue(useSudo);
-
- @SuppressWarnings("unchecked")
- ResolveContext<JcrProviderState> mockContext = mock(ResolveContext.class);
- when(mockContext.getProviderState()).thenReturn(jcrProviderState);
- Object reportedImpersonator = jcrResourceProvider.getAttribute(mockContext, ResourceResolver.USER_IMPERSONATOR);
-
- assertEquals(AUTH_USER, reportedImpersonator);
- }
-
- @Test
- public void clonesAreIndependent() {
- assumeTrue(loginStyle == LoginStyle.SESSION && doClone);
-
- assertNotSame(explicitSession, jcrProviderState.getSession());
- }
-
-}
diff --git a/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProviderTest.java b/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProviderTest.java
index 32c6bfb..aa40162 100644
--- a/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProviderTest.java
+++ b/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProviderTest.java
@@ -19,11 +19,18 @@
package org.apache.sling.jcr.resource.internal.helper.jcr;
import java.security.Principal;
+import java.util.HashMap;
+import java.util.Map;
import javax.jcr.Repository;
+import javax.jcr.RepositoryException;
import javax.jcr.Session;
+import javax.naming.NamingException;
+import org.apache.sling.api.resource.LoginException;
+import org.apache.sling.api.resource.ResourceResolverFactory;
import org.apache.sling.commons.testing.jcr.RepositoryTestBase;
+import org.apache.sling.jcr.resource.api.JcrResourceConstants;
import org.apache.sling.spi.resource.provider.ResolveContext;
import org.junit.Assert;
import org.mockito.Mockito;
@@ -40,23 +47,33 @@
super.setUp();
// create the session
session = getSession();
+ }
+
+ @Override
+ protected void tearDown() throws Exception {
+ super.tearDown();
+ }
+
+ public void testAdaptTo_Principal() {
+ jcrResourceProvider = new JcrResourceProvider();
+ ResolveContext ctx = Mockito.mock(ResolveContext.class);
+ Mockito.when(ctx.getProviderState()).thenReturn(new JcrProviderState(session, null, false));
+ Assert.assertNotNull(jcrResourceProvider.adaptTo(ctx, Principal.class));
+ }
+
+ public void testLeakOnSudo() throws LoginException, RepositoryException, NamingException {
Repository repo = getRepository();
ComponentContext ctx = Mockito.mock(ComponentContext.class);
Mockito.when(ctx.locateService(Mockito.anyString(), Mockito.any(ServiceReference.class))).thenReturn(repo);
jcrResourceProvider = new JcrResourceProvider();
jcrResourceProvider.activate(ctx);
- }
-
- @Override
- protected void tearDown() throws Exception {
- jcrResourceProvider.deactivate();
- super.tearDown();
- }
-
- public void testAdaptTo_Principal() {
- ResolveContext ctx = Mockito.mock(ResolveContext.class);
- Mockito.when(ctx.getProviderState()).thenReturn(new JcrProviderState(session, null, false));
- Assert.assertNotNull(jcrResourceProvider.adaptTo(ctx, Principal.class));
+ Map<String, Object> authInfo = new HashMap<String, Object>();
+ authInfo.put(JcrResourceConstants.AUTHENTICATION_INFO_SESSION, session);
+ authInfo.put(ResourceResolverFactory.USER_IMPERSONATION, "anonymous");
+ JcrProviderState providerState = jcrResourceProvider.authenticate(authInfo);
+ Assert.assertNotEquals("Impersonation didn't start new session", session, providerState.getSession());
+ jcrResourceProvider.logout(providerState);
+ assertFalse("Impersonated session wasn't closed.", providerState.getSession().isLive());
}
}
