fix actframework#394

greenlaw110 · greenlaw110 · commit f97b6a52e575 · 2017-10-28T22:41:06.000+11:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,9 @@
 # ActFramework Change Log
 
+**1.4.14**
+
+* Send back `400 Bad Request` on response trigger an obscure error page when content type is `application/vnd.openxmlformats-officedocument.spreadsheetml.sheet` #394
+
 **1.4.13 16/Oct/2017**
 
 * Update riotjs to 3.7.2
diff --git a/src/main/java/act/app/ActionContext.java b/src/main/java/act/app/ActionContext.java
@@ -510,19 +510,47 @@ public void disableCORS() {
         this.disableCors = true;
     }
 
-    public ActionContext applyContentType
-            () {
+    /**
+     * Apply content type to response with result provided.
+     *
+     * If `result` is an error then it might not apply content type as requested:
+     * * If request is not ajax request, then use `text/html`
+     * * If request is ajax request then apply requested content type only when `json` or `xml` is requested
+     * * otherwise use `text/html`
+     *
+     * @param result
+     *      the result used to check if it is error result
+     * @return
+     *      this `ActionContext`.
+     */
+    public ActionContext applyContentType(Result result) {
+        if (!result.status().isError()) {
+            return applyContentType();
+        }
+        if (req().isAjax()) {
+            H.Request req = req();
+            H.Format fmt = req.accept();
+            if (H.Format.UNKNOWN == fmt) {
+                fmt = req.contentType();
+            }
+            if (H.Format.JSON == fmt || H.Format.XML == fmt) {
+                applyContentType(fmt);
+            } else {
+                applyContentType(H.Format.HTML);
+            }
+        } else {
+            applyContentType(H.Format.HTML);
+        }
+        return this;
+    }
+
+    public ActionContext applyContentType() {
         H.Request req = req();
         H.Format fmt = req.accept();
         if (H.Format.UNKNOWN == fmt) {
             fmt = req.contentType();
         }
-
-        ResponseImplBase resp = $.cast(resp());
-        if (null != fmt) {
-            resp.initContentType(fmt.contentType());
-        }
-        resp.commitContentType();
+        applyContentType(fmt);
         return this;
     }
 
@@ -536,6 +564,14 @@ public ActionContext applyCorsSpec() {
         return this;
     }
 
+    private void applyContentType(H.Format fmt) {
+        if (null != fmt) {
+            ResponseImplBase resp = $.cast(resp());
+            resp.initContentType(fmt.contentType());
+            resp.commitContentType();
+        }
+    }
+
     private void applyGlobalCorsSetting() {
         if (this.disableCors) {
             return;
diff --git a/src/main/java/act/handler/event/ResultEvent.java b/src/main/java/act/handler/event/ResultEvent.java
@@ -58,7 +58,7 @@ public H.Response response() {
                 @Override
                 public Void apply(Result result, H.Request<?> request, H.Response<?> response) throws NotAppliedException, Osgl.Break {
                     ActionContext context = request.context();
-                    context.applyCorsSpec().applyContentType();
+                    context.applyCorsSpec().applyContentType(result);
                     context.app().eventBus().trigger(new BeforeResultCommit(result, request, response));
                     return null;
                 }

Original file line number	Diff line number	Diff line change
`@@ -58,7 +58,7 @@ public H.Response response() {`
`58`	`58`	`@Override`
`59`	`59`	`public Void apply(Result result, H.Request<?> request, H.Response<?> response) throws NotAppliedException, Osgl.Break {`
`60`	`60`	`ActionContext context = request.context();`
`61`		`- context.applyCorsSpec().applyContentType();`
	`61`	`+ context.applyCorsSpec().applyContentType(result);`
`62`	`62`	`context.app().eventBus().trigger(new BeforeResultCommit(result, request, response));`
`63`	`63`	`return null;`
`64`	`64`	`}`