Groovy
  1. Groovy
  2. GROOVY-4715

StreamingMarkupBuilder can produce invalid xml

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 1.7.8
    • Fix Version/s: 1.8-rc-2, 1.7.10
    • Component/s: XML Processing
    • Labels:
      None
    • Environment:
      Windows and Linux

      Description

      • From #GROOVY-4115, StreamingMarkupBuilder provides option for using double quotes around attributes. But if we use this option, it can output invalid xml:
        • For example:
          def builder = new StreamingMarkupBuilder(useDoubleQuotes: true);
          builder.bind (
            {
              div(onmouseover:'foo("some text")')
            }
          }
          
        • Produce the following output, which is invalid.
              <div onmouseout="foo("some text")"></div>
          
        • Another example:
          def builder = new StreamingMarkupBuilder(useDoubleQuotes: true);
          builder.bind (
             {
               div(onmouseover:"foo('some text')")
             }
          }
          
        • Produce the following output
              <div onmouseout="foo(&apos;some text&apos;)"></div>
          
        • However, it should produce
              <div onmouseout="foo('some text')"></div>
          
      • As far as we know, MarkupBuilder will escape the apostrophe if the value is for an attribute, as opposed to element content, and if the builder is configured to surround attribute values with single quotes.
        • So if we use the MarkupBuilder to build the above html block:
          def builder = new MarkupBuilder();
          builder.setDoubleQuotes(true)
          ...
          
        • It will produce the similar xml as the above, except the apostrophe entity
          ...
          <div onmouseout= "foo('some text')"></div>
          ...
          
      • The single-quote should be displayed instead of the entity ( &apos; ) in both of the builders when attributes are surrounded by double quotes. It appears that StreamingMarkupBuilder is not communicating useDoubleQuotes to StreamingMarkupWriter. One solution we can think of is passing in useDoubleQuotes to the StreamingMarkupWriter from the StreamingMarkupBuilder in the constructor and changing StreamingMarkupWriter to use similar logic to MarkupBuilder when checking attribute value for quotes to escape.
        StreamingMarkupBuilder.java
        public bind(closure) {
        ...
        out = new StreamingMarkupWriter(out, enc, useDoubleQuotes)
        ...
        }
        
        StreamingMarkupWriter
        public void write(final c) throws IOException {
           ...
           else if(c == '\'' && this.writingAttribute && !useDoubleQuotes) {
               this.writer.write("&apos;");
           }
           else if(c == '"' && this.writingAttribute && useDoubleQuotes) {
               this.writer.write("&quot;");
           }
           ...
        }
        

      Thanks

        Activity

        Tony Trung Thanh Vo created issue -
        Paul King made changes -
        Field Original Value New Value
        Assignee Paul King [ paulk ]
        Hide
        Paul King added a comment -

        Thanks for the suggestion, fixed.

        Show
        Paul King added a comment - Thanks for the suggestion, fixed.
        Paul King made changes -
        Fix Version/s 1.8-rc-2 [ 17176 ]
        Fix Version/s 1.7.9 [ 17164 ]
        Resolution Fixed [ 1 ]
        Status Open [ 1 ] Resolved [ 5 ]
        Hide
        Tony Trung Thanh Vo added a comment -

        Hi Paul,

        Adam (my colleague) and me upgraded groovy to version 1.7.9 and the fix is not included in 1.7.9 version. However, we do see the fix is included in version 1.8.0-rc-2. So it seems that you can deploy the version 1.7.9 again or you need to include this fix in 1.7.10. You also need to make sure that the release notes of v1.7.10 include the this bug (#GROOVY-4715), since this bug (#GROOVY-4715) is already included in the release-notes of v1.7.9

        Thanks

        PS: Thanks for using our suggestion, Adam and especially I are so proud of this. Today is the super awesome day

        Show
        Tony Trung Thanh Vo added a comment - Hi Paul, Adam (my colleague) and me upgraded groovy to version 1.7.9 and the fix is not included in 1.7.9 version. However, we do see the fix is included in version 1.8.0-rc-2. So it seems that you can deploy the version 1.7.9 again or you need to include this fix in 1.7.10. You also need to make sure that the release notes of v1.7.10 include the this bug (# GROOVY-4715 ), since this bug (# GROOVY-4715 ) is already included in the release-notes of v1.7.9 Thanks PS: Thanks for using our suggestion, Adam and especially I are so proud of this. Today is the super awesome day
        Tony Trung Thanh Vo made changes -
        Status Resolved [ 5 ] Reopened [ 4 ]
        Resolution Fixed [ 1 ]
        Hide
        Paul King added a comment -

        We always appreciate feedback and especially ones with well-considered suggestions. Thanks for spotting the problem with 1.7.9. The fix was targeted for that release but there was a glitch during the release and it didn't make it. We will probably do a 1.7.10 shortly to fix up the glitch and we'll update the version numbering in the issue to reflect the change. Thanks again!

        Show
        Paul King added a comment - We always appreciate feedback and especially ones with well-considered suggestions. Thanks for spotting the problem with 1.7.9. The fix was targeted for that release but there was a glitch during the release and it didn't make it. We will probably do a 1.7.10 shortly to fix up the glitch and we'll update the version numbering in the issue to reflect the change. Thanks again!
        Paul King made changes -
        Fix Version/s 1.7.10 [ 17229 ]
        Fix Version/s 1.7.9 [ 17164 ]
        Paul King made changes -
        Resolution Fixed [ 1 ]
        Status Reopened [ 4 ] Resolved [ 5 ]
        Paul King made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Mark Thomas made changes -
        Project Import Sun Apr 05 13:32:57 UTC 2015 [ 1428240777691 ]
        Mark Thomas made changes -
        Workflow jira [ 12733646 ] Default workflow, editable Closed status [ 12745452 ]
        Mark Thomas made changes -
        Project Import Mon Apr 06 02:11:23 UTC 2015 [ 1428286283443 ]
        Mark Thomas made changes -
        Workflow jira [ 12973399 ] Default workflow, editable Closed status [ 12974581 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Resolved Resolved
        1d 14h 41m 1 Paul King 09/Mar/11 07:39
        Resolved Resolved Reopened Reopened
        2d 6h 47m 1 Tony Trung Thanh Vo 11/Mar/11 14:26
        Reopened Reopened Resolved Resolved
        10h 16m 1 Paul King 12/Mar/11 00:43
        Resolved Resolved Closed Closed
        32d 13h 49m 1 Paul King 13/Apr/11 15:33

          People

          • Assignee:
            Paul King
            Reporter:
            Tony Trung Thanh Vo
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development