Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-71139

XStream2 unable to round-trip ASCII NUL in stdout/stderr in junitResult.xml

      It appears that the switch from KXml2Driver to StandardStaxDriver in JENKINS-69129 (included in 2.387.3-rc33338.349f385d65d2 as well as weeklies) broke the ability to round-trip ASCII NUL characters in text. Prior to the change, this character was written as � and (whether correct or not) read back successfully. After the change, it is still written the same way, but cannot be read.

      Seems unlikely to affect many users, though it may affect the Jenkins project itself: for example

      mvn -pl test surefire:test -Dtest=ExecutorTest#disconnectCause_WithoutTrace -Dmaven.test.redirectTestOutputToFile=true
      

      causes TEST-hudson.model.ExecutorTest.xml to include

      <===[JENKINS REMOTING CAPACITY]===>&amp#0;&amp#0;&amp#0;channel started
      

      which looks to be bogus output from Surefire but which does not trigger a problem, whereas hudson.model.ExecutorTest-output.txt contains

      <===[JENKINS REMOTING CAPACITY]===>^@^@^@channel started
      

      (with ^@ standing in for actual ASCII NUL for display). In case the NUL makes it into a stdout field in junitResult.xml, the symptom is that the junit step publishes test results normally (setting the build status, sending checks to GitHub, etc.) but if you visit the testReport/ display in the GUI, it claims there are no tests found (after printing a stack trace to the system log about a parse error). Other plugins looking up test results (such as parallel-test-executor) can also be affected.

          [JENKINS-71139] XStream2 unable to round-trip ASCII NUL in stdout/stderr in junitResult.xml

          Jesse Glick added a comment -

          As far as I can tell, it is not possible to encode NUL in XML at all, in which case it would suffice to ensure that the junit plugin does not attempt to do so (and the core PR could either be closed or amended to just demonstrate the expected behavior).

          Jesse Glick added a comment - As far as I can tell, it is not possible to encode NUL in XML at all, in which case it would suffice to ensure that the junit plugin does not attempt to do so (and the core PR could either be closed or amended to just demonstrate the expected behavior).

          Noting that the proposed PRs do not fix the symptoms, rather avoid the condition that caused it (by failing fast serialization of invalid xml, and fixing the junit plugin to not produce invalid xml).

          This means test reports of existing builds produced using an older junit plugin will still be unreadable with the new version, unless https://github.com/jenkinsci/jenkins/pull/7778 is eventually reverted.

          Vincent Latombe added a comment - Noting that the proposed PRs do not fix the symptoms, rather avoid the condition that caused it (by failing fast serialization of invalid xml, and fixing the junit plugin to not produce invalid xml). This means test reports of existing builds produced using an older junit plugin will still be unreadable with the new version, unless https://github.com/jenkinsci/jenkins/pull/7778 is eventually reverted.

          Jesse Glick added a comment -

          Good point; noted in the core “proposed upgrade guidelines”.

          Kris Stern (not sure of your Jira id): I am leaving this marked lts-candidate though I do not think it is necessary to delay 2.387.3 for it, given the junit fix, so long as a mention is made in the LTS changelog / upgrade guidelines.

          Jesse Glick added a comment - Good point; noted in the core “proposed upgrade guidelines”. Kris Stern (not sure of your Jira id): I am leaving this marked lts-candidate though I do not think it is necessary to delay 2.387.3 for it, given the junit fix, so long as a mention is made in the LTS changelog / upgrade guidelines.

          Mark Waite added a comment -

          kmartens27 can you include a mention of this in the LTS 2.387.3 changelog and upgrade guide?

          Mark Waite added a comment - kmartens27 can you include a mention of this in the LTS 2.387.3 changelog and upgrade guide?

          Kevin Martens added a comment -

          markewaite sure thing!

          Kevin Martens added a comment - markewaite sure thing!

          Kris Stern added a comment - - edited

          jglick Just seeubg your comment to me now. I have just made a backporting PR for this. For details, please see: https://github.com/jenkinsci/jenkins/pull/7885

          Kris Stern added a comment - - edited jglick Just seeubg your comment to me now. I have just made a backporting PR for this. For details, please see: https://github.com/jenkinsci/jenkins/pull/7885 . 

          Mark Waite added a comment -

          krisstern I don't think that we need to backport https://github.com/jenkinsci/jenkins/pull/7875 to 2.387.3 LTS. It may need to be backported to 2.401.1. Kevin will add a note to the upgrade guide that users need to update to most recent release of junit plugin

          Mark Waite added a comment - krisstern I don't think that we need to backport https://github.com/jenkinsci/jenkins/pull/7875 to 2.387.3 LTS. It may need to be backported to 2.401.1. Kevin will add a note to the upgrade guide that users need to update to most recent release of junit plugin

          Kris Stern added a comment -

          markewaite Okay, maybe I should close the PR then? 

          Kris Stern added a comment - markewaite Okay, maybe I should close the PR then? 

            jglick Jesse Glick
            jglick Jesse Glick
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

              Created:
              Updated:
              Resolved: