You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jena.apache.org by an...@apache.org on 2020/08/28 16:43:21 UTC

[jena] branch master updated: JENA-1946: Plain text error messages

This is an automated email from the ASF dual-hosted git repository.

andy pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/jena.git


The following commit(s) were added to refs/heads/master by this push:
     new 33151fa  JENA-1946: Plain text error messages
     new b69fb49  Merge pull request #784 from afs/fuseki-errors
33151fa is described below

commit 33151fa7869d5a61b1494614732157f4f6bb726e
Author: Andy Seaborne <an...@apache.org>
AuthorDate: Fri Aug 21 19:06:56 2020 +0100

    JENA-1946: Plain text error messages
---
 .../jena/fuseki/jetty/FusekiErrorHandler.java      | 51 ++++++--------------
 .../jena/fuseki/jetty/FusekiErrorHandler1.java     | 56 ----------------------
 .../jena/fuseki/servlets/ActionErrorException.java |  2 +-
 .../apache/jena/fuseki/servlets/ActionExecLib.java |  2 +-
 .../apache/jena/fuseki/servlets/ServletOps.java    | 51 +++++++++++++++-----
 .../org/apache/jena/fuseki/main/FusekiServer.java  |  4 +-
 .../apache/jena/fuseki/cmd/JettyFusekiWebapp.java  |  4 +-
 7 files changed, 60 insertions(+), 110 deletions(-)

diff --git a/jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/jetty/FusekiErrorHandler.java b/jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/jetty/FusekiErrorHandler.java
index fd6bc03..b4b880e 100644
--- a/jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/jetty/FusekiErrorHandler.java
+++ b/jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/jetty/FusekiErrorHandler.java
@@ -20,66 +20,43 @@ package org.apache.jena.fuseki.jetty;
 
 import static java.lang.String.format;
 
-import java.io.*;
+import java.io.IOException;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
-import org.apache.jena.atlas.io.IO;
 import org.apache.jena.fuseki.servlets.ServletOps;
 import org.apache.jena.web.HttpSC;
 import org.eclipse.jetty.http.HttpMethod;
-import org.eclipse.jetty.http.MimeTypes;
 import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.server.Response;
 import org.eclipse.jetty.server.handler.ErrorHandler;
 
-/** The usual Fuseki error handler.
- *  Outputs a plain text message.
+/**
+ * Fuseki error handler (used with ServletAPI HttpServletResponse.sendError).
+ * Typically ServletOps.responseSendError is used which directly send the error and a message. 
  */
-
 public class FusekiErrorHandler extends ErrorHandler
 {
+    // Only used if ServletOps.responseSendError calls Servlet API response.sendError
+    // or a non-Fuseki error occurs.
     public FusekiErrorHandler() {}
 
     @Override
     public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException {
         String method = request.getMethod();
 
-        if ( !method.equals(HttpMethod.GET.asString())
-             && !method.equals(HttpMethod.POST.asString())
-             && !method.equals(HttpMethod.HEAD.asString()) )
+        if ( !method.equals(HttpMethod.GET.asString()) 
+            && !method.equals(HttpMethod.POST.asString()) 
+            && !method.equals(HttpMethod.HEAD.asString()) )
             return;
-
-        response.setContentType(MimeTypes.Type.TEXT_PLAIN_UTF_8.asString());
+        
         ServletOps.setNoCache(response);
-
-        ByteArrayOutputStream bytes = new ByteArrayOutputStream(1024);
-        try ( Writer writer = IO.asUTF8(bytes) ) {
-            String reason = (response instanceof Response) ? ((Response)response).getReason() : null;
-            handleErrorPage(request, writer, response.getStatus(), reason);
-            writer.flush();
-            response.setContentLength(bytes.size());
-            response.getOutputStream().write(bytes.toByteArray());
-        }
-    }
-
-    @Override
-    protected void handleErrorPage(HttpServletRequest request, Writer writer, int code, String message) throws IOException {
+        int code = response.getStatus();
+        String message = (response instanceof Response) ? ((Response)response).getReason() : HttpSC.getMessage(code);
         if ( message == null )
             message = HttpSC.getMessage(code);
-        writer.write(format("Error %d: %s\n", code, message));
-
-        Throwable th = (Throwable)request.getAttribute("javax.servlet.error.exception");
-        while (th != null) {
-            writer.write("\n");
-            StringWriter sw = new StringWriter();
-            PrintWriter pw = new PrintWriter(sw);
-            th.printStackTrace(pw);
-            pw.flush();
-            writer.write(sw.getBuffer().toString());
-            writer.write("\n");
-            th = th.getCause();
-        }
+        String msg = format("Error %d: %s\n", code, message);
+        ServletOps.writeMessagePlainText(response, msg);
     }
 }
