Cors: fix some tests and remove the isSimple check, just write back headers when Origin is present

jknack · jknack · commit f0e130fa07c4 · 2019-11-12T11:06:57.000-03:00
diff --git a/jooby/src/main/java/io/jooby/CorsHandler.java b/jooby/src/main/java/io/jooby/CorsHandler.java
@@ -68,7 +68,11 @@ public CorsHandler() {
   @Nonnull @Override public Route.Handler apply(@Nonnull Route.Handler next) {
     return ctx -> {
       String origin = ctx.header("Origin").valueOrNull();
-      if (origin != null && options.allowOrigin(origin)) {
+      if (origin != null) {
+        if (!options.allowOrigin(origin)) {
+          log.debug("denied origin: {}", origin);
+          return ctx.send(StatusCode.FORBIDDEN);
+        }
         log.debug("allowed origin: {}", origin);
         if (isPreflight(ctx)) {
           log.debug("handling preflight for: {}", origin);
@@ -80,12 +84,8 @@ public CorsHandler() {
             return ctx.send(StatusCode.FORBIDDEN);
           }
         } else {
-          // Origin is present, is Simple CORS?
-          if (isSimple(ctx)) {
-            log.debug("handling simple cors for: {}", origin);
-            ctx.setResetHeadersOnError(false);
-            simple(ctx, options, origin);
-          } else if (ctx.getMethod().equalsIgnoreCase(Router.OPTIONS)) {
+          // OPTIONS?
+          if (ctx.getMethod().equalsIgnoreCase(Router.OPTIONS)) {
             // handle normal OPTIONS
             Router router = ctx.getRouter();
             List<String> allow = new ArrayList<>();
@@ -97,8 +97,11 @@ public CorsHandler() {
             }
             ctx.setResponseHeader("Allow", allow.stream().collect(Collectors.joining(",")));
             return ctx.send(StatusCode.OK);
+          } else {
+            log.debug("handling simple cors for: {}", origin);
+            ctx.setResetHeadersOnError(false);
+            simple(ctx, options, origin);
           }
-          //
         }
       }
       return next.apply(ctx);
@@ -113,14 +116,6 @@ private static Context methodContext(Context ctx, String method) {
     };
   }
 
-  private static boolean isSimple(Context ctx) {
-    return ctx.getMethod().equals(Router.GET)
-        || ctx.getMethod().equals(Router.POST)
-        || ctx.getMethod().equals(Router.HEAD);
-    // Suggested:
-    //  return !ctx.getMethod().equals(Router.OPTIONS);
-  }
-
   private static void simple(final Context ctx, final Cors options, final String origin) {
     if ("null".equals(origin)) {
       ctx.setResponseHeader(AC_ALLOW_ORIGIN, ANY_ORIGIN);
diff --git a/tests/src/test/java/io/jooby/Issue1413.java b/tests/src/test/java/io/jooby/Issue1413.java
@@ -47,7 +47,7 @@ public void shouldDoPreflightWithCredentials() {
           .header("Origin", "http://bar.com")
           .get("/api/v1/machines/123", rsp -> {
             assertEquals(403, rsp.code());
-            assertNull(rsp.body().string());
+            assertEquals("", rsp.body().string());
             assertNull(rsp.header("Access-Control-Allow-Origin"));
             assertNull(rsp.header("Access-Control-Allow-Credentials"));
           });
@@ -105,7 +105,7 @@ public void shouldDoPreflightWithoutCredentials() {
           .header("Origin", "http://bar.com")
           .get("/api/v1/machines/123", rsp -> {
             assertEquals(403, rsp.code());
-            assertNull(rsp.body().string());
+            assertEquals("", rsp.body().string());
             assertNull(rsp.header("Access-Control-Allow-Origin"));
             assertNull(rsp.header("Access-Control-Allow-Credentials"));
           });
