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

emails not escaped properly

    XMLWordPrintable

Details

    • Bug
    • Status: Resolved (View Workflow)
    • Minor
    • Resolution: Fixed
    • core
    • Fedora 17, Tomcat 6.0.35, Java 7, Latest Jenkins and plugins as of today

    Description

      On most pages, like these:

      https://ci.jenkins-ci.org/job/jenkins_rc_branch/changes
      https://ci.jenkins-ci.org/user/kohsuke/

      When the username is something like "Joe User <joe.user@example.com>", it is incorrectly escaped in the HTML as:

      Joe User &lt;joe.user@example.com>

      Then on the changes page for a specific build:

      https://ci.jenkins-ci.org/job/jenkins_rc_branch/300/changes

      A username like the above wouldn't be escaped at all, so would be "Joe User <joe.user@example.com>" in the HTML.

      Of course the proper way to escape this would be:

      Joe User &lt;joe.user@example.com&gt;

      We are using the mercurial plugin with rhodecode as the mercurial server, and I'm not sure if it's the job of the SCM plugin to escape these or whatever outputs the HTML, though I would think the latter.

      Attachments

        Issue Links

          Activity

            Code changed in jenkins
            User: Oleg Nenashev
            Path:
            core/src/main/java/hudson/Util.java
            core/src/test/java/hudson/FunctionsTest.java
            core/src/test/java/hudson/MarkupTextTest.java
            core/src/test/java/hudson/UtilTest.java
            core/src/test/java/hudson/console/UrlAnnotatorTest.java
            http://jenkins-ci.org/commit/jenkins/d158334f3b3dbed35d9f0ef042215dbf2076fc74
            Log:
            Merge pull request #1420 from daniel-beck/JENKINS-16184

            JENKINS-16184 Also escape greater-than character

            Compare: https://github.com/jenkinsci/jenkins/compare/aac8c239721e...d158334f3b3d

            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Oleg Nenashev Path: core/src/main/java/hudson/Util.java core/src/test/java/hudson/FunctionsTest.java core/src/test/java/hudson/MarkupTextTest.java core/src/test/java/hudson/UtilTest.java core/src/test/java/hudson/console/UrlAnnotatorTest.java http://jenkins-ci.org/commit/jenkins/d158334f3b3dbed35d9f0ef042215dbf2076fc74 Log: Merge pull request #1420 from daniel-beck/ JENKINS-16184 JENKINS-16184 Also escape greater-than character Compare: https://github.com/jenkinsci/jenkins/compare/aac8c239721e...d158334f3b3d
            dogfood dogfood added a comment -

            Integrated in jenkins_main_trunk #3733
            JENKINS-16184 Also escape greater-than character (Revision daacb02057cd702900f986a0a6867730ece13014)

            Result = SUCCESS
            daniel-beck : daacb02057cd702900f986a0a6867730ece13014
            Files :

            • core/src/test/java/hudson/UtilTest.java
            • core/src/test/java/hudson/FunctionsTest.java
            • core/src/test/java/hudson/MarkupTextTest.java
            • core/src/main/java/hudson/Util.java
            • core/src/test/java/hudson/console/UrlAnnotatorTest.java
            dogfood dogfood added a comment - Integrated in jenkins_main_trunk #3733 JENKINS-16184 Also escape greater-than character (Revision daacb02057cd702900f986a0a6867730ece13014) Result = SUCCESS daniel-beck : daacb02057cd702900f986a0a6867730ece13014 Files : core/src/test/java/hudson/UtilTest.java core/src/test/java/hudson/FunctionsTest.java core/src/test/java/hudson/MarkupTextTest.java core/src/main/java/hudson/Util.java core/src/test/java/hudson/console/UrlAnnotatorTest.java
            danielbeck Daniel Beck added a comment -

            A change related to this will be in Jenkins 1.586, so confirmation this resolves the issue would be great.

            danielbeck Daniel Beck added a comment - A change related to this will be in Jenkins 1.586, so confirmation this resolves the issue would be great.
            jglick Jesse Glick added a comment -

            Note that escaping > is not mandatory (nor is escaping " or ' in element content); it suffices to escape & and <.

            It sounds like there are two or three distinct issues here. The first was about > in the changes of a job, which is not really a bug (IMO) but which this core change may tighten up, if the Mercurial plugin is using the core function to do escaping, which I think it is. The second was also about > escaping, this time in the user page, certainly in core, which I guess Daniel checked.

            The third is about a lack of escaping in changes of a build, which is certainly a bug and unrelated to the first two; this bug might be in the Rhodecode section of the Mercurial plugin (which I cannot personally test changes to), or in a more generic section of the Mercurial plugin applicable also to e.g. Bitbucket and others, or in core and applicable to any SCM. Most likely it is the second case and a duplicate of JENKINS-5452, already fixed.

            jglick Jesse Glick added a comment - Note that escaping > is not mandatory (nor is escaping " or ' in element content); it suffices to escape & and < . It sounds like there are two or three distinct issues here. The first was about > in the changes of a job, which is not really a bug (IMO) but which this core change may tighten up, if the Mercurial plugin is using the core function to do escaping, which I think it is. The second was also about > escaping, this time in the user page, certainly in core, which I guess Daniel checked. The third is about a lack of escaping in changes of a build, which is certainly a bug and unrelated to the first two; this bug might be in the Rhodecode section of the Mercurial plugin (which I cannot personally test changes to), or in a more generic section of the Mercurial plugin applicable also to e.g. Bitbucket and others, or in core and applicable to any SCM. Most likely it is the second case and a duplicate of JENKINS-5452 , already fixed.
            jglick Jesse Glick added a comment -

            Assuming > is now also escaped, I guess this is fixed.

            jglick Jesse Glick added a comment - Assuming > is now also escaped, I guess this is fixed.

            People

              danielbeck Daniel Beck
              moparisthebest Travis Burtrum
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: