• Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Minor Minor
    • core
    • None
    • jenkins-2.196

      While fixing JENKINS-59231, I noticed that Jenkins RSS 2.0 and Atom XML feeds have malformed XML.

      Inserting an interactive break and fetching the URL, we see that Jenkins is returning malformed XML:

      $ curl http://localhost:43969/jenkins/rssAll
      <?xml version="1.0" encoding="UTF-8"?>
      <feed xmlns="http://www.w3.org/2005/Atom"><title>All all builds</title><link rel="alternate" type="text/html" href="http://localhost:43969/jenkins/"><updated>2001-01-01T00:00:00Z</updated><author><name>Jenkins Server</name></author><id>urn:uuid:903deee0-7bfa-11db-9fe1-0800200c9a66</id></feed>
      

      Note that the <link> tag is not terminated, despite the fact that it is terminated in the template:

      $ grep link core/src/main/resources/hudson/atom.jelly
      <link rel="alternate" type="text/html" href="${app.rootUrl}${url}"/>
      <link rel="alternate" type="text/html" href="${app.rootUrl}${h.encode(adapter.getEntryUrl(e))}"/>
      

      So Jelly must be screwing this up when filling in the template. Where is that happening? Stepping through the code in the debugger reveals this stack trace:

      createXMLOutput:68, DefaultScriptInvoker (org.kohsuke.stapler.jelly)
      invokeScript:51, DefaultScriptInvoker (org.kohsuke.stapler.jelly)
      execute:56, ScriptInvoker (org.kohsuke.stapler.jelly)
      execute:43, ScriptInvoker (org.kohsuke.stapler.jelly)
      forward:95, ScriptRequestDispatcher (org.kohsuke.stapler)
      forwardToRss:91, RSS (hudson.model)
      

      CreateXMLOutput looks like this:

      protected XMLOutput createXMLOutput(StaplerRequest req, StaplerResponse rsp, Script script, Object it) throws IOException {
          // TODO: make XMLOutput auto-close OutputStream to avoid leak
          String ct = rsp.getContentType();
          XMLOutput output;
          if (ct != null && !ct.startsWith("text/html")) {
              output = XMLOutput.createXMLOutput(createOutputStream(req, rsp, script, it));
          } else {
              output = HTMLWriterOutput.create(createOutputStream(req, rsp, script, it));
      
          }
          return output;
      }
      

      As you can see, there are two paths, one for XML output and one for HTML output. Stepping through this, I see that for RSS feeds rsp.getContentType() is null, so we go through the (incorrect) HTML output path. This results in malformed XML.

      The solution is to set the content type appropriately on the response in RSS#forwardToRss. I stole the existing content types from core/src/main/resources/hudson/atom.jelly and core/src/main/resources/hudson/rss20.jelly. With this set, we now use the XML output path in RSS#forwardToRss and produce well-formed XML:

      $ curl  http://localhost:43831/jenkins/rssAll
      <?xml version="1.0" encoding="UTF-8"?>
      <feed xmlns="http://www.w3.org/2005/Atom"><title>All all builds</title><link rel="alternate" type="text/html" href="http://localhost:43831/jenkins/"></link><updated>2019-09-13T20:13:05Z</updated><author><name>Jenkins Server</name></author><id>urn:uuid:903deee0-7bfa-11db-9fe1-0800200c9a66</id><entry><title>test0 #1 (stable)</title><link rel="alternate" type="text/html" href="http://localhost:43831/jenkins/job/test0/1/"></link><id>tag:hudson.dev.java.net,2019:test0:1</id><published>2019-09-13T20:13:05Z</published><updated>2019-09-13T20:13:05Z</updated></entry></feed>
      

          [JENKINS-59393] Fix malformed XML in Atom and RSS 2.0 feeds

          Wadeck Follonier added a comment - - edited

          basil very interesting piece of investigation, thank you very much for the contribution there  Do you plan to provide a PR for this? Thank you for the PR

          We have a task in our backlog to work on the test coverage, so please, tell us if you are working on anything else in that area to avoid duplicate work

          Wadeck Follonier added a comment - - edited basil very interesting piece of investigation, thank you very much for the contribution there   Do you plan to provide a PR for this?  Thank you for the PR We have a task in our backlog to work on the test coverage, so please, tell us if you are working on anything else in that area to avoid duplicate work

          Basil Crow added a comment -

          No problem! I was bitten by this bug myself, so I wanted to give back by helping to improve the situation in the future.

          We have a task in our backlog to work on the test coverage, so please, tell us if you are working on anything else in that area to avoid duplicate work

          I don't have any plans to work on this further at the moment, but this fix should make it easier for anyone else to improve test coverage in the future by allowing such changes to be integrated without being marked with @Ignore.

          Basil Crow added a comment - No problem! I was bitten by this bug myself, so I wanted to give back by helping to improve the situation in the future. We have a task in our backlog to work on the test coverage, so please, tell us if you are working on anything else in that area to avoid duplicate work I don't have any plans to work on this further at the moment, but this fix should make it easier for anyone else to improve test coverage in the future by allowing such changes to be integrated without being marked with @Ignore .

            Unassigned Unassigned
            basil Basil Crow
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated:
              Resolved: