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

Stash Pullrequest Builder modify cookie handling

    Details

    • Similar Issues:

      Description

      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);
      

        Attachments

          Activity

          Hide
          jbochenski 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
          
          Show
          jbochenski 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
          Hide
          bran2k7 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.

          Show
          bran2k7 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.
          Hide
          jbochenski 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

          Show
          jbochenski 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
          Hide
          bran2k7 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.

          Show
          bran2k7 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.
          Hide
          bran2k7 Brandon Johnson added a comment -

          Jakub Bochenski 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.

          Show
          bran2k7 Brandon Johnson added a comment - Jakub Bochenski 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.
          Hide
          jbochenski 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

          Show
          jbochenski 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

            People

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

              Dates

              • Created:
                Updated: