JSPWiki
  1. JSPWiki
  2. JSPWIKI-712

Entities in ChangeNote should be decoded when "keep editing"

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.8.4, 2.9
    • Fix Version/s: 2.10.1
    • Component/s: Default template
    • Labels:
    • Environment:

      Windows XP, Tomcat 7.0

      Description

      Steps to reproduce the bug:-
      1. Go here: http://doc.jspwiki.org/2.4/Edit.jsp?page=WindowsInstall
      2. Type in changenote: Testing "quotes" & ampersand
      3. Click preview
      4. Click Keep Editing
      5. The changenote looks like this: Testing "quotes" & ampersand

      Now the user has to remove it and type (Testing "quotes" & ampersand) again before saving. If the user didn't notice it, then the comment will be saved as "Testing "quotes" & ampersand" in the history.

      =================================================

      I know that entities need to be encoded for security reasons but this is a Bug.

      When "keep editing" button is clicked, the comment should appear in decoded format. For that, there should be a javascript that execute "after" the page is loaded. No need to change any of the TextUtil.replaceEntities() methods.

      =================================================

      This is how I fixed it in my pc:
      1. Added the following script to commonheader.jsp (or prettify.js)

      <script type="text/javascript">
      function decodeChangeNote()

      { document.getElementById("changenote").value = document.getElementById("changenote").value .replace(/&amp;/g,"&") .replace(/&lt;/g,"<") .replace(/&gt;/g,">") .replace(/&quot;/g,"\""); }

      </script>

      2. Changed <body> tag in EditTemplate.jsp to call this js function on load.

      <body onload="decodeChangeNote()">

      ====================================================

      Now JSPWiki works fine for me. Polish this fix if needed and commit it. Please correct me if I'm wrong.

      1. JSPWIKI-712-test3.png
        88 kB
        Harry Metske
      2. JSPWIKI-712-test2.png
        77 kB
        Harry Metske
      3. JSPWIKI-712-test1.png
        48 kB
        Harry Metske
      4. JSPWIKI-712.patch
        0.8 kB
        Harry Metske

        Activity

        Hide
        Harry Metske added a comment -

        I agree this is a bug.
        Any comments on the suggested solution (Dirk) ?

        Show
        Harry Metske added a comment - I agree this is a bug. Any comments on the suggested solution (Dirk) ?
        Hide
        brushed added a comment -

        Agree this is a bug.
        However, I think the root casue of the problem is in edit.jsp, and would better be solved there.
        BTW, the same wrong behaviour is happening on other fields like author and link, which you can set when adding a comment to a page.

        Need to dig a bit deeper for a solution.

        dirk

        Show
        brushed added a comment - Agree this is a bug. However, I think the root casue of the problem is in edit.jsp, and would better be solved there. BTW, the same wrong behaviour is happening on other fields like author and link, which you can set when adding a comment to a page. Need to dig a bit deeper for a solution. dirk
        Hide
        Florian Holeczek added a comment -

        Still present in v2.9.0-incubating-6
        Guess we will move this to 2.9.1.

        Show
        Florian Holeczek added a comment - Still present in v2.9.0-incubating-6 Guess we will move this to 2.9.1.
        Hide
        Harry Metske added a comment -

        Proposed patch.

        Show
        Harry Metske added a comment - Proposed patch.
        Hide
        Harry Metske added a comment -

        proposed patch is wrong, it reintroduces an xss vulnerability (see JSPWIKI-319), the replaceEntities() should not be removed, but be done somewhere else, preferably just before saving the page.

        Show
        Harry Metske added a comment - proposed patch is wrong, it reintroduces an xss vulnerability (see JSPWIKI-319 ), the replaceEntities() should not be removed, but be done somewhere else, preferably just before saving the page.
        Hide
        Harry Metske added a comment -

        fixed in 2.9.1-svn-18

        Show
        Harry Metske added a comment - fixed in 2.9.1-svn-18
        Hide
        brushed added a comment - - edited

        JSPWIKI-742

        The current fix solves the issue only partly.
        Note that this issue also occurs on the 'author' and 'link' fields.

        The issue is caused by multiple invocations of TextUtil.replaceEntities();
        once in the top-level Edit.jsp; once in the template plan.jsp and once more in the template preview.jsp.
        This is because <c:out> (used in the template jsp's) by default converts the HTML special characters to their corresponding character entity codes.

        Current code:
        Edit:jsp / Comment.jsp

        String changenote = TextUtil.replaceEntities( findParam( pageContext, "changenote" ) );
        

        plain.jsp

        <td><input type="text" name="changenote" id="changenote" size="80" maxlength="80" value="<c:out value='${changenote}'/>"/></td>
        

        preview.jsp

            <input type="hidden" name="author" value="<c:out value='${author}' />" />
            <input type="hidden" name="link" value="<c:out value='${link}' />" />
            <input type="hidden" name="remember" value="<c:out value='${remember}' />" />
            <input type="hidden" name="changenote" value="<c:out value='${changenote}' />" />
        

        I'd prefer that the fix would done on the template jsp's, keeping the top-level jsp's unchanged.
        The top-level JSP's ensure that content which need to be escaped is properly formatted. The template jsp merely display that content.
        This way, also the top-level Comment.jsp don't need to be changed.

        Proposed fixes:
        preview.jsp:

            <input type="hidden" name="author" value="${author}" />
            <input type="hidden" name="link" value="${link}" />
            <input type="hidden" name="remember" value="${remember}" />
            <input type="hidden" name="changenote" value="${changenote}" />
        

        plain.jsp:
        replace

        <td><input type="text" name="changenote" id="changenote" size="80" maxlength="80" value="<c:out value='${changenote}'/>"/></td>
        

        by

        <td><input type="text" name="changenote" id="changenote" size="80" maxlength="80" value="${changenote}"/></td>
        

        replace

        <input type="text" name="author" id="authorname" value="<c:out value='${sessionScope.author}' />" />
        

        by

        <input type="text" name="author" id="authorname" value="${author}" />
        

        replace

        <input type="text" name="link" id="link" size="24" value="<c:out value='${sessionScope.link}' />" />
        

        by

        <input type="text" name="link" id="link" size="24" value="${link}" />
        
        Show
        brushed added a comment - - edited JSPWIKI-742 The current fix solves the issue only partly. Note that this issue also occurs on the 'author' and 'link' fields. The issue is caused by multiple invocations of TextUtil.replaceEntities(); once in the top-level Edit.jsp; once in the template plan.jsp and once more in the template preview.jsp. This is because <c:out> (used in the template jsp's) by default converts the HTML special characters to their corresponding character entity codes. Current code: Edit:jsp / Comment.jsp String changenote = TextUtil.replaceEntities( findParam( pageContext, "changenote" ) ); plain.jsp <td><input type= "text" name= "changenote" id= "changenote" size= "80" maxlength= "80" value= "<c:out value='${changenote}'/>" /></td> preview.jsp <input type= "hidden" name= "author" value= "<c:out value='${author}' />" /> <input type= "hidden" name= "link" value= "<c:out value='${link}' />" /> <input type= "hidden" name= "remember" value= "<c:out value='${remember}' />" /> <input type= "hidden" name= "changenote" value= "<c:out value='${changenote}' />" /> I'd prefer that the fix would done on the template jsp's, keeping the top-level jsp's unchanged. The top-level JSP's ensure that content which need to be escaped is properly formatted. The template jsp merely display that content. This way, also the top-level Comment.jsp don't need to be changed. Proposed fixes: preview.jsp: <input type= "hidden" name= "author" value= "${author}" /> <input type= "hidden" name= "link" value= "${link}" /> <input type= "hidden" name= "remember" value= "${remember}" /> <input type= "hidden" name= "changenote" value= "${changenote}" /> plain.jsp: replace <td><input type= "text" name= "changenote" id= "changenote" size= "80" maxlength= "80" value= "<c:out value='${changenote}'/>" /></td> by <td><input type= "text" name= "changenote" id= "changenote" size= "80" maxlength= "80" value= "${changenote}" /></td> replace <input type= "text" name= "author" id= "authorname" value= "<c:out value='${sessionScope.author}' />" /> by <input type= "text" name= "author" id= "authorname" value= "${author}" /> replace <input type= "text" name= "link" id= "link" size= "24" value= "<c:out value='${sessionScope.link}' />" /> by <input type= "text" name= "link" id= "link" size= "24" value= "${link}" />
        Hide
        Harry Metske added a comment -

        +1

        Show
        Harry Metske added a comment - +1
        Hide
        brushed added a comment -

        Fixed in 2.9.1-svn-19.

        Show
        brushed added a comment - Fixed in 2.9.1-svn-19.
        Hide
        Harry Metske added a comment -

        There are a few more places where we still have "double escaping" , see attached screenshots.

        Show
        Harry Metske added a comment - There are a few more places where we still have "double escaping" , see attached screenshots.
        Hide
        brushed added a comment -

        Solved in v2.10.1-svn-5, for the HADDOCK template

        Show
        brushed added a comment - Solved in v2.10.1-svn-5, for the HADDOCK template

          People

          • Assignee:
            brushed
            Reporter:
            Vigneshwaran Raveendran
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development