diff --git a/jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/jetty/FusekiErrorHandler1.java b/jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/jetty/FusekiErrorHandler1.java
deleted file mode 100644
index 7210afc..0000000
--- a/jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/jetty/FusekiErrorHandler1.java
+++ /dev/null
@@ -1,56 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you 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 org.apache.jena.fuseki.jetty;
-
-import static java.lang.String.format;
-
-import java.io.IOException;
-
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-
-import org.apache.jena.fuseki.servlets.ServletOps;
-import org.apache.jena.web.HttpSC;
-import org.eclipse.jetty.http.HttpMethod;
-import org.eclipse.jetty.http.MimeTypes;
-import org.eclipse.jetty.server.Request;
-import org.eclipse.jetty.server.Response;
-import org.eclipse.jetty.server.handler.ErrorHandler;
-
-/** One line Fuseki error handler */
-public class FusekiErrorHandler1 extends ErrorHandler
-{
-    public FusekiErrorHandler1() {}
-
-    @Override
-    public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException {
-        String method = request.getMethod();
-
-        if ( !method.equals(HttpMethod.GET.asString()) 
-            && !method.equals(HttpMethod.POST.asString()) 
-            && !method.equals(HttpMethod.HEAD.asString()) )
-            return;
-
-        response.setContentType(MimeTypes.Type.TEXT_PLAIN_UTF_8.asString());
-        ServletOps.setNoCache(response);
-        int code = response.getStatus();
-        String message = (response instanceof Response) ? ((Response)response).getReason() : HttpSC.getMessage(code);
-        response.getOutputStream().print(format("Error %d: %s\n", code, message));
-    }
-}
diff --git a/jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/servlets/ActionErrorException.java b/jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/servlets/ActionErrorException.java
index d3784cf..2cef6c3 100644
--- a/jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/servlets/ActionErrorException.java
+++ b/jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/servlets/ActionErrorException.java
@@ -21,7 +21,7 @@ package org.apache.jena.fuseki.servlets;
 public class ActionErrorException extends RuntimeException {
     private final int rc;
 
-    public ActionErrorException(Throwable ex, String message, int rc) {
+    public ActionErrorException(int rc, String message, Throwable ex) {
         super(message, ex);
         this.rc = rc;
     }
diff --git a/jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/servlets/ActionExecLib.java b/jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/servlets/ActionExecLib.java
index 6ce59dc..27735c9 100644
--- a/jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/servlets/ActionExecLib.java
+++ b/jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/servlets/ActionExecLib.java
@@ -111,7 +111,7 @@ public class ActionExecLib {
                 //    global -- cxt.get(ARQ.queryTimeout)
                 //    dataset -- dataset.getContect(ARQ.queryTimeout)
                 //    protocol -- SPARQL_Query.setAnyTimeouts
-                String message = String.format("Query timed out");
+                String message = "Query timed out";
                 ServletOps.responseSendError(response, HttpSC.SERVICE_UNAVAILABLE_503, message);
             } catch (ActionErrorException ex) {
                 if ( ex.getCause() != null )
diff --git a/jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/servlets/ServletOps.java b/jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/servlets/ServletOps.java
index 6bee2ee..21f6563 100644
--- a/jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/servlets/ServletOps.java
+++ b/jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/servlets/ServletOps.java
@@ -37,22 +37,50 @@ import org.apache.jena.web.HttpSC.Code;
 
 public class ServletOps {
 
+    /** Send an HTTP error response. 
+     *  Include an optional message in the body (as text/plain), if provided.
+     *  Note that we do not set a custom Reason Phrase.
+     *  <br/>
+     *  HTTPS/2 does not have a "Reason Phrase". 
+     * 
+     * @param response
+     * @param statusCode
+     * @param message
+     */
     public static void responseSendError(HttpServletResponse response, int statusCode, String message) {
-        try {
-            response.sendError(statusCode, message);
-        } catch (IOException ex) {
-            errorOccurred(ex);
-        } catch (IllegalStateException ex) {}
+        response.setStatus(statusCode);
+        if ( message != null )
+            writeMessagePlainText(response, message);
+        //response.sendError(statusCode, message);
     }
 
+    /** Send an HTTP response with no body */
     public static void responseSendError(HttpServletResponse response, int statusCode) {
+        response.setStatus(statusCode);
+    }
+
+    /** Write a plain text body.
+     * <p>
+     * Use Content-Length so the connection is preserved.
+     */
+    public static void writeMessagePlainText(HttpServletResponse response, String message) {
+        if ( message == null )
+            return;
         try {
-            response.sendError(statusCode);
+            if ( ! message.endsWith("\n") )
+                message = message+"\n";
+            response.setContentLength(message.length());
+            response.setContentType(WebContent.contentTypeTextPlain);
+            response.setCharacterEncoding(WebContent.charsetUTF8);
+            ServletOps.setNoCache(response);
+            try(ServletOutputStream out = response.getOutputStream()){
+                out.print(message);
+            }
         } catch (IOException ex) {
             errorOccurred(ex);
-        }
+        } catch (IllegalStateException ex) {}
     }
-
+    
     public static void successNoContent(HttpAction action) {
         success(action, HttpSC.NO_CONTENT_204);
     }
@@ -78,6 +106,7 @@ public class ServletOps {
     public static void successPage(HttpAction action, String message) {
         try {
             action.response.setContentType("text/html");
+            action.response.setCharacterEncoding(WebContent.charsetUTF8);
             action.response.setStatus(HttpSC.OK_200);
             PrintWriter out = action.response.getWriter();
             out.println("<html>");
@@ -146,11 +175,11 @@ public class ServletOps {
     }
 
     public static void error(int statusCode) {
-        throw new ActionErrorException(null, null, statusCode);
+        throw new ActionErrorException(statusCode, null, null);
     }
 
     public static void error(int statusCode, String string) {
-        throw new ActionErrorException(null, string, statusCode);
+        throw new ActionErrorException(statusCode, string, null);
     }
 
     public static void errorOccurred(String message) {
@@ -162,7 +191,7 @@ public class ServletOps {
     }
 
     public static void errorOccurred(String message, Throwable ex) {
-        throw new ActionErrorException(ex, message, HttpSC.INTERNAL_SERVER_ERROR_500);
+        throw new ActionErrorException(HttpSC.INTERNAL_SERVER_ERROR_500, message, ex);
     }
 
     public static String formatForLog(String string) {
diff --git a/jena-fuseki2/jena-fuseki-main/src/main/java/org/apache/jena/fuseki/main/FusekiServer.java b/jena-fuseki2/jena-fuseki-main/src/main/java/org/apache/jena/fuseki/main/FusekiServer.java
index 2b09b94..a4e2aaa 100644
--- a/jena-fuseki2/jena-fuseki-main/src/main/java/org/apache/jena/fuseki/main/FusekiServer.java
+++ b/jena-fuseki2/jena-fuseki-main/src/main/java/org/apache/jena/fuseki/main/FusekiServer.java
@@ -40,7 +40,7 @@ import org.apache.jena.fuseki.auth.AuthPolicy;
 import org.apache.jena.fuseki.build.FusekiConfig;
 import org.apache.jena.fuseki.ctl.ActionPing;
 import org.apache.jena.fuseki.ctl.ActionStats;
-import org.apache.jena.fuseki.jetty.FusekiErrorHandler1;
+import org.apache.jena.fuseki.jetty.FusekiErrorHandler;
 import org.apache.jena.fuseki.jetty.JettyHttps;
 import org.apache.jena.fuseki.jetty.JettyLib;
 import org.apache.jena.fuseki.metrics.MetricsProviderRegistry;
@@ -972,7 +972,7 @@ public class FusekiServer {
                 contextPath = "/" + contextPath;
             ServletContextHandler context = new ServletContextHandler();
             context.setDisplayName(Fuseki.servletRequestLogName);
-            context.setErrorHandler(new FusekiErrorHandler1());
+            context.setErrorHandler(new FusekiErrorHandler());
             context.setContextPath(contextPath);
             // SPARQL Update by HTML - not the best way but.
             context.setMaxFormContentSize(1024*1024);
diff --git a/jena-fuseki2/jena-fuseki-webapp/src/main/java/org/apache/jena/fuseki/cmd/JettyFusekiWebapp.java b/jena-fuseki2/jena-fuseki-webapp/src/main/java/org/apache/jena/fuseki/cmd/JettyFusekiWebapp.java
index b1086a8..420d362 100644
--- a/jena-fuseki2/jena-fuseki-webapp/src/main/java/org/apache/jena/fuseki/cmd/JettyFusekiWebapp.java
+++ b/jena-fuseki2/jena-fuseki-webapp/src/main/java/org/apache/jena/fuseki/cmd/JettyFusekiWebapp.java
@@ -199,8 +199,8 @@ public class JettyFusekiWebapp {
         // which happens during server startup.
         // This the name of the ServletContext logger as well
         webapp.setDisplayName(Fuseki.servletRequestLogName);
-        webapp.setParentLoaderPriority(true);  // Normal Java classloader behaviour.
-        webapp.setErrorHandler(new FusekiErrorHandler());
+        webapp.setParentLoaderPriority(true);               // Normal Java classloader behaviour.
+        webapp.setErrorHandler(new FusekiErrorHandler());   // If used.
         return webapp;
     }