FroMage / jax-doclets

Set of JavaDoc doclets for modern Java annotations APIs
http://www.lunatech-labs.com/open-source/jax-doclets
GNU Lesser General Public License v3.0
26 stars 23 forks source link

Ampersand (&) in query string should be properly HTML escaped #21

Closed stanio closed 12 years ago

stanio commented 12 years ago

There are few places which output raw & (ampersand) character in the HTML source, like:

foo?bar&current_baz=...

which leads to undesired rendering (the least) like:

foo?bar¤t_baz=...

Proposed patch:

diff --git a/doclets/src/main/java/com/lunatech/doclets/jax/jaxrs/writers/IndexWriter.java b/doclets/src/main/java/com/lunatech/doclets/jax/jaxrs/writers/IndexWriter.java
--- a/doclets/src/main/java/com/lunatech/doclets/jax/jaxrs/writers/IndexWriter.java
+++ b/doclets/src/main/java/com/lunatech/doclets/jax/jaxrs/writers/IndexWriter.java
@@ -96,11 +96,11 @@
     around("tt", httpMethod);
     close("a");
     close("td");
     open("td");
     open("a href='" + path + "/index.html'");
-    around("tt", Utils.getDisplayURL(this, resource, method));
+    around("tt", escape(Utils.getDisplayURL(this, resource, method)));
     close("a");
     close("td");
     open("td");
     Doc javaDoc = method.getJavaDoc();
     if (javaDoc != null && javaDoc.firstSentenceTags() != null)
diff --git a/doclets/src/main/java/com/lunatech/doclets/jax/jaxrs/writers/MethodWriter.java b/doclets/src/main/java/com/lunatech/doclets/jax/jaxrs/writers/MethodWriter.java
--- a/doclets/src/main/java/com/lunatech/doclets/jax/jaxrs/writers/MethodWriter.java
+++ b/doclets/src/main/java/com/lunatech/doclets/jax/jaxrs/writers/MethodWriter.java
@@ -384,11 +384,11 @@
     if (!queryParameters.isEmpty()) {
       print("?");
       boolean first = true;
       for (String name : queryParameters.keySet()) {
         if (!first)
-          print("&");
+          print("&");
         print(name);
         print("=…");
         first = false;
       }
     }
@@ -414,11 +414,11 @@
     if (!formParameters.isEmpty()) {
       print("\n");
       boolean first = true;
       for (String name : formParameters.keySet()) {
         if (!first)
-          print("&");
+          print("&");
         print(name);
         print("=…");
         first = false;
       }
     }
diff --git a/doclets/src/main/java/com/lunatech/doclets/jax/jaxrs/writers/ResourceWriter.java b/doclets/src/main/java/com/lunatech/doclets/jax/jaxrs/writers/ResourceWriter.java
--- a/doclets/src/main/java/com/lunatech/doclets/jax/jaxrs/writers/ResourceWriter.java
+++ b/doclets/src/main/java/com/lunatech/doclets/jax/jaxrs/writers/ResourceWriter.java
@@ -84,11 +84,11 @@
         continue;
       for (String httpMethod : method.getMethods()) {
         open("tr");
         open("td");
         open("tt");
-        around("a href='#" + httpMethod + "'", httpMethod + " " + Utils.getDisplayURL(this, resource, method));
+        around("a href='#" + httpMethod + "'", httpMethod + " " + escape(Utils.getDisplayURL(this, resource, method)));
         close("tt");
         close("td");
         open("td");
         Doc javaDoc = method.getJavaDoc();
         if (javaDoc != null && javaDoc.firstSentenceTags() != null)
stanio commented 12 years ago

This appears same as/duplicate of issue #7. The patch here however doesn't touch jax/jaxrs/model/ResourceMethod.java, but takes care of escaping/encoding in the Writer classes, which is more appropriate place, in my opinion.

FroMage commented 12 years ago

This is fixed by #7. Thanks for reporting. I personally think it's better to correct the encoding issue in ResourceMethod, but only time will tell if that's right or not.

stanio commented 11 years ago

I personally think it's better to correct the encoding issue in ResourceMethod, but only time will tell if that's right or not.

My rationale is the Writer takes care of generating output content in some format, that is encoding the content in that format, while ResourceMethod is a model object encapsulating the content independent of specific output format. "Mangling" the content in the model in first place would make the output in a different format much trickier, the least.