Escape < and > in the reporter. · testng-team/testng@6a577ff

dunse · 2012-07-10T12:40:52Z

One issue with this change:
Reporter's log()/getOutput() is used by many reporters and would break them.
This is because the have their own handling of manipulating the strings. (It is handy to get unescaped output in some cases)

I would suggest moving this code into the *HTMLReporter.java code to allow the flexibility to pass unescaped log entries to the final reporter.

cbeust · 2012-07-10T13:33:16Z

Can you be more specific? I don't see any place in SuiteHTMLReporter and TestHTMLReporter that would get broken by this change.

Thanks.

dunse · 2012-07-10T13:42:36Z

It would not break the internal reporters.
What I'm saying is that for example ReportNG calls Reporter.getOutput() to get the raw data and then escapes the output depending on a user specified option.
If this code is put into the log function it prevents ReportNG to offer unescaped output in their report.

The suggestion I made was to remove this code and place it inside the *HTMLReporters themselves to maintain the consistency of the log messages inside the Reporter class for others to use.

cbeust · 2012-07-10T14:04:06Z

I see. The problem is that every user of Reporter would have to do the escaping then, which is not optimal. I'd rather do this at the Reporter level and if users want to output a less-than or greater-than sign, they can always use the entities < and >...

dunse · 2012-07-12T09:29:14Z

How about something like this?

diff --git a/src/main/java/org/testng/Reporter.java b/src/main/java/org/testng/Reporter.java
old mode 100755
new mode 100644
index 8abdba2..b0841fc
--- a/src/main/java/org/testng/Reporter.java
+++ b/src/main/java/org/testng/Reporter.java
@@ -31,6 +31,15 @@
   // spawns its own thread.
   private static ThreadLocal<ITestResult> m_currentTestResult = new InheritableThreadLocal<ITestResult>();

+  /** Available EscapeFormats:<br/>
+   * RAW (default), HTML.
+   */
+  public enum EscapeFormat {
+     /** Default unescape output. */
+     RAW,
+     /** Specify Escaped Format in HTML. */
+     HTML
+  }
   /**
    * All output logged in a sequential order.
    */
@@ -42,8 +51,11 @@
     m_currentTestResult.set(m);
   }

+  /**
+   * @return All raw log entries added by {@link #log(String)}
+   */
   public static List<String> getOutput() {
-    return m_output;
+    return getEncodedOutput(EscapeFormat.RAW);
   }

   /**
@@ -55,9 +67,6 @@
   }

   private static synchronized void log(String s, ITestResult m) {
-    // Escape for the HTML reports
-    s = s.replace("<", "[").replace(">", "]") + "<br>";
-
     // synchronization needed to ensure the line number and m_output are updated atomically
     int n = getOutput().size();
     List<Integer> lines = m_methodOutputMap.get(m);
@@ -130,15 +139,68 @@
     return m_currentTestResult.get();
   }

-  public static synchronized List<String> getOutput(ITestResult tr) {
+  /**
+   * Encodes the internal log entries before returned.<br/>
+   * This will only return log entries associated with given TestResult.<br/>
+   * Valid formats are defined in {@link EscapeFormat}.
+   * @param tr TestResult.
+   * @param ef {@link EscapeFormat}.
+   * @return Encoded log entries for TestResult.
+   */
+  public static synchronized List<String> getEncodedOutput(ITestResult tr, EscapeFormat ef) {
     List<String> result = Lists.newArrayList();
     List<Integer> lines = m_methodOutputMap.get(tr);
     if (lines != null) {
-      for (Integer n : lines) {
-        result.add(getOutput().get(n));
-      }
+       for (Integer n : lines) {
+           switch(ef) {
+           case HTML:
+               result.add(getOutput().get(n));
+               break;
+           case RAW:
+           default:
+               result.add(getOutput().get(n));
+               break;
+           }
+       }
     }

     return result;
   }
