[8u] RFR 8236645: JDK 8u231 introduces a regression with incompatible handling of XML messages

[Severin Gehwolf]

Hi Elliott,

On Wed, 2020-08-19 at 12:54 -0400, Elliott Baron wrote:
> Hi,
> 
> I'd like to request a review of 8236645 for 8u. 8236645 is a bug fix for 
> Oracle JDK 8 that I have re-implemented for OpenJDK 8u.
> 
> Original fix:
> https://bugs.openjdk.java.net/browse/JDK-8236645
> CSR:
> https://bugs.openjdk.java.net/browse/JDK-8237765
> 
> This fixes a regression introduced by 8177334 and 8217878 by introducing 
> a new system property 
> "com.sun.org.apache.xml.internal.security.lineFeedOnly", which gives 
> Base64 encodings in XML digital signatures the same line-ending format 
> as before these changes were made. The CSR outlined the behaviour of 
> this system property and how it was originally implemented.
> 
> I added a new test file for this change, based on test cases in 
> javax/xml/crypto/dsig/GenerationTests. This allows us to run the test 
> with the property enabled and disabled, and also doesn't interfere with 
> future backports affecting GenerationTests.
> 
> 8u webrev:
> https://cr.openjdk.java.net/~ebaron/jdk8u/JDK-8236645/webrev.00/

The CSR states:
"""
This new property has no effect on default behavior nor when
com.sun.org.apache.xml.internal.security.ignoreLineBreaks property is
set.
"""

If I read your patch right, then it would not work as specified when
both com.sun.org.apache.xml.internal.security.ignoreLineBreaks *and* 
com.sun.org.apache.xml.internal.security.lineFeedOnly are both set:

 451     public static String encodeToString(byte[] bytes) {
 452         if (lineFeedOnly) {
 453             return LF_ENCODER.encodeToString(bytes);
 454         }
 455         if (ignoreLineBreaks) {
 456             return Base64.getEncoder().encodeToString(bytes);
 457         }
 458         return Base64.getMimeEncoder().encodeToString(bytes);
 459     }

Lines 452-454 should move after line 457.

test/javax/xml/crypto/dsig/LineFeedOnlyTest.java

  63 /* @test id=LF
  64  * @bug 8236645
  65  * @summary Test "lineFeedOnly" property prevents CR in Base64 encoded output
  66  * @run main/othervm/timeout=300 -Dcom.sun.org.apache.xml.internal.security.lineFeedOnly=true LineFeedOnlyTest
  67  */
  68 
  69 /* @test id=CRLF
  70  * @bug 8236645
  71  * @summary Test "lineFeedOnly" property prevents CR in Base64 encoded output
  72  * @run main/othervm/timeout=300 -Dcom.sun.org.apache.xml.internal.security.lineFeedOnly=false LineFeedOnlyTest
  73  */

Couldn't we simplify this by using just multiple @run tags?

How about this?

/* @test
 * @bug 8236645
 * @summary Test "lineFeedOnly" property prevents CR in Base64 encoded output
 * @run main/othervm/timeout=300 -Dcom.sun.org.apache.xml.internal.security.lineFeedOnly=false LineFeedOnlyTest
 * @run main/othervm/timeout=300 -Dcom.sun.org.apache.xml.internal.security.lineFeedOnly=true LineFeedOnlyTest
 */

Thanks,
Severin