Merge pull request #598 from jooby-project/585

Assets from file system location fix #585

Author: jknack

coverage-report/src/test/java/org/jooby/issues/Issue585.java

@@ -0,0 +1,33 @@
+package org.jooby.issues;
+
+import java.nio.file.Paths;
+
+import org.jooby.test.ServerFeature;
+import org.junit.Test;
+
+public class Issue585 extends ServerFeature {
+
+  {
+    assets("/static/**", Paths.get("src/test/resources/static"));
+  }
+
+  @Test
+  public void shouldServeStaticFiles() throws Exception {
+    request()
+        .get("/static/images/fun.gif")
+        .expect(200)
+        .header("Content-Length", "79530");
+
+    request()
+        .get("/static/images/prey.jpg")
+        .expect(200)
+        .header("Content-Length", "39003");
+  }
+
+  @Test
+  public void shouldNotAllowAccessToResourceOutsideScope() throws Exception {
+    request()
+        .get("/static/../forbidden.txt")
+        .expect(404);
+  }
+}

coverage-report/src/test/resources/forbidden.txt

@@ -58,6 +58,7 @@
 import java.io.File;
 import java.lang.reflect.Type;
 import java.nio.charset.Charset;
+import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.text.DecimalFormat;
 import java.text.NumberFormat;
@@ -1727,21 +1728,16 @@ private Route.Filter filter(final Class<? extends Route.Filter> filter) {
   }
 
   @Override
-  public Route.Definition assets(final String path) {
-    return assets(path, "/");
+  public Definition assets(final String path, final Path basedir) {
+    AssetHandler handler = new AssetHandler(basedir);
+    configureAssetHandler(handler);
+    return assets(path, handler);
   }
 
   @Override
   public Route.Definition assets(final String path, final String location) {
     AssetHandler handler = new AssetHandler(location);
-    onStart(r -> {
-      Config conf = r.require(Config.class);
-      handler
-          .cdn(conf.getString("assets.cdn"))
-          .lastModified(conf.getBoolean("assets.lastModified"))
-          .etag(conf.getBoolean("assets.etag"))
-          .maxAge(conf.getString("assets.cache.maxAge"));
-    });
+    configureAssetHandler(handler);
     return assets(path, handler);
   }
 
@@ -2028,7 +2024,8 @@ public Jooby map(final Mapper<?> mapper) {
    * @param injectorFactory the injection provider
    * @return this instance.
    */
-  public Jooby injector(final BiFunction<Stage, com.google.inject.Module, Injector> injectorFactory) {
+  public Jooby injector(
+      final BiFunction<Stage, com.google.inject.Module, Injector> injectorFactory) {
     this.injectorFactory = injectorFactory;
     return this;
   }
@@ -3200,4 +3197,15 @@ private static Logger logger(final Jooby app) {
     return LoggerFactory.getLogger(app.getClass());
   }
 
+  public void configureAssetHandler(final AssetHandler handler) {
+    onStart(r -> {
+      Config conf = r.require(Config.class);
+      handler
+          .cdn(conf.getString("assets.cdn"))
+          .lastModified(conf.getBoolean("assets.lastModified"))
+          .etag(conf.getBoolean("assets.etag"))
+          .maxAge(conf.getString("assets.cache.maxAge"));
+    });
+  }
+
 }

coverage-report/src/test/resources/static/images/fun.gif

@@ -18,6 +18,7 @@
  */
 package org.jooby;
 
+import java.nio.file.Path;
 import java.util.Optional;
 import java.util.concurrent.Executor;
 import java.util.function.Predicate;
@@ -1435,7 +1436,40 @@ Route.Collection delete(String path1,
    * @param path The path to publish.
    * @return A new route definition.
    */
-  Route.Definition assets(String path);
+  default Route.Definition assets(final String path) {
+    return assets(path, "/");
+  }
+
+  /**
+   * Static files handler on external location.
+   *
+   * <pre>
+   *   assets("/assets/**", Paths.get("/www"));
+   * </pre>
+   *
+   * For example <code>GET /assets/file.js</code> will be resolve as <code>/www/file.js</code> on
+   * server file system.
+   *
+   * <p>
+   * The {@link AssetHandler} one step forward and add support for serving files from a CDN out of
+   * the box. All you have to do is to define a <code>assets.cdn</code> property:
+   * </p>
+   * <pre>
+   * assets.cdn = "http://d7471vfo50fqt.cloudfront.net"
+   * </pre>
+   *
+   * A GET to <code>/assets/js/index.js</code> will be redirected to:
+   * <code>http://d7471vfo50fqt.cloudfront.net/assets/js/index.js</code>.
+   *
+   * You can turn on/off <code>ETag</code> and <code>Last-Modified</code> headers too using
+   * <code>assets.etag</code> and <code>assets.lastModified</code>. These two properties are enabled
+   * by default.
+   *
+   * @param path The path to publish.
+   * @param basedir Base directory.
+   * @return A new route definition.
+   */
+  Route.Definition assets(final String path, Path basedir);
 
   /**
    * Static files handler. Like {@link #assets(String)} but let you specify a different classpath

coverage-report/src/test/resources/static/images/prey.jpg

@@ -20,10 +20,11 @@
 
 import static java.util.Objects.requireNonNull;
 
-import java.io.File;
 import java.net.MalformedURLException;
 import java.net.URL;
-import java.net.URLClassLoader;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.text.MessageFormat;
 import java.time.Duration;
 import java.util.Date;
@@ -84,13 +85,15 @@
  */
 public class AssetHandler implements Route.Handler {
 
-  private static final Function1<ClassLoader, ClassLoader> cloader = loader().memoized();
+  private interface Loader {
+    URL getResource(String name);
+  }
 
   private static final Function1<String, String> prefix = prefix().memoized();
 
   private Function2<Request, String, String> fn;
 
-  private ClassLoader loader;
+  private Loader loader;
 
   private String cdn;
 
@@ -128,7 +131,38 @@ public class AssetHandler implements Route.Handler {
    * @param loader The one who load the static resources.
    */
   public AssetHandler(final String pattern, final ClassLoader loader) {
-    init(Route.normalize(pattern), loader);
+    init(Route.normalize(pattern), Paths.get("public"), loader);
+  }
+
+  /**
+   * <p>
+   * Creates a new {@link AssetHandler}. The handler accepts a location pattern, that serve for
+   * locating the static resource.
+   * </p>
+   *
+   * Given <code>assets("/assets/**", "/")</code> with:
+   *
+   * <pre>
+   *   GET /assets/js/index.js it translates the path to: /assets/js/index.js
+   * </pre>
+   *
+   * Given <code>assets("/js/**", "/assets")</code> with:
+   *
+   * <pre>
+   *   GET /js/index.js it translate the path to: /assets/js/index.js
+   * </pre>
+   *
+   * Given <code>assets("/webjars/**", "/META-INF/resources/webjars/{0}")</code> with:
+   *
+   * <pre>
+   *   GET /webjars/jquery/2.1.3/jquery.js it translate the path to: /META-INF/resources/webjars/jquery/2.1.3/jquery.js
+   * </pre>
+   *
+   * @param pattern Pattern to locate static resources.
+   * @param basedir Base directory.
+   */
+  public AssetHandler(final Path basedir) {
+    init("/{0}", basedir, getClass().getClassLoader());
   }
 
   /**
@@ -158,7 +192,7 @@ public AssetHandler(final String pattern, final ClassLoader loader) {
    * @param pattern Pattern to locate static resources.
    */
   public AssetHandler(final String pattern) {
-    init(Route.normalize(pattern), getClass().getClassLoader());
+    init(Route.normalize(pattern), Paths.get("public"), getClass().getClassLoader());
   }
 
   /**
@@ -317,40 +351,34 @@ protected URL resolve(final String path) throws Exception {
     return loader.getResource(path);
   }
 
-  private void init(final String pattern, final ClassLoader loader) {
+  private void init(final String pattern, final Path basedir, final ClassLoader loader) {
     requireNonNull(loader, "Resource loader is required.");
     this.fn = pattern.equals("/")
         ? (req, p) -> prefix.apply(p)
         : (req, p) -> MessageFormat.format(prefix.apply(pattern), vars(req));
-    this.loader = cloader.apply(loader);
+    this.loader = loader(basedir, loader);
   }
 
   private static Object[] vars(final Request req) {
     Map<Object, String> vars = req.route().vars();
     return vars.values().toArray(new Object[vars.size()]);
   }
 
-  private static Function1<ClassLoader, ClassLoader> loader() {
-    return parent -> {
-      File publicDir = new File("public");
-      if (publicDir.exists()) {
-        try {
-          return new URLClassLoader(new URL[]{publicDir.toURI().toURL() }, null) {
-            @Override
-            public URL getResource(final String name) {
-              URL url = findResource(name);
-              if (url == null) {
-                url = parent.getResource(name);
-              }
-              return url;
-            };
-          };
-        } catch (MalformedURLException ex) {
-          // shh
+  private static Loader loader(final Path basedir, final ClassLoader classloader) {
+    if (Files.exists(basedir)) {
+      return name -> {
+        Path path = basedir.resolve(name).normalize();
+        if (Files.exists(path) && path.startsWith(basedir)) {
+          try {
+            return path.toUri().toURL();
+          } catch (MalformedURLException x) {
+            // shh
+          }
         }
-      }
-      return parent;
-    };
+        return classloader.getResource(name);
+      };
+    }
+    return classloader::getResource;
   }
 
   private static Function1<String, String> prefix() {

jooby/src/main/java/org/jooby/Jooby.java

@@ -7,6 +7,8 @@
 import java.net.MalformedURLException;
 import java.net.URI;
 import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.nio.file.Paths;
 
 import org.jooby.test.MockUnit;
@@ -17,14 +19,14 @@
 import org.powermock.modules.junit4.PowerMockRunner;
 
 @RunWith(PowerMockRunner.class)
-@PrepareForTest({AssetHandler.class, File.class })
+@PrepareForTest({AssetHandler.class, File.class, Paths.class, Files.class })
 public class AssetHandlerTest {
 
   @Test
   public void customClassloader() throws Exception {
     URI uri = Paths.get("src", "test", "resources", "org", "jooby").toUri();
     new MockUnit(ClassLoader.class)
-        .expect(publicDir(uri))
+        .expect(publicDir(uri, "JoobyTest.js"))
         .run(unit -> {
           URL value = new AssetHandler("/", unit.get(ClassLoader.class))
               .resolve("JoobyTest.js");
@@ -36,7 +38,7 @@ public void customClassloader() throws Exception {
   public void shouldCallParentOnMissing() throws Exception {
     URI uri = Paths.get("src", "test", "resources", "org", "jooby").toUri();
     new MockUnit(ClassLoader.class)
-        .expect(publicDir(uri))
+        .expect(publicDir(uri, "index.js", false))
         .expect(unit -> {
           ClassLoader loader = unit.get(ClassLoader.class);
           expect(loader.getResource("index.js")).andReturn(uri.toURL());
@@ -50,15 +52,16 @@ public void shouldCallParentOnMissing() throws Exception {
 
   @Test
   public void ignoreMalformedURL() throws Exception {
+    Path path = Paths.get("src", "test", "resources", "org", "jooby");
     new MockUnit(ClassLoader.class, URI.class)
-        .expect(publicDir(null))
+        .expect(publicDir(null, "index.js"))
         .expect(unit -> {
           URI uri = unit.get(URI.class);
           expect(uri.toURL()).andThrow(new MalformedURLException());
         })
         .expect(unit -> {
           ClassLoader loader = unit.get(ClassLoader.class);
-          expect(loader.getResource("index.js")).andReturn(Paths.get("src", "test", "resources", "org", "jooby").toUri().toURL());
+          expect(loader.getResource("index.js")).andReturn(path.toUri().toURL());
         })
         .run(unit -> {
           URL value = new AssetHandler("/", unit.get(ClassLoader.class))
@@ -67,18 +70,37 @@ public void ignoreMalformedURL() throws Exception {
         });
   }
 
-  private Block publicDir(final URI uri) {
+  private Block publicDir(final URI uri, final String name) {
+    return publicDir(uri, name, true);
+  }
+
+  private Block publicDir(final URI uri, final String name, final boolean exists) {
     return unit -> {
-      File publicDir = unit.constructor(File.class)
-          .build("public");
-      expect(publicDir.exists()).andReturn(true);
-      if (uri != null) {
-        expect(publicDir.toURI()).andReturn(uri);
-      } else {
-        expect(publicDir.toURI()).andReturn(unit.get(URI.class));
+      unit.mockStatic(Paths.class);
+
+      Path basedir = unit.mock(Path.class);
+
+      expect(Paths.get("public")).andReturn(basedir);
+
+      Path path = unit.mock(Path.class);
+      expect(basedir.resolve(name)).andReturn(path);
+      expect(path.normalize()).andReturn(path);
+
+      if (exists) {
+        expect(path.startsWith(basedir)).andReturn(true);
       }
 
-      unit.registerMock(File.class, publicDir);
+      unit.mockStatic(Files.class);
+      expect(Files.exists(basedir)).andReturn(true);
+      expect(Files.exists(path)).andReturn(exists);
+
+      if (exists) {
+        if (uri != null) {
+          expect(path.toUri()).andReturn(uri);
+        } else {
+          expect(path.toUri()).andReturn(unit.get(URI.class));
+        }
+      }
     };
   }