+
+  /**
+   * 
+   * @param tr TestResult.
+   * @return Log entries for TestResult.
+   */
+  public static synchronized List<String> getOutput(ITestResult tr) {
+     return getEncodedOutput(tr, EscapeFormat.RAW);
+  }
+
+  /**
+   * Encodes the internal log entries before returned.<br/>
+   * This will return all log entries.<br/>
+   * Valid formats are defined in {@link EscapeFormat}.
+   * @param tr TestResult.
+   * @param ef {@link EscapeFormat}
+   * @return All encoded log entries.
+   */
+  public static synchronized List<String> getEncodedOutput(EscapeFormat ef) {
+     List<String> result = Lists.newArrayList();
+     switch(ef) {
+     case HTML:
+         for (String line : m_output) {
+             result.add(getHTMLEncodedString(line));
+         }
+         break;
+     case RAW:
+     default:
+         result = m_output;
+         break;
+     }
+     return result;
+  }
+  
+  public static String getHTMLEncodedString(String input) {
+     return input.replace("<", "[").replace(">", "]") + "<br>";
+  }
 }

cbeust · 2012-07-13T06:37:41Z

To be honest, this looks a bit overkill for such a simple problem...

dunse · 2012-07-13T10:38:59Z

I'm just concerned about the impact of the committed change and put together an extendable way of achieving the same.
By helping all Reporters by escaping the special characters is also preventing all Reporters who wish to use them unescaped.
The reason the impact is so high is that replacing < with [ is impossible to revert. Where I use TestNG, this will completly wreck the implementation and will rely on writing custom code for implementation in both the loggers and reporters just to bypass this small change.
(And I won't be the only one that will have to do this)

cbeust · 2012-07-13T14:45:28Z

If you really, really want a "less than" character, just use the entity <, I don't see the problem...

dunse · 2012-07-13T23:37:25Z

But that would only display a < character on the screen. If I want to create a clickable hyperlink on screen, that would not really work.

SilverFox13 · 2012-07-24T17:16:00Z

I also experience problem after this change, we include links to screen shots in reporter output:

Reporter.log("Error Screenshot created for: "+"<a href=\"../ScreenShots/" +testName + "_Error.png" + "\">"+testName+"</a>");

Now link does not work because "<" and ">" is changed to "[" and "]" accordingly.

Also we use ">" symbols in other places and now it is changed :(

Now my reporter log looks like this:

-----] Selenuim Server Started

=====] OH_AccountCreation: Started
Error Screenshot created for: [a href="../ScreenShots/OH_AccountCreation_Error.png"]OH_AccountCreation[/a]
[===== OH_AccountCreation: FAILED

[----- Selenuim Server Stopped

Only solution that would work for us without making major changes in our framework is to take TestNG source and build it without this change, but I believe this is not right way to go :)

jcolebrand · 2012-08-07T14:51:17Z

I suggest just htmlencoding all the output, and then the client can htmlunencode it if need be. This way you prevent issues with < and > and do the same thing everyone else usually does in such a case.

herikwebb · 2012-08-07T16:50:53Z

Same issue as SilverFox. I am unable to include html in my report as a result of this. < and > are not a feasible workaround in this regard.

hackndoes · 2012-08-13T10:52:18Z

If you are so interested in escaping yourself, why don't you use some well known html escaper classes such as StringEscapeUtils to do StringEscapeUtils.escapeHtml4("log information link") ?
I thing that would be much safer.
after 6.7 I can't do any html tag reporting (which you came to solve) everything is treated as plain text.

cbeust · 2012-08-13T15:54:36Z

Agreed. I think I'll revert the change so that the reporter doesn't do any escaping. Any client that wants escaping will have to escape the string before passing it to Reporter.log().

cbeust · 2012-08-18T10:38:12Z

Done: Reporter.log() doesn't do any HTML escaping by default. If you want to escape, call Reporter.setEscapeHtml(true) before you start logging.

abouquet · 2012-10-09T08:45:22Z

Hi, I encountered this problem too. When does the next version will be released ?

Thanks,

sudarshanch · 2013-04-17T19:00:23Z

Does it works in 6.8.1? It still not worked for me in 6.8.1

snowmass · 2014-02-01T01:26:22Z

I am using 6.8.7 and what I have figured out is that in the ./test-report/Ant Suite/*.html there is no escaping, but in the ./test-report/emailable-report.html there is escaping, that is,

converting < to &lt;

in the sections generated by involking Reporter.log().

snowmass · 2014-02-03T22:51:03Z

Perhaps the escaping in the emailable-report generated during Report.log() originally stems from changes here:

http://code.google.com/p/testng/issues/detail?id=91

Now in todays' codebase (I am looking at 6.8.7) I see a EmailableReporter2.java and EmailableReporter.java with some logic to choose one or the other. What is the proper way to invoke one or the other as it seems one is escaping and one is not escaping?

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Commit

There are no files selected for viewing

19 comments on commit `6a577ff`

dunse commented on `6a577ff` Jul 10, 2012

cbeust commented on `6a577ff` Jul 10, 2012

dunse commented on `6a577ff` Jul 10, 2012

cbeust commented on `6a577ff` Jul 10, 2012

dunse commented on `6a577ff` Jul 12, 2012

cbeust commented on `6a577ff` Jul 13, 2012

dunse commented on `6a577ff` Jul 13, 2012

cbeust commented on `6a577ff` Jul 13, 2012

dunse commented on `6a577ff` Jul 13, 2012

SilverFox13 commented on `6a577ff` Jul 24, 2012

jcolebrand commented on `6a577ff` Aug 7, 2012

herikwebb commented on `6a577ff` Aug 7, 2012

hackndoes commented on `6a577ff` Aug 13, 2012

cbeust commented on `6a577ff` Aug 13, 2012

cbeust commented on `6a577ff` Aug 18, 2012

abouquet commented on `6a577ff` Oct 9, 2012

sudarshanch commented on `6a577ff` Apr 17, 2013

snowmass commented on `6a577ff` Feb 1, 2014

snowmass commented on `6a577ff` Feb 3, 2014

Commit

There are no files selected for viewing

19 comments on commit 6a577ff

dunse commented on 6a577ff Jul 10, 2012

Choose a reason for hiding this comment

cbeust commented on 6a577ff Jul 10, 2012

Choose a reason for hiding this comment

dunse commented on 6a577ff Jul 10, 2012

Choose a reason for hiding this comment

cbeust commented on 6a577ff Jul 10, 2012

Choose a reason for hiding this comment

dunse commented on 6a577ff Jul 12, 2012

Choose a reason for hiding this comment

cbeust commented on 6a577ff Jul 13, 2012

Choose a reason for hiding this comment

dunse commented on 6a577ff Jul 13, 2012

Choose a reason for hiding this comment

cbeust commented on 6a577ff Jul 13, 2012

Choose a reason for hiding this comment

dunse commented on 6a577ff Jul 13, 2012

Choose a reason for hiding this comment

SilverFox13 commented on 6a577ff Jul 24, 2012

Choose a reason for hiding this comment

jcolebrand commented on 6a577ff Aug 7, 2012

Choose a reason for hiding this comment

herikwebb commented on 6a577ff Aug 7, 2012

Choose a reason for hiding this comment

hackndoes commented on 6a577ff Aug 13, 2012

Choose a reason for hiding this comment

cbeust commented on 6a577ff Aug 13, 2012

Choose a reason for hiding this comment

cbeust commented on 6a577ff Aug 18, 2012

Choose a reason for hiding this comment

abouquet commented on 6a577ff Oct 9, 2012

Choose a reason for hiding this comment

sudarshanch commented on 6a577ff Apr 17, 2013

Choose a reason for hiding this comment

snowmass commented on 6a577ff Feb 1, 2014

Choose a reason for hiding this comment

snowmass commented on 6a577ff Feb 3, 2014

Choose a reason for hiding this comment

19 comments on commit `6a577ff`

dunse commented on `6a577ff` Jul 10, 2012

cbeust commented on `6a577ff` Jul 10, 2012

dunse commented on `6a577ff` Jul 10, 2012

cbeust commented on `6a577ff` Jul 10, 2012

dunse commented on `6a577ff` Jul 12, 2012

cbeust commented on `6a577ff` Jul 13, 2012

dunse commented on `6a577ff` Jul 13, 2012

cbeust commented on `6a577ff` Jul 13, 2012

dunse commented on `6a577ff` Jul 13, 2012

SilverFox13 commented on `6a577ff` Jul 24, 2012

jcolebrand commented on `6a577ff` Aug 7, 2012

herikwebb commented on `6a577ff` Aug 7, 2012

hackndoes commented on `6a577ff` Aug 13, 2012

cbeust commented on `6a577ff` Aug 13, 2012

cbeust commented on `6a577ff` Aug 18, 2012

abouquet commented on `6a577ff` Oct 9, 2012

sudarshanch commented on `6a577ff` Apr 17, 2013

snowmass commented on `6a577ff` Feb 1, 2014

snowmass commented on `6a577ff` Feb 3, 2014