groovy
  1. groovy
  2. GROOVY-4715

StreamingMarkupBuilder can produce invalid xml

    Details

    • Type: Bug Bug
    • Status: Closed 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
    • Number of attachments :
      0

      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

        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 -
        Status Open [ 1 ] Resolved [ 5 ]
        Fix Version/s 1.8-rc-2 [ 17176 ]
        Fix Version/s 1.7.9 [ 17164 ]
        Resolution Fixed [ 1 ]
        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 -
        Resolution Fixed [ 1 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        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 -
        Status Reopened [ 4 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Paul King made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          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: