JCR-5255: Migrate to Apache HttpClient 5

jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authentication/token/TokenProvider.java

@@ -31,6 +31,8 @@
 import java.util.Map;
 import java.util.Set;
 import javax.jcr.AccessDeniedException;
+import javax.jcr.InvalidItemStateException;
+import javax.jcr.ItemNotFoundException;
 import javax.jcr.NamespaceRegistry;
 import javax.jcr.Node;
 import javax.jcr.Property;
@@ -76,6 +78,13 @@ public class TokenProvider extends ProtectedItemModifier {
 
     private static final char DELIM = '_';
 
+    /**
+     * Number of attempts to persist a new token node before giving up. Concurrent logins
+     * of the same user write below a shared token parent and may invalidate each other's
+     * pending changes (JCR-5095).
+     */
+    private static final int CREATE_TOKEN_MAX_ATTEMPTS = 3;
+
     private static final Set<String> RESERVED_ATTRIBUTES = new HashSet(3);
     static {
         RESERVED_ATTRIBUTES.add(TOKEN_ATTRIBUTE);
@@ -146,6 +155,39 @@ public TokenInfo createToken(User user, SimpleCredentials sc) throws RepositoryE
      */
     private TokenInfo createToken(User user, Map<String, ?> attributes) throws RepositoryException {
         String error = "Failed to create login token. ";
+        // Concurrent logins of the same user add token nodes below the very same token
+        // parent. A concurrent commit below that parent may invalidate the pending changes
+        // of this session, so that the token node can neither be saved
+        // (InvalidItemStateException) nor resolved afterwards (ItemNotFoundException while
+        // building its path). Both are transient, so retry with a refreshed session,
+        // analogous to the conflict handling in getTokenParent (JCR-5095).
+        for (int attempt = 1; ; attempt++) {
+            try {
+                return createTokenNode(user, attributes, error);
+            } catch (InvalidItemStateException | ItemNotFoundException e) {
+                if (attempt >= CREATE_TOKEN_MAX_ATTEMPTS) {
+                    throw e;
+                }
+                log.debug("Conflict while creating login token (attempt {}) -> retrying", attempt, e);
+                // discard the token node that could not be persisted before retrying
+                session.refresh(false);
+            }
+        }
+    }
+
+    /**
+     * Creates and persists a single token node below the token parent of the given user.
+     *
+     * @param user       The user for which a new token should be created.
+     * @param attributes The attributes associated with the new token.
+     * @param error      Prefix used for log messages.
+     * @return A new {@code TokenInfo} or {@code null} if the token could not be created.
+     * @throws InvalidItemStateException If the token node could not be persisted because
+     *                                   the token parent was modified concurrently.
+     * @throws ItemNotFoundException If the token node could not be resolved after saving
+     *                               because the token parent was modified concurrently.
+     */
+    private TokenInfo createTokenNode(User user, Map<String, ?> attributes, String error) throws RepositoryException {
         NodeImpl tokenParent = getTokenParent(user);
         if (tokenParent != null) {
             try {

jackrabbit-it-osgi/pom.xml

@@ -71,15 +71,13 @@
       <scope>test</scope>
     </dependency>
     <dependency>
-      <groupId>org.apache.httpcomponents</groupId>
-      <artifactId>httpcore-osgi</artifactId>
-      <version>4.4.16</version>
+      <groupId>org.apache.httpcomponents.core5</groupId>
+      <artifactId>httpcore5</artifactId>
       <scope>test</scope>
     </dependency>
     <dependency>
-      <groupId>org.apache.httpcomponents</groupId>
-      <artifactId>httpclient-osgi</artifactId>
-      <version>4.5.14</version>
+      <groupId>org.apache.httpcomponents.client5</groupId>
+      <artifactId>httpclient5</artifactId>
       <scope>test</scope>
     </dependency>
     <dependency>

jackrabbit-it-osgi/src/test/java/org/apache/jackrabbit/osgi/OSGiIT.java

@@ -17,7 +17,6 @@
 package org.apache.jackrabbit.osgi;
 
 import static org.junit.Assert.assertEquals;
-import static org.ops4j.pax.exam.CoreOptions.bundle;
 import static org.ops4j.pax.exam.CoreOptions.frameworkProperty;
 import static org.ops4j.pax.exam.CoreOptions.junitBundles;
 import static org.ops4j.pax.exam.CoreOptions.mavenBundle;
@@ -28,7 +27,6 @@
 import java.io.File;
 import java.io.IOException;
 import java.net.URI;
-import java.net.MalformedURLException;
 import java.net.URISyntaxException;
 
 import javax.inject.Inject;
@@ -103,14 +101,8 @@ private String getConfigDir(){
         return new File(new File("src", "test"), "config").getAbsolutePath();
     }
 
-    private Option jarBundles() throws MalformedURLException {
-        DefaultCompositeOption composite = new DefaultCompositeOption();
-        for (File bundle : new File("target", "test-bundles").listFiles()) {
-            if (bundle.getName().endsWith(".jar") && bundle.isFile()) {
-                composite.add(bundle(bundle.toURI().toURL().toString()));
-            }
-        }
-        return composite;
+    private Option jarBundles() throws IOException {
+        return TestBundles.jarBundles();
     }
 
     @Inject

jackrabbit-it-osgi/src/test/java/org/apache/jackrabbit/osgi/TestBundles.java

@@ -0,0 +1,149 @@
+/*
+ * 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.jackrabbit.osgi;
+
+import static org.ops4j.pax.exam.CoreOptions.bundle;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.util.Enumeration;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.jar.Attributes;
+import java.util.jar.JarEntry;
+import java.util.jar.JarFile;
+import java.util.jar.JarOutputStream;
+import java.util.jar.Manifest;
+
+import org.ops4j.pax.exam.Option;
+import org.ops4j.pax.exam.options.DefaultCompositeOption;
+
+/**
+ * Provisions the JARs assembled into {@code target/test-bundles} for the OSGi tests.
+ * <p>
+ * Anything that is not already a bundle is repacked as one. HttpComponents publishes no
+ * OSGi bundles for the 5.x line: {@code httpclient5-osgi} and {@code httpcore5-osgi} stop
+ * at 5.0-beta, and the plain 5.x JARs carry no {@code Bundle-SymbolicName}, so without
+ * this the bundles depending on them cannot resolve. This is what the {@code wrap:} URL
+ * handler does; provisioning {@code wrap:} URLs was tried first and left the Pax Exam
+ * native container unable to start, so the repack is done here instead.
+ */
+public final class TestBundles {
+
+    private TestBundles() {
+    }
+
+    public static Option jarBundles() throws IOException {
+        DefaultCompositeOption composite = new DefaultCompositeOption();
+        for (File jar : new File("target", "test-bundles").listFiles()) {
+            if (jar.getName().endsWith(".jar") && jar.isFile()) {
+                File installable = isBundle(jar) ? jar : asBundle(jar);
+                composite.add(bundle(installable.toURI().toURL().toString()));
+            }
+        }
+        return composite;
+    }
+
+    private static boolean isBundle(File jar) throws IOException {
+        try (JarFile jarFile = new JarFile(jar)) {
+            Manifest manifest = jarFile.getManifest();
+            return manifest != null
+                    && manifest.getMainAttributes().getValue("Bundle-SymbolicName") != null;
+        }
+    }
+
+    /**
+     * Repacks a plain JAR as an OSGi bundle exporting every package it contains and
+     * resolving its own dependencies dynamically.
+     */
+    private static File asBundle(File jar) throws IOException {
+        File dir = new File("target", "wrapped-bundles");
+        dir.mkdirs();
+        File wrapped = new File(dir, jar.getName());
+
+        try (JarFile in = new JarFile(jar)) {
+            Set<String> packages = new TreeSet<String>();
+            for (Enumeration<JarEntry> e = in.entries(); e.hasMoreElements();) {
+                String name = e.nextElement().getName();
+                int slash = name.lastIndexOf('/');
+                if (name.endsWith(".class") && slash > 0) {
+                    packages.add(name.substring(0, slash).replace('/', '.'));
+                }
+            }
+
+            String symbolicName = jar.getName().substring(0, jar.getName().length() - ".jar".length());
+            Manifest manifest = new Manifest();
+            Attributes main = manifest.getMainAttributes();
+            main.put(Attributes.Name.MANIFEST_VERSION, "1.0");
+            main.putValue("Bundle-ManifestVersion", "2");
+            main.putValue("Bundle-SymbolicName", symbolicName);
+            main.putValue("Bundle-Version", bundleVersion(in));
+            main.putValue("Export-Package", String.join(",", packages));
+            main.putValue("DynamicImport-Package", "*");
+
+            byte[] buffer = new byte[8192];
+            try (JarOutputStream out = new JarOutputStream(new FileOutputStream(wrapped), manifest)) {
+                for (Enumeration<JarEntry> e = in.entries(); e.hasMoreElements();) {
+                    JarEntry entry = e.nextElement();
+                    if (entry.isDirectory() || entry.getName().equals(JarFile.MANIFEST_NAME)) {
+                        continue;
+                    }
+                    out.putNextEntry(new JarEntry(entry.getName()));
+                    try (InputStream content = in.getInputStream(entry)) {
+                        copy(content, out, buffer);
+                    }
+                    out.closeEntry();
+                }
+            }
+        }
+        return wrapped;
+    }
+
+    /**
+     * Derives an OSGi-legal Bundle-Version from the JAR's Implementation-Version. The
+     * assembly renames JARs to {@code <artifactId>.jar}, so the file name carries no
+     * version.
+     */
+    private static String bundleVersion(JarFile jar) throws IOException {
+        Manifest manifest = jar.getManifest();
+        String version = manifest == null
+                ? null : manifest.getMainAttributes().getValue("Implementation-Version");
+        if (version == null) {
+            return "0.0.0";
+        }
+        // OSGi wants major.minor.micro; anything after the third segment is a qualifier
+        String[] parts = version.split("[.-]");
+        StringBuilder result = new StringBuilder();
+        for (int i = 0; i < 3; i++) {
+            result.append(i < parts.length && parts[i].matches("\\d+") ? parts[i] : "0");
+            if (i < 2) {
+                result.append('.');
+            }
+        }
+        return result.toString();
+    }
+
+    private static void copy(InputStream in, OutputStream out, byte[] buffer) throws IOException {
+        int read;
+        while ((read = in.read(buffer)) != -1) {
+            out.write(buffer, 0, read);
+        }
+    }
+}

jackrabbit-it-osgi/src/test/java/org/apache/jackrabbit/osgi/slf4j2/Slf4j_v2_Tika_v2_4_OSGiIT.java

@@ -17,7 +17,6 @@
 package org.apache.jackrabbit.osgi.slf4j2;
 
 import static org.junit.Assert.assertEquals;
-import static org.ops4j.pax.exam.CoreOptions.bundle;
 import static org.ops4j.pax.exam.CoreOptions.frameworkProperty;
 import static org.ops4j.pax.exam.CoreOptions.junitBundles;
 import static org.ops4j.pax.exam.CoreOptions.mavenBundle;
@@ -115,14 +114,9 @@ private String getConfigDir(){
         return new File(new File("src", "test"), "config").getAbsolutePath();
     }
 
-    private Option jarBundles() throws MalformedURLException {
-        DefaultCompositeOption composite = new DefaultCompositeOption();
-        for (File bundle : new File("target", "test-bundles").listFiles()) {
-            if (bundle.getName().endsWith(".jar") && bundle.isFile()) {
-                composite.add(bundle(bundle.toURI().toURL().toString()));
-            }
-        }
-        return composite;
+    private Option jarBundles() throws IOException {
+        // shared with OSGiIT: repacks the HttpClient 5 JARs, which are not bundles
+        return org.apache.jackrabbit.osgi.TestBundles.jarBundles();
     }
 
     @Inject

jackrabbit-it-osgi/src/test/java/org/apache/jackrabbit/osgi/slf4j2/Slf4j_v2_Tika_v2_9_OSGiIT.java

@@ -17,7 +17,6 @@
 package org.apache.jackrabbit.osgi.slf4j2;
 
 import static org.junit.Assert.assertEquals;
-import static org.ops4j.pax.exam.CoreOptions.bundle;
 import static org.ops4j.pax.exam.CoreOptions.frameworkProperty;
 import static org.ops4j.pax.exam.CoreOptions.junitBundles;
 import static org.ops4j.pax.exam.CoreOptions.mavenBundle;
@@ -115,14 +114,9 @@ private String getConfigDir(){
         return new File(new File("src", "test"), "config").getAbsolutePath();
     }
 
-    private Option jarBundles() throws MalformedURLException {
-        DefaultCompositeOption composite = new DefaultCompositeOption();
-        for (File bundle : new File("target", "test-bundles").listFiles()) {
-            if (bundle.getName().endsWith(".jar") && bundle.isFile()) {
-                composite.add(bundle(bundle.toURI().toURL().toString()));
-            }
-        }
-        return composite;
+    private Option jarBundles() throws IOException {
+        // shared with OSGiIT: repacks the HttpClient 5 JARs, which are not bundles
+        return org.apache.jackrabbit.osgi.TestBundles.jarBundles();
     }
 
     @Inject

jackrabbit-it-osgi/test-bundles.xml

@@ -36,8 +36,8 @@
         <include>org.apache.jackrabbit:jackrabbit-jcr-commons</include>
         <include>org.apache.jackrabbit:jackrabbit-spi</include>
         <include>org.apache.jackrabbit:jackrabbit-spi-commons</include>
-        <include>org.apache.httpcomponents:httpclient-osgi</include>
-        <include>org.apache.httpcomponents:httpcore-osgi</include>
+        <include>org.apache.httpcomponents.client5:httpclient5</include>
+        <include>org.apache.httpcomponents.core5:httpcore5</include>
         <include>org.apache.jackrabbit:jackrabbit-webdav</include>
         <include>org.apache.jackrabbit:jackrabbit-jcr-server</include>
       </includes>