Merge remote-tracking branch 'origin/master' into jetty-http2

jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java

@@ -820,7 +820,6 @@ public class SslConnection extends AbstractConnection
             }
             catch (Exception e)
             {
-                getEndPoint().close();
                 throw e;
             }
             finally

jetty-security/src/test/java/org/eclipse/jetty/security/AliasedConstraintTest.java

@@ -0,0 +1,175 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2014 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.security;
+
+import static org.hamcrest.Matchers.*;
+import static org.junit.Assert.*;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import org.eclipse.jetty.http.HttpStatus;
+import org.eclipse.jetty.server.Connector;
+import org.eclipse.jetty.server.LocalConnector;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.handler.AllowSymLinkAliasChecker;
+import org.eclipse.jetty.server.handler.ContextHandler;
+import org.eclipse.jetty.server.handler.ResourceHandler;
+import org.eclipse.jetty.server.session.SessionHandler;
+import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
+import org.eclipse.jetty.util.security.Constraint;
+import org.eclipse.jetty.util.security.Password;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+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;
+
+/**
+ * Some requests for static data that is served by ResourceHandler, but some is secured.
+ * <p>
+ * This is mainly here to test security bypass techniques using aliased names that should be caught.
+ */
+@RunWith(Parameterized.class)
+public class AliasedConstraintTest
+{
+    private static final String TEST_REALM = "TestRealm";
+    private static Server server;
+    private static LocalConnector connector;
+    private static ConstraintSecurityHandler security;
+
+    @BeforeClass
+    public static void startServer() throws Exception
+    {
+        server = new Server();
+        connector = new LocalConnector(server);
+        server.setConnectors(new Connector[] { connector });
+
+        ContextHandler context = new ContextHandler();
+        SessionHandler session = new SessionHandler();
+
+        HashLoginService loginService = new HashLoginService(TEST_REALM);
+        loginService.putUser("user0",new Password("password"),new String[] {});
+        loginService.putUser("user",new Password("password"),new String[] { "user" });
+        loginService.putUser("user2",new Password("password"),new String[] { "user" });
+        loginService.putUser("admin",new Password("password"),new String[] { "user", "administrator" });
+        loginService.putUser("user3",new Password("password"),new String[] { "foo" });
+
+        context.setContextPath("/ctx");
+        context.setResourceBase(MavenTestingUtils.getTestResourceDir("docroot").getAbsolutePath());
+        server.setHandler(context);
+        context.setHandler(session);
+        // context.addAliasCheck(new AllowSymLinkAliasChecker());
+
+        server.addBean(loginService);
+
+        security = new ConstraintSecurityHandler();
+        session.setHandler(security);
+        ResourceHandler handler = new ResourceHandler();
+        security.setHandler(handler);
+
+        List<ConstraintMapping> constraints = new ArrayList<>();
+
+        Constraint constraint0 = new Constraint();
+        constraint0.setAuthenticate(true);
+        constraint0.setName("forbid");
+        ConstraintMapping mapping0 = new ConstraintMapping();
+        mapping0.setPathSpec("/forbid/*");
+        mapping0.setConstraint(constraint0);
+        constraints.add(mapping0);
+
+        Set<String> knownRoles = new HashSet<>();
+        knownRoles.add("user");
+        knownRoles.add("administrator");
+
+        security.setConstraintMappings(constraints,knownRoles);
+        server.start();
+    }
+
+    @AfterClass
+    public static void stopServer() throws Exception
+    {
+        server.stop();
+    }
+
+    @Parameters(name = "{0}: {1}")
+    public static Collection<Object[]> data()
+    {
+        List<Object[]> data = new ArrayList<>();
+
+        final String OPENCONTENT = "this is open content";
+
+        data.add(new Object[] { "/ctx/all/index.txt", HttpStatus.OK_200, OPENCONTENT });
+        data.add(new Object[] { "/ctx/ALL/index.txt", HttpStatus.NOT_FOUND_404, null });
+        data.add(new Object[] { "/ctx/ALL/Fred/../index.txt", HttpStatus.NOT_FOUND_404, null });
+        data.add(new Object[] { "/ctx/../bar/../ctx/all/index.txt", HttpStatus.OK_200, OPENCONTENT });
+        data.add(new Object[] { "/ctx/forbid/index.txt", HttpStatus.FORBIDDEN_403, null });
+        data.add(new Object[] { "/ctx/all/../forbid/index.txt", HttpStatus.FORBIDDEN_403, null });
+        data.add(new Object[] { "/ctx/FoRbId/index.txt", HttpStatus.NOT_FOUND_404, null });
+
+        return data;
+    }
+
+    @Parameter(value = 0)
+    public String uri;
+
+    @Parameter(value = 1)
+    public int expectedStatusCode;
+
+    @Parameter(value = 2)
+    public String expectedContent;
+
+    @Test
+    public void testAccess() throws Exception
+    {
+        StringBuilder request = new StringBuilder();
+        request.append("GET ").append(uri).append(" HTTP/1.1\r\n");
+        request.append("Host: localhost\r\n");
+        request.append("Connection: close\r\n");
+        request.append("\r\n");
+
+        String response = connector.getResponses(request.toString());
+
+        switch (expectedStatusCode)
+        {
+            case 200:
+                assertThat(response,startsWith("HTTP/1.1 200 OK"));
+                break;
+            case 403:
+                assertThat(response,startsWith("HTTP/1.1 403 Forbidden"));
+                break;
+            case 404:
+                assertThat(response,startsWith("HTTP/1.1 404 Not Found"));
+                break;
+            default:
+                fail("Write a handler for response status code: " + expectedStatusCode);
+                break;
+        }
+
+        if (expectedContent != null)
+        {
+            assertThat(response,containsString("this is open content"));
+        }
+    }
+}

