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

Spaces in URL (from project name or view) cause invalid cookies to be saved

    Details

    • Similar Issues:

      Description

      One of our developers noticed that our PC-Lint warnings trend graph only showed builds from the last 30 days and that configuring the graph to show more had no effect.

      I noticed while debugging this that the cookie being generated to save the trend graph configuration had the wrong path set so that the project dashboard wasn't loading it. The path was being set to the URL of the trend graph's configuration page (ex. http://jenkins/project with spaces/warnings0/config). If I manually edited the cookie's path to point to the project dashboard (ex. http://jenkins/project with spaces/) it worked. The same issue exists if a project dashboard is opened from a view with spaces in the name (ex. http://jenkins/view/View with spaces/Project_with_underscores).

      Near as I can tell the source of the issue is in CookieHandler.java:

      CookieHandler.java
          /**
           * Sends a cookie with the specified value.
           *
           * @param requestAncestors
           *            the ancestors of the request
           * @param value
           *            the cookie value
           * @return the created cookie
           */
          public Cookie create(final List<Ancestor> requestAncestors, final String value) {
              Cookie cookie = new Cookie(name, value);       
      Ancestor ancestor = requestAncestors.get(requestAncestors.size() - ANCESTOR_PATH_PREFIX);
              cookie.setPath(ancestor.getUrl());
              cookie.setMaxAge(ONE_YEAR);
      
              return cookie;
          }
      

      I think this line of code is the culprit which returning the wrong URL for the cookie's path.

      Ancestor ancestor = requestAncestors.get(requestAncestors.size() - ANCESTOR_PATH_PREFIX);

        Attachments

          Issue Links

            Activity

            Hide
            drulli Ulli Hafner added a comment -

            I see. Since you already found the problematic statement: are you interested in providing a fix as pull request? I currently quite busy with other work...

            Show
            drulli Ulli Hafner added a comment - I see. Since you already found the problematic statement: are you interested in providing a fix as pull request? I currently quite busy with other work...
            Hide
            drulli Ulli Hafner added a comment -

            I finally managed it to reproduce this issue: Seems that even if the correct path is set with cookie.setPath(ancestor.getUrl()); it will be somehow changed in staplers ResponseImpl.

            This problem also occurs with the JUnit view that uses a cookie to decide which graph to use (failures only vs. all). So the problem must be somewhere in core (or any of the bundled libraries).

            Show
            drulli Ulli Hafner added a comment - I finally managed it to reproduce this issue: Seems that even if the correct path is set with cookie.setPath(ancestor.getUrl()); it will be somehow changed in staplers ResponseImpl. This problem also occurs with the JUnit view that uses a cookie to decide which graph to use (failures only vs. all). So the problem must be somewhere in core (or any of the bundled libraries).
            Hide
            evernat evernat added a comment -

            yes, cookie path is correct and http header "Set-Cookie" received by the browser seems also correct:

            Server	Jetty(winstone-2.9)
            Set-Cookie	TestResultAction_failureOnly=false;Path="/job/project%20with%20spaces";Expires=Mon, 09-Jan-2017 11:27:25 GMT
            

            But the cookie is not saved correctly by the browser (Chrome and Firefox), certainly because of quotes in the Path.

            It happens that Jenkins uses Jetty 8.1.13 (https://github.com/jenkinsci/winstone/blob/master/pom.xml#L221)
            which is buggy (see here and here)
            It will be needed to upgrade Jetty to 9.0.6 or later.

            Show
            evernat evernat added a comment - yes, cookie path is correct and http header "Set-Cookie" received by the browser seems also correct: Server Jetty(winstone-2.9) Set-Cookie TestResultAction_failureOnly= false ;Path= "/job/project%20with%20spaces" ;Expires=Mon, 09-Jan-2017 11:27:25 GMT But the cookie is not saved correctly by the browser (Chrome and Firefox), certainly because of quotes in the Path. It happens that Jenkins uses Jetty 8.1.13 ( https://github.com/jenkinsci/winstone/blob/master/pom.xml#L221 ) which is buggy (see here and here ) It will be needed to upgrade Jetty to 9.0.6 or later.
            Hide
            drulli Ulli Hafner added a comment -

            Thanks for finding the actual reason!

            Show
            drulli Ulli Hafner added a comment - Thanks for finding the actual reason!
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Ulli Hafner
            Path:
            src/main/java/org/jenkinsci/test/acceptance/plugins/analysis_core/GraphConfigurationView.java
            src/test/java/plugins/AbstractAnalysisTest.java
            http://jenkins-ci.org/commit/acceptance-test-harness/b3691320192217d8a2d5e7e60e65119f3b13ea7f
            Log:
            Added a test case that checks trend configuration via cookie.

            Basically a test the checks that JENKINS-25917 and JENKINS-32377 are resolved in Jenkins 2.1.

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Ulli Hafner Path: src/main/java/org/jenkinsci/test/acceptance/plugins/analysis_core/GraphConfigurationView.java src/test/java/plugins/AbstractAnalysisTest.java http://jenkins-ci.org/commit/acceptance-test-harness/b3691320192217d8a2d5e7e60e65119f3b13ea7f Log: Added a test case that checks trend configuration via cookie. Basically a test the checks that JENKINS-25917 and JENKINS-32377 are resolved in Jenkins 2.1.
            Hide
            drulli Ulli Hafner added a comment -

            Fixed in Jenkins 2.x.

            Show
            drulli Ulli Hafner added a comment - Fixed in Jenkins 2.x.
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Ulli Hafner
            Path:
            src/test/java/plugins/AbstractAnalysisTest.java
            http://jenkins-ci.org/commit/acceptance-test-harness/75c8f774c6474f5bfb6a67b256b4acff57a977fc
            Log:
            JENKINS-25917 JENKINS-32377 Let cookie test case run with 2.x only.

            Actually use a job name in the cookie test case that contains spaces.

            Compare: https://github.com/jenkinsci/acceptance-test-harness/compare/00068c6c59b0...75c8f774c647

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Ulli Hafner Path: src/test/java/plugins/AbstractAnalysisTest.java http://jenkins-ci.org/commit/acceptance-test-harness/75c8f774c6474f5bfb6a67b256b4acff57a977fc Log: JENKINS-25917 JENKINS-32377 Let cookie test case run with 2.x only. Actually use a job name in the cookie test case that contains spaces. Compare: https://github.com/jenkinsci/acceptance-test-harness/compare/00068c6c59b0...75c8f774c647

              People

              • Assignee:
                ci_jenkinsci_org Kohsuke Kawaguchi
                Reporter:
                schambda David Schamber
              • Votes:
                2 Vote for this issue
                Watchers:
                6 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: