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

Stash Pullrequest Builder modify cookie handling

XMLWordPrintable

      Using the default RFC_2109 cookie handling from httpclient results in logs full of this warning.

      Mar 22, 2019 7:46:25 AM stashpullrequestbuilder.stashpullrequestbuilder.StashRepository getTargetPullRequests
INFO: Fetch PullRequests (dashboard-tf-release).
Mar 22, 2019 7:46:25 AM org.apache.commons.httpclient.HttpMethodBase processCookieHeaders
WARNING: Cookie rejected: "$Version=0; crowd.token_key=**; $Path=/; $Domain=domain.com". Domain attribute "domain.com" violates RFC 2109: domain must start with a dot

      something like this in stashapiclient should work

      httpGet.getParams().setParameter(ClientPNames.COOKIE_POLICY, CookiePolicy.BROWSER_COMPATIBILITY);

          [JENKINS-56685] Stash Pullrequest Builder modify cookie handling

          Jakub Bochenski added a comment -

          Can you please try the latest plugin version. I'm not seeing this:

          $ grep 'processCookieHeaders' /var/log/jenkins/jenkins.log || echo nothing
nothing

          Jakub Bochenski added a comment - Can you please try the latest plugin version. I'm not seeing this: $ grep 'processCookieHeaders' / var /log/jenkins/jenkins.log || echo nothing nothing

          Brandon Johnson added a comment -

          I updated the plugin and still have it filling up the logs.
          I marked this as an improvement because this isn't necessarily an issue with the plugin. It's the bitbucket server sending back a bad cookie that violates RFC 2109. I am working internally (it's an enterprise, on-site server) to try and get our bitbucket instance updated or patched to fix this.
          I am asking if it's possible to see if this team wouldn't be inclined to reduce the cookie handling from the very strict RFC 2109 to BROWSER_COMPATIBILITY. Lots of sites and indeed chrome and firefox handle cookies this way.
          RFC_2109 is the default cookie handling if nothing else is specified.

          Brandon Johnson added a comment - I updated the plugin and still have it filling up the logs. I marked this as an improvement because this isn't necessarily an issue with the plugin. It's the bitbucket server sending back a bad cookie that violates RFC 2109. I am working internally (it's an enterprise, on-site server) to try and get our bitbucket instance updated or patched to fix this. I am asking if it's possible to see if this team wouldn't be inclined to reduce the cookie handling from the very strict RFC 2109 to BROWSER_COMPATIBILITY. Lots of sites and indeed chrome and firefox handle cookies this way. RFC_2109 is the default cookie handling if nothing else is specified.

          Jakub Bochenski added a comment - - edited

          We are currently working on adding a global configuration for the plugin. We could add an option to override the cookie handling there. Would that be good enough for you?

          BTW If you need this change faster you please file a pull request simply changing it. You will then be able to install the PR version via the experimental update center

          Jakub Bochenski added a comment - - edited We are currently working on adding a global configuration for the plugin. We could add an option to override the cookie handling there. Would that be good enough for you? BTW If you need this change faster you please file a pull request simply changing it. You will then be able to install the PR version via the experimental update center

          Brandon Johnson added a comment - - edited

          Jakub, having that in the global configuration would work great. I'm not too good with java, i've been trying to follow it along to see where best to change it but I haven't found it. I do see where I believe I could just change it so it only does BROWSER_COMPATIBILITY but I didn't know if that would be an ok PR to make.

          Brandon Johnson added a comment - - edited Jakub, having that in the global configuration would work great. I'm not too good with java, i've been trying to follow it along to see where best to change it but I haven't found it. I do see where I believe I could just change it so it only does BROWSER_COMPATIBILITY but I didn't know if that would be an ok PR to make.

          Brandon Johnson added a comment -

          jbochenski If you have a link to a branch or some commits that have the global configuration stuff I can take a look at it and see if I can't make a PR for adding this part.

          Brandon Johnson added a comment - jbochenski If you have a link to a branch or some commits that have the global configuration stuff I can take a look at it and see if I can't make a PR for adding this part.

          Jakub Bochenski added a comment -

          The work on global configuration is only planned still. What can be done now is: create a PR changing this directly. This build will then be available through the Experimental Update Center

          Jakub Bochenski added a comment - The work on global configuration is only planned still. What can be done now is: create a PR changing this directly. This build will then be available through the Experimental Update Center

            jbochenski Jakub Bochenski
            bran2k7 Brandon Johnson
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated: