Some cleanup and more tests.

ninja-core/src/main/java/ninja/AssetsController.java

@@ -60,11 +60,15 @@ public class AssetsController {
     private final HttpCacheToolkit httpCacheToolkit;
 
     private final NinjaProperties ninjaProperties;
+
+    private final AssetsControllerHelper assetsControllerHelper;
 
     @Inject
-    public AssetsController(HttpCacheToolkit httpCacheToolkit,
+    public AssetsController(AssetsControllerHelper assetsControllerHelper,
+                            HttpCacheToolkit httpCacheToolkit,
                             MimeTypes mimeTypes,
                             NinjaProperties ninjaProperties) {
+        this.assetsControllerHelper = assetsControllerHelper;
         this.httpCacheToolkit = httpCacheToolkit;
         this.mimeTypes = mimeTypes;
         this.ninjaProperties = ninjaProperties;
@@ -86,7 +90,7 @@ public AssetsController(HttpCacheToolkit httpCacheToolkit,
      * /assets/app/app.css (from your jar).
      * 
      */
-    public Result serveStatic(Context context) {
+    public Result serveStatic() {
         Object renderable = new Renderable() {
             @Override
             public void render(Context context, Result result) {
@@ -107,7 +111,7 @@ public void render(Context context, Result result) {
      * Request to /public/css/app.css will be served from /assets/css/app.css.
      * 
      */
-    public Result serveWebJars(Context context) {
+    public Result serveWebJars() {
         Object renderable = new Renderable() {
             @Override
             public void render(Context context, Result result) {
@@ -147,14 +151,12 @@ private void streamOutUrlEntity(URL url, Context context, Result result) {
                             .finalizeHeadersWithoutFlashAndSessionCookie(result);
 
                     try (InputStream inputStream = urlConnection.getInputStream();
-                         OutputStream outputStream = responseStreams.getOutputStream()) {
+                        OutputStream outputStream = responseStreams.getOutputStream()) {
                         ByteStreams.copy(inputStream, outputStream);
                     }
 
                 }
 
-            } catch (FileNotFoundException e) {
-                logger.error("error streaming file", e);
             } catch (IOException e) {
                 logger.error("error streaming file", e);
             }
@@ -170,22 +172,18 @@ private void streamOutUrlEntity(URL url, Context context, Result result) {
      * be overridden by static.asset.base.dir in application conf file.
      */
     private URL getStaticFileFromAssetsDir(String fileName) {
-
-        String finalNameWithoutLeadingSlash = normalizePathWithoutLeadingSlash(fileName);
 
         URL url = null;
 
-         if (ninjaProperties.isDev()) {
+        if (ninjaProperties.isDev()) {
+            String finalNameWithoutLeadingSlash = assetsControllerHelper.normalizePathWithoutLeadingSlash(fileName, false);
             File possibleFile = new File(
                     assetsDirInDevModeWithoutTrailingSlash() 
                             + File.separator 
                             + finalNameWithoutLeadingSlash);
             url = getUrlForFile(possibleFile);
         } else {
-            // In mode test and prod, if static.asset.base.dir not specified then we stream via the classloader.
-            //
-            // In dev mode: If we cannot find the file in src we are also looking for the file
-            // on the classpath (can be the case for plugins that ship their own assets.
+            String finalNameWithoutLeadingSlash = assetsControllerHelper.normalizePathWithoutLeadingSlash(fileName, true);
             url = this.getClass().getClassLoader()
                     .getResource(ASSETS_DIR
                                  + "/"
@@ -214,52 +212,12 @@ private URL getUrlForFile(File possibleFileInSrc) {
      */
     private URL getStaticFileFromMetaInfResourcesDir(String fileName) {
         String finalNameWithoutLeadingSlash 
-                = normalizePathWithoutLeadingSlash(fileName, true);
-
-        URL url = null;
-
+                = assetsControllerHelper.normalizePathWithoutLeadingSlash(fileName, true);
+        URL url = null; 
         url = this.getClass().getClassLoader().getResource("META-INF/resources/webjars/" + finalNameWithoutLeadingSlash);
-
         return url;
     }
 
-    /**
-     * This function mirrors legacy behavior of the AssetsController
-     *
-     * @param fileName A potential "fileName"
-     * @see #normalizePathWithoutLeadingSlash(java.lang.String, boolean)
-     * @deprecated This method was replaced and should not be used.
-     * @return A normalized fileName.
-     */
-    @VisibleForTesting
-    @Deprecated
-    protected String normalizePathWithoutLeadingSlash(String fileName) {
-        return normalizePathWithoutLeadingSlash(fileName, false);
-    }
-
-    /**
-     * If we get - for whatever reason - a relative URL like 
-     * assets/../conf/application.conf we expand that to the "real" path.
-     * In the above case conf/application.conf.
-     * 
-     * You should then add the assets prefix.
-     * 
-     * Otherwise someone can create an attack and read all resources of our
-     * app. If we expand and normalize the incoming path this is no longer
-     * possible.
-     * 
-     * @param fileName A potential "fileName"
-     * @param enforceUnixSeparator Forces to normalise unix style
-     * @return A normalized fileName.
-     */
-    @VisibleForTesting
-    protected String normalizePathWithoutLeadingSlash(String fileName, boolean enforceUnixSeparator) {
-        String fileNameNormalized = enforceUnixSeparator
-                ? FilenameUtils.normalize(fileName, true)
-                : FilenameUtils.normalize(fileName);
-        return StringUtils.removeStart(fileNameNormalized, "/");
-    }
-
     private static String getFileNameFromPathOrReturnRequestPath(Context context) {
 
         String fileName = context.getPathParameter(FILENAME_PATH_PARAM);

ninja-core/src/main/java/ninja/AssetsControllerHelper.java

@@ -0,0 +1,51 @@
+/**
+ * Copyright (C) 2012-2015 the original author or authors.
+ *
+ * Licensed 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 ninja;
+
+import org.apache.commons.io.FilenameUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.commons.lang.StringUtils;
+
+public class AssetsControllerHelper {
+
+    private final static Logger logger = LoggerFactory
+            .getLogger(AssetsControllerHelper.class);
+
+    /**
+     * If we get - for whatever reason - a relative URL like
+     * assets/../conf/application.conf we expand that to the "real" path. In the
+     * above case conf/application.conf.
+     *
+     * You should then add the assets prefix.
+     *
+     * Otherwise someone can create an attack and read all resources of our app.
+     * If we expand and normalize the incoming path this is no longer possible.
+     *
+     * @param fileName A potential "fileName"
+     * @param enforceUnixSeparator If true it will force the usage of the unix separator '/'
+     *                             If false it will use the separator of the underlying system.
+     *                             usually '/' in case of unix and '\' in case of windows.
+     * @return A normalized fileName.
+     */
+    public String normalizePathWithoutLeadingSlash(String fileName, boolean enforceUnixSeparator) {
+        String fileNameNormalized = enforceUnixSeparator
+                ? FilenameUtils.normalize(fileName, true)
+                : FilenameUtils.normalize(fileName);
+        return StringUtils.removeStart(fileNameNormalized, "/");
+    }
+}

ninja-core/src/site/markdown/developer/changelog.md

@@ -1,3 +1,10 @@
+Version X.X.X
+=============
+
+ * 2015-06-30 Fix for asset controller incompatibility with windows file system (BjoernAkAManf).
+ * 2015-06-30 Improved tests in asset controller for serving webjars (ra).
+
+
 Version 5.1.3
 =============
 

ninja-core/src/test/java/ninja/AssetsControllerHelperTest.java

@@ -0,0 +1,64 @@
+/**
+ * Copyright (C) 2012-2015 the original author or authors.
+ *
+ * Licensed 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 ninja;
+
+import static org.junit.Assert.assertEquals;
+import static org.powermock.api.mockito.PowerMockito.mockStatic;
+import org.apache.commons.io.FilenameUtils;
+import org.junit.Before;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mockito;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+
+@RunWith(PowerMockRunner.class)
+@PrepareForTest(FilenameUtils.class)
+public class AssetsControllerHelperTest {
+
+    AssetsControllerHelper assetsControllerHelper;
+
+    @Before
+    public void setup() {
+        assetsControllerHelper = new AssetsControllerHelper();
+    }
+
+    @Test
+    public void testNormalizePathWithoutLeadingSlash() {
+        assertEquals("dir1/test.test", assetsControllerHelper.normalizePathWithoutLeadingSlash("/dir1/test.test", true));
+        assertEquals("dir1/test.test", assetsControllerHelper.normalizePathWithoutLeadingSlash("dir1/test.test", true));
+        assertEquals(null, assetsControllerHelper.normalizePathWithoutLeadingSlash("/../test.test", true));
+        assertEquals(null, assetsControllerHelper.normalizePathWithoutLeadingSlash("../test.test", true));
+        assertEquals("dir2/file.test", assetsControllerHelper.normalizePathWithoutLeadingSlash("/dir1/../dir2/file.test", true));
+        assertEquals(null, assetsControllerHelper.normalizePathWithoutLeadingSlash(null, true));
+        assertEquals("", assetsControllerHelper.normalizePathWithoutLeadingSlash("", true));
+    }
+
+    @Test
+    public void testNormalizePathWithoutLeadingSlashCorrectFilnameUtilStaticMethodsCalled() {
+        PowerMockito.mockStatic(FilenameUtils.class, Mockito.CALLS_REAL_METHODS);
+
+        assetsControllerHelper.normalizePathWithoutLeadingSlash("/dir1/test.test", false);
+        PowerMockito.verifyStatic();
+        FilenameUtils.normalize("/dir1/test.test");
+
+        assetsControllerHelper.normalizePathWithoutLeadingSlash("/dir1/test.test", true);
+        PowerMockito.verifyStatic();
+        FilenameUtils.normalize("/dir1/test.test", true);
+    }
+}