Fixes #91: deprecate HttpResponses methods adding platform-specific n…

…ewlines in favor of alternatives which emit the response body as supplied.
jenkinsci · May 31, 2017 · 67096fb · 67096fb
1 parent a6d8680
commit 67096fb
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 29 deletions.
diff --git a/core/src/main/java/org/kohsuke/stapler/CrumbIssuer.java b/core/src/main/java/org/kohsuke/stapler/CrumbIssuer.java
@@ -27,7 +27,7 @@ public final String issueCrumb() {
      * Sends the crumb value in plain text, enabling retrieval through XmlHttpRequest.
      */
     public HttpResponse doCrumb() {
-        return HttpResponses.plainText(issueCrumb());
+        return HttpResponses.text(issueCrumb());
     }
 
     /**

diff --git a/core/src/main/java/org/kohsuke/stapler/HttpResponses.java b/core/src/main/java/org/kohsuke/stapler/HttpResponses.java
@@ -203,8 +203,9 @@ public void generateResponse(StaplerRequest req, StaplerResponse rsp, Object nod
     }
 
     /**
-     * Serves the literal HTML.
+     * @deprecated Adds a platform-specific newline; prefer {@link #literalHtml}.
      */
+    @Deprecated
     public static HttpResponse html(final String literalHtml) {
         return new HttpResponse() {
             public void generateResponse(StaplerRequest req, StaplerResponse rsp, Object node) throws IOException, ServletException {
@@ -215,8 +216,24 @@ public void generateResponse(StaplerRequest req, StaplerResponse rsp, Object nod
     }
 
     /**
-     * Serves the plain text.
+     * Serves an HTML response.
      */
+    public static HttpResponse literalHtml(final String text) {
+        return new HttpResponse() {
+            @Override
+            public void generateResponse(StaplerRequest req, StaplerResponse rsp, Object node) throws IOException, ServletException {
+                rsp.setContentType("text/html;charset=UTF-8");
+                PrintWriter pw = rsp.getWriter();
+                pw.print(text);
+                pw.flush();
+            }
+        };
+    }
+
+    /**
+     * @deprecated Adds a platform-specific newline; prefer {@link #text}.
+     */
+    @Deprecated
     public static HttpResponse plainText(final String plainText) {
         return new HttpResponse() {
             public void generateResponse(StaplerRequest req, StaplerResponse rsp, Object node) throws IOException, ServletException {
@@ -226,6 +243,21 @@ public void generateResponse(StaplerRequest req, StaplerResponse rsp, Object nod
         };
     }
 
+    /**
+     * Serves a plain text response.
+     */
+    public static HttpResponse text(final String text) {
+        return new HttpResponse() {
+            @Override
+            public void generateResponse(StaplerRequest req, StaplerResponse rsp, Object node) throws IOException, ServletException {
+                rsp.setContentType("text/plain;charset=UTF-8");
+                PrintWriter pw = rsp.getWriter();
+                pw.print(text);
+                pw.flush();
+            }
+        };
+    }
+
     public static ForwardToView forwardToView(Object it, String view) {
         return new ForwardToView(it,view);
     }

diff --git a/core/src/main/java/org/kohsuke/stapler/bind/BoundObjectTable.java b/core/src/main/java/org/kohsuke/stapler/bind/BoundObjectTable.java
@@ -173,7 +173,7 @@ private synchronized Object resolve(String id) {
         public HttpResponse doEnableLogging() {
             if (DEBUG_LOGGING) {
                 this.logging = true;
-                return HttpResponses.plainText("Logging enabled for this session: "+toString());
+                return HttpResponses.text("Logging enabled for this session: " + this + "\n");
             } else {
                 return HttpResponses.forbidden();
             }

diff --git a/core/src/test/java/org/kohsuke/stapler/DispatcherTest.java b/core/src/test/java/org/kohsuke/stapler/DispatcherTest.java
@@ -27,7 +27,7 @@ public class DispatcherTest extends JettyTestCase {
     public class IndexDispatchByName {
         @WebMethod(name="")
         public HttpResponse doHelloWorld() {
-            return HttpResponses.plainText("Hello world");
+            return HttpResponses.text("Hello world");
         }
     }
 
@@ -37,7 +37,7 @@ public HttpResponse doHelloWorld() {
     public void testIndexDispatchByName() throws Exception {
         WebClient wc = new WebClient();
         TextPage p = wc.getPage(new URL(url, "indexDispatchByName"));
-        assertEquals("Hello world\n", p.getContent());
+        assertEquals("Hello world", p.getContent());
     }
 
 
@@ -49,12 +49,12 @@ public void testIndexDispatchByName() throws Exception {
     public class VerbMatch {
         @WebMethod(name="") @GET
         public HttpResponse doGet() {
-            return HttpResponses.plainText("Got GET");
+            return HttpResponses.text("Got GET");
         }
 
         @WebMethod(name="") @POST
         public HttpResponse doPost() {
-            return HttpResponses.plainText("Got POST");
+            return HttpResponses.text("Got POST");
         }
     }
 
@@ -76,7 +76,7 @@ public void testVerbMatch() throws Exception {
 
     private void check(WebClient wc, HttpMethod m) throws java.io.IOException {
         TextPage p = wc.getPage(new WebRequestSettings(new URL(url, "verbMatch/"), m));
-        assertEquals("Got "+m.name()+"\n", p.getContent());
+        assertEquals("Got " + m.name(), p.getContent());
     }
 
 
@@ -88,23 +88,23 @@ private void check(WebClient wc, HttpMethod m) throws java.io.IOException {
     public class ArbitraryWebMethodName {
         @WebMethod(name="")
         public HttpResponse notDoPrefix() {
-            return HttpResponses.plainText("I'm index");
+            return HttpResponses.text("I'm index");
         }
 
         // this method is implicitly web method by its name
         public HttpResponse doTheNeedful() {
-            return HttpResponses.plainText("DTN");
+            return HttpResponses.text("DTN");
         }
     }
 
 
     public void testArbitraryWebMethodName() throws Exception {
         WebClient wc = new WebClient();
         TextPage p = wc.getPage(new URL(url, "arbitraryWebMethodName"));
-        assertEquals("I'm index\n", p.getContent());
+        assertEquals("I'm index", p.getContent());
 
         p = wc.getPage(new URL(url, "arbitraryWebMethodName/theNeedful"));
-        assertEquals("DTN\n", p.getContent());
+        assertEquals("DTN", p.getContent());
 
     }
 
@@ -117,7 +117,7 @@ public void testArbitraryWebMethodName() throws Exception {
     public class InterceptorStage {
         @POST
         public HttpResponse doFoo(@JsonBody Point body) {
-            return HttpResponses.plainText(body.x+","+body.y);
+            return HttpResponses.text(body.x + "," + body.y);
         }
     }
 
@@ -141,7 +141,7 @@ public void testInterceptorStage() throws Exception {
         req.setAdditionalHeader("Content-Type","application/json");
         req.setRequestBody("{x:3,y:5}");
         TextPage p = wc.getPage(req);
-        assertEquals("3,5\n",p.getContent());
+        assertEquals("3,5",p.getContent());
 
     }
 
@@ -158,7 +158,7 @@ public void testInterceptorStageContentTypeWithCharset() throws Exception {
         req.setAdditionalHeader("Content-Type","application/json; charset=utf-8");
         req.setRequestBody("{x:3,y:5}");
         TextPage p = wc.getPage(req);
-        assertEquals("3,5\n",p.getContent());
+        assertEquals("3,5",p.getContent());
 
     }
 
@@ -169,14 +169,14 @@ public void testInterceptorStageContentTypeWithCharset() throws Exception {
     public class Inheritance {
         @WebMethod(name="foo")
         public HttpResponse doBar(@QueryParameter String q) {
-            return HttpResponses.plainText("base");
+            return HttpResponses.text("base");
         }
     }
 
     public class Inheritance2 extends Inheritance {
         @Override
         public HttpResponse doBar(String q) {
-            return HttpResponses.plainText(q);
+            return HttpResponses.text(q);
         }
     }
 
@@ -185,7 +185,7 @@ public void testInheritance() throws Exception {
 
         // the request should get to the overriding method and it should still see all the annotations in the base type
         TextPage p = wc.getPage(new URL(url, "inheritance/foo?q=abc"));
-        assertEquals("abc\n", p.getContent());
+        assertEquals("abc", p.getContent());
 
         // doBar is a web method for 'foo', so bar endpoint shouldn't respond
         try {
@@ -203,14 +203,14 @@ public abstract class PutInheritance {
 
         @POST
         public HttpResponse doAcme(StaplerRequest req) throws IOException {
-            return HttpResponses.plainText("POST: "+IOUtils.toString(req.getInputStream()));
+            return HttpResponses.text("POST: " + IOUtils.toString(req.getInputStream()));
         }
     }
 
     public class PutInheritanceImpl extends PutInheritance{
         @Override
         public HttpResponse doBar(StaplerRequest req) throws IOException {
-            return HttpResponses.plainText(IOUtils.toString(req.getInputStream())+" World!");
+            return HttpResponses.text(IOUtils.toString(req.getInputStream()) + " World!");
         }
     }
 
@@ -221,7 +221,7 @@ public void testPutInheritance() throws Exception {
         WebRequestSettings wrs = new WebRequestSettings(new URL(url, "putInheritance/foo"), HttpMethod.PUT);
         wrs.setRequestBody("Hello");
         TextPage p = wc.getPage(wrs);
-        assertEquals("Hello World!\n", p.getContent());
+        assertEquals("Hello World!", p.getContent());
 
         // doBar is a web method for 'foo', so bar endpoint shouldn't respond
         try {
@@ -235,7 +235,7 @@ public void testPutInheritance() throws Exception {
         wrs = new WebRequestSettings(new URL(url, "putInheritance/acme"), HttpMethod.POST);
         wrs.setRequestBody("Hello");
         p = wc.getPage(wrs);
-        assertEquals("POST: Hello\n", p.getContent());
+        assertEquals("POST: Hello", p.getContent());
     }
 
 
@@ -275,26 +275,26 @@ public void testRequirePostOnBase() throws Exception {
 
     public void testOverloads() throws Exception {
         TextPage p = new WebClient().getPage(new URL(url, "overloaded/x"));
-        assertEquals("doX(StaplerRequest)", p.getContent().trim());
+        assertEquals("doX(StaplerRequest)", p.getContent());
     }
     public final Object overloaded = new Overloaded();
     public static class Overloaded {
         @Deprecated
         public HttpResponse doX() {
-            return HttpResponses.plainText("doX()");
+            return HttpResponses.text("doX()");
         }
         public HttpResponse doX(StaplerRequest req) {
-            return HttpResponses.plainText("doX(StaplerRequest)");
+            return HttpResponses.text("doX(StaplerRequest)");
         }
         public HttpResponse doX(StaplerResponse rsp) {
-            return HttpResponses.plainText("doX(StaplerResponse)");
+            return HttpResponses.text("doX(StaplerResponse)");
         }
         public HttpResponse doX(StaplerRequest req, StaplerResponse rsp) {
-            return HttpResponses.plainText("doX(StaplerRequest, StaplerResponse)");
+            return HttpResponses.text("doX(StaplerRequest, StaplerResponse)");
         }
         @WebMethod(name = "x")
         public HttpResponse x() {
-            return HttpResponses.plainText("x()");
+            return HttpResponses.text("x()");
         }
     }