This is an automated email from the ASF dual-hosted git repository.
rombert pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-resource.git
The following commit(s) were added to refs/heads/master by this push:
new 882a538 SLING-7805 - NPE in Oak SessionImpl when starting up
882a538 is described below
commit 882a53869c4918828c5cbd5aa5776340a950efde
Author: Robert Munteanu <[email protected]>
AuthorDate: Wed Aug 8 15:15:04 2018 +0300
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.
---
pom.xml | 3 +-
.../helper/jcr/JcrProviderStateFactory.java | 37 ++--
.../JcrResourceProviderSessionHandlingTest.java | 199 ---------------------
.../helper/jcr/JcrResourceProviderTest.java | 29 ++-
4 files changed, 35 insertions(+), 233 deletions(-)
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 @@ public class JcrProviderStateFactory {
@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 @@ public class JcrProviderStateFactory {
* @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,24 +47,34 @@ public class JcrResourceProviderTest extends
RepositoryTestBase {
super.setUp();
// create the session
session = getSession();
- 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() {
+ 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);
+ 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());
+ }
}