jetty-security/src/test/resources/docroot/all/index.txt

@@ -0,0 +1 @@
+this is open content.

jetty-security/src/test/resources/docroot/forbid/index.txt

@@ -0,0 +1 @@
+this is forbidden content.

jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java

@@ -421,11 +421,7 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
     @Override
     public void onClose()
     {
-        if (_sendCallback.isInUse())
+        _sendCallback.close();
-        {
-            LOG.warn("Closed with pending write:"+this);
-            _sendCallback.failed(new EofException("Connection closed"));
-        }
         super.onClose();
     }
 
@@ -480,6 +476,10 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
                 _header = null;
                 _shutdownOut = false;
             }
+            else if (isClosed())
+            {
+                callback.failed(new EofException());
+            }
             else
             {
                 callback.failed(new WritePendingException());

jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java

@@ -1648,10 +1648,15 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu
     }
 
     /* ------------------------------------------------------------ */
+    /**
+     * @param path
+     * @param resource
+     * @return True if the alias is OK
+     */
     public boolean checkAlias(String path, Resource resource)
     {
         // Is the resource aliased?
-            if (resource.getAlias() != null)
+        if (resource.getAlias() != null)
         {
             if (LOG.isDebugEnabled())
                 LOG.debug("Aliased resource: " + resource + "~=" + resource.getAlias());

jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java

@@ -308,15 +308,16 @@ public class ResourceHandler extends HandlerWrapper
         {
             if (_context==null)
                 return null;
-            base=_context.getBaseResource();
+            return _context.getResource(path);
-            if (base==null)
-                return null;
         }
 
         try
         {
             path=URIUtil.canonicalPath(path);
-            return base.addPath(path);
+            Resource r = base.addPath(path);
+            if (r!=null && r.getAlias()!=null && !_context.checkAlias(path, r))
+                return null;
+            return r;
         }
         catch(Exception e)
         {

jetty-start/src/main/java/org/eclipse/jetty/start/PathMatchers.java

@@ -96,15 +96,11 @@ public class PathMatchers
 
         // If the pattern starts with a root path then its assumed to
         // be a full system path
-        for (Path root : fs.getRootDirectories())
+        if (isAbsolute(pattern))
         {
-            StartLog.debug("root: " + root);
+            String pat = "glob:" + pattern;
-            if (pattern.startsWith(root.toString()))
+            StartLog.debug("Using absolute path pattern: " + pat);
-            {
+            return fs.getPathMatcher(pat);
-                String pat = "glob:" + pattern;
-                StartLog.debug("Using absolute path pattern: " + pat);
-                return fs.getPathMatcher(pat);
-            }
         }
 
         // Doesn't start with filesystem root, then assume the pattern

jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java

@@ -664,6 +664,8 @@ public class StartArgs
         {
             return;
         }
+        
+        StartLog.debug("parse(\"%s\", \"%s\", %b)",rawarg,source,replaceProps);
 
         final String arg = rawarg.trim();
 

jetty-start/src/test/java/org/eclipse/jetty/start/ConfigurationAssert.java

@@ -51,7 +51,7 @@ public class ConfigurationAssert
      */
     public static void assertConfiguration(BaseHome baseHome, StartArgs args, String filename) throws FileNotFoundException, IOException
     {
-        File testResourcesDir = MavenTestingUtils.getTestResourcesDir();
+        Path testResourcesDir = MavenTestingUtils.getTestResourcesDir().toPath().toAbsolutePath();
         File file = MavenTestingUtils.getTestResourceFile(filename);
         TextFile textFile = new TextFile(file.toPath());
 
@@ -149,12 +149,17 @@ public class ConfigurationAssert
         assertContainsUnordered("Files/Dirs",expectedFiles,actualFiles);
     }
 
-    private static String shorten(BaseHome baseHome, Path path, File testResourcesDir)
+    private static String shorten(BaseHome baseHome, Path path, Path testResourcesDir)
     {
         String value = baseHome.toShortForm(path);
-        if (value.startsWith(testResourcesDir.getAbsolutePath()))
+        if (value.startsWith("${"))
         {
-            int len = testResourcesDir.getAbsolutePath().length();
+            return value;
+        }
+
+        if (path.startsWith(testResourcesDir))
+        {
+            int len = testResourcesDir.toString().length();
             value = "${maven-test-resources}" + value.substring(len);
         }
         return value;

jetty-start/src/test/java/org/eclipse/jetty/start/MainTest.java

@@ -18,20 +18,27 @@
 
 package org.eclipse.jetty.start;
 
-import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.*;
+import static org.junit.Assert.*;
 
 import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.List;
 
 import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
-import org.junit.Assert;
+import org.eclipse.jetty.toolchain.test.TestTracker;
 import org.junit.Before;
 import org.junit.Ignore;
+import org.junit.Rule;
 import org.junit.Test;
 
 public class MainTest
 {
+    @Rule
+    public TestTracker ttracker = new TestTracker();
+    
     @Before
     public void clearSystemProperties()
     {
@@ -70,9 +77,9 @@ public class MainTest
         System.err.println(args);
 
         // Assert.assertEquals("--stop should not build module tree", 0, args.getEnabledModules().size());
-        Assert.assertEquals("--stop missing port","10000",args.getProperties().getString("STOP.PORT"));
+        assertEquals("--stop missing port","10000",args.getProperties().getString("STOP.PORT"));
-        Assert.assertEquals("--stop missing key","foo",args.getProperties().getString("STOP.KEY"));
+        assertEquals("--stop missing key","foo",args.getProperties().getString("STOP.KEY"));
-        Assert.assertEquals("--stop missing wait","300",args.getProperties().getString("STOP.WAIT"));
+        assertEquals("--stop missing wait","300",args.getProperties().getString("STOP.WAIT"));
     }
 
     @Test
@@ -114,9 +121,22 @@ public class MainTest
         cmdLineArgs.add("-Xmx1024m");
 
         // Arbitrary Libs
-        File extraJar = MavenTestingUtils.getTestResourceFile("extra-libs/example.jar");
+        Path extraJar = MavenTestingUtils.getTestResourceFile("extra-libs/example.jar").toPath().normalize();
-        File extraDir = MavenTestingUtils.getTestResourceDir("extra-resources");
+        Path extraDir = MavenTestingUtils.getTestResourceDir("extra-resources").toPath().normalize();
-        cmdLineArgs.add(String.format("--lib=%s%s%s",extraJar.getAbsolutePath(),File.pathSeparatorChar,extraDir.getAbsolutePath()));
+        
+        extraJar = extraJar.toAbsolutePath();
+        extraDir = extraDir.toAbsolutePath();
+        
+        assertThat("Extra Jar exists: " + extraJar,Files.exists(extraJar),is(true));
+        assertThat("Extra Dir exists: " + extraDir,Files.exists(extraDir),is(true));
+        
+        StringBuilder lib = new StringBuilder();
+        lib.append("--lib=");
+        lib.append(extraJar.toString());
+        lib.append(File.pathSeparator);
+        lib.append(extraDir.toString());
+        
+        cmdLineArgs.add(lib.toString());
 
         // Arbitrary XMLs
         cmdLineArgs.add("jetty.xml");
@@ -128,8 +148,8 @@ public class MainTest
         StartArgs args = main.processCommandLine(cmdLineArgs.toArray(new String[cmdLineArgs.size()]));
         BaseHome baseHome = main.getBaseHome();
 
-        Assert.assertThat("jetty.home",baseHome.getHome(),is(homePath.getAbsolutePath()));
+        assertThat("jetty.home",baseHome.getHome(),is(homePath.getAbsolutePath()));
-        Assert.assertThat("jetty.base",baseHome.getBase(),is(homePath.getAbsolutePath()));
+        assertThat("jetty.base",baseHome.getBase(),is(homePath.getAbsolutePath()));
 
         ConfigurationAssert.assertConfiguration(baseHome,args,"assert-home-with-jvm.txt");
     }
@@ -154,8 +174,8 @@ public class MainTest
         StartArgs args = main.processCommandLine(cmdLineArgs.toArray(new String[cmdLineArgs.size()]));
         BaseHome baseHome = main.getBaseHome();
 
-        Assert.assertThat("jetty.home",baseHome.getHome(),is(homePath.getAbsolutePath()));
+        assertThat("jetty.home",baseHome.getHome(),is(homePath.getAbsolutePath()));
-        Assert.assertThat("jetty.base",baseHome.getBase(),is(homePath.getAbsolutePath()));
+        assertThat("jetty.base",baseHome.getBase(),is(homePath.getAbsolutePath()));
 
         ConfigurationAssert.assertConfiguration(baseHome,args,"assert-home-with-spdy.txt");
     }
@@ -173,8 +193,8 @@ public class MainTest
         StartArgs args = main.processCommandLine(cmdLineArgs.toArray(new String[cmdLineArgs.size()]));
         BaseHome baseHome = main.getBaseHome();
 
-        Assert.assertThat("jetty.home",baseHome.getHome(),is(homePath.getAbsolutePath()));
+        assertThat("jetty.home",baseHome.getHome(),is(homePath.getAbsolutePath()));
-        Assert.assertThat("jetty.base",baseHome.getBase(),is(homePath.getAbsolutePath()));
+        assertThat("jetty.base",baseHome.getBase(),is(homePath.getAbsolutePath()));
 
         ConfigurationAssert.assertConfiguration(baseHome,args,"assert-home-with-spaces.txt");
     }

jetty-util/src/main/java/org/eclipse/jetty/util/IteratingCallback.java

@@ -18,6 +18,7 @@
 
 package org.eclipse.jetty.util;
 
+import java.io.EOFException;
 import java.util.concurrent.atomic.AtomicReference;
 
 /**
@@ -50,6 +51,46 @@ import java.util.concurrent.atomic.AtomicReference;
  */
 public abstract class IteratingCallback implements Callback
 {
+
+    /**
+     * The internal states of this callback
+     */
+    private enum State
+    {
+        /**
+         * This callback is inactive, ready to iterate.
+         */
+        INACTIVE,
+        /**
+         * This callback is iterating and {@link #process()} has scheduled an
+         * asynchronous operation by returning {@link Action#SCHEDULED}, but
+         * the operation is still undergoing.
+         */
+        ACTIVE,
+        /**
+         * This callback is iterating and {@link #process()} has been called
+         * but not returned yet.
+         */
+        ITERATING,
+        /**
+         * While this callback was iterating, another request for iteration
+         * has been issued, so the iteration must continue even if a previous
+         * call to {@link #process()} returned {@link Action#IDLE}.
+         */
+        ITERATE_AGAIN,
+        /**
+         * The overall job has succeeded.
+         */
+        SUCCEEDED,
+        /**
+         * The overall job has failed.
+         */
+        FAILED,
+        /**
+         * The ICB has been closed and cannot be reset
+         */ 
+        CLOSED
+    }
     /**
      * The indication of the overall progress of the overall job that
      * implementations of {@link #process()} must return.
@@ -211,8 +252,25 @@ public abstract class IteratingCallback implements Callback
                 case SUCCEEDED:
                 {
                     // The overall job has completed.
-                    completeSuccess();
+                    while (true)
-                    return true;
+                    {
+                        State current = _state.get();
+                        switch(current)
+                        {
+                            case SUCCEEDED:
+                            case FAILED:
+                                // Already complete!.
+                                return true;
+                            case CLOSED:
+                                throw new IllegalStateException();
+                            default:
+                                if (_state.compareAndSet(current, State.SUCCEEDED))
+                                {
+                                    onCompleteSuccess();
+                                    return true;
+                                }
+                        }
+                    }
                 }
                 default:
                 {
@@ -260,6 +318,10 @@ public abstract class IteratingCallback implements Callback
                     iterate();
                     return;
                 }
+                case CLOSED:
+                    // too late!
+                    return;
+                    
                 default:
                 {
                     throw new IllegalStateException(toString());
@@ -275,32 +337,6 @@ public abstract class IteratingCallback implements Callback
      */
     @Override
     public final void failed(Throwable x)
-    {
-        completeFailure(x);
-    }
-
-    protected void completeSuccess()
-    {
-        while (true)
-        {
-            State current = _state.get();
-            switch(current)
-            {
-                case SUCCEEDED:
-                case FAILED:
-                    // Already complete!.
-                    return;
-                default:
-                    if (_state.compareAndSet(current, State.SUCCEEDED))
-                    {
-                        onCompleteSuccess();
-                        return;
-                    }
-            }
-        }
-    }
-
-    protected void completeFailure(Throwable x)
     {
         while (true)
         {
@@ -309,8 +345,11 @@ public abstract class IteratingCallback implements Callback
             {
                 case SUCCEEDED:
                 case FAILED:
+                case INACTIVE:
+                case CLOSED:
                     // Already complete!.
                     return;
+                    
                 default:
                     if (_state.compareAndSet(current, State.FAILED))
                     {
@@ -321,31 +360,43 @@ public abstract class IteratingCallback implements Callback
         }
     }
 
-    /**
+    public final void close()
-     * @return whether this callback is idle and {@link #iterate()} needs to be called
-     */
-    public boolean isIdle()
     {
-        return _state.get() == State.INACTIVE;
+        while (true)
-    }
-    
-    /* ------------------------------------------------------------ */
-    /**
-     * @return true if the callback is not INACTIVE, FAILED or SUCCEEDED.
-     */
-    public boolean isInUse()
-    {
-        switch(_state.get())
         {
-            case INACTIVE:
+            State current = _state.get();
-            case FAILED:
+            switch(current)
-            case SUCCEEDED:
+            {
-                return false;
+                case INACTIVE:
-            default:
+                case SUCCEEDED:
-                return true;
+                case FAILED:
+                    if (_state.compareAndSet(current, State.CLOSED))
+                        return;
+                    break;
+                default:
+                    if (_state.compareAndSet(current, State.CLOSED))
+                    {
+                        onCompleteFailure(new IllegalStateException("Closed with pending callback "+current));
+                        return;
+                    }
+            }
         }
     }
 
+    /*
+     * only for testing
+     * @return whether this callback is idle and {@link #iterate()} needs to be called
+     */
+    boolean isIdle()
+    {
+        return _state.get() == State.INACTIVE;
+    }
+
+    public boolean isClosed()
+    {
+        return _state.get() == State.CLOSED;
+    }
+    
     /**
      * @return whether this callback has failed
      */
@@ -397,40 +448,4 @@ public abstract class IteratingCallback implements Callback
     {
         return String.format("%s[%s]", super.toString(), _state);
     }
-    
-    /**
-     * The internal states of this callback
-     */
-    private enum State
-    {
-        /**
-         * This callback is inactive, ready to iterate.
-         */
-        INACTIVE,
-        /**
-         * This callback is iterating and {@link #process()} has scheduled an
-         * asynchronous operation by returning {@link Action#SCHEDULED}, but
-         * the operation is still undergoing.
-         */
-        ACTIVE,
-        /**
-         * This callback is iterating and {@link #process()} has been called
-         * but not returned yet.
-         */
-        ITERATING,
-        /**
-         * While this callback was iterating, another request for iteration
-         * has been issued, so the iteration must continue even if a previous
-         * call to {@link #process()} returned {@link Action#IDLE}.
-         */
-        ITERATE_AGAIN,
-        /**
-         * The overall job has succeeded.
-         */
-        SUCCEEDED,
-        /**
-         * The overall job has failed.
-         */
-        FAILED
-    }
 }

jetty-util/src/test/java/org/eclipse/jetty/util/resource/AbstractFSResourceTest.java

@@ -83,12 +83,18 @@ public abstract class AbstractFSResourceTest
     {
         if (OS.IS_UNIX)
         {
+            // A windows path is invalid under unix
             newResource(new URI("file://Z:/:"));
         }
         else if (OS.IS_WINDOWS)
         {
-            newResource(new URI("file://some\"text\""));
+            // "CON" is a reserved name under windows
+            newResource(new URI("file://CON"));
         }
+        else
+        {
+            assumeFalse("Unknown OS type",false);
+        }   
     }
 
     @Test