[JENKINS-22342] Gitweb URL can be mangled (if it doesn't contain a path?) - Jenkins Jira

XML

Word

Printable

Details

Type: Bug
Status: Closed (View Workflow)
Priority: Major
Resolution: Fixed
Component/s: git-plugin
Labels:
None

Similar Issues:

Show

Description

With git plugin at b2a731e62272c93a60f7f45d9d3c5d3b1e57fa11 (so that repository browser URLs are saved) then the URL to our repository https://gerrit.company.local/gitweb?p=repo.git gets mangled to https://gerrit.company.local/gitweb/?p=repo.git. Note the extra slash after gitweb.

I believe this is introduced by the call to normalizeToEndWithSlash() which was done for some repository types prior to https://github.com/jenkinsci/git-plugin/commit/7abfafcf58addbce2c268dbfed1910a90d4fe2a3 but is now done for all repository types in GitRepositoryBrowser. This is wrong for our particular usage. I'm happy to create a patch to fix this for all repo types that didn't formerly do this if some knows that this is the right way forward.

Attachments

Activity

Ascending order - Click to sort in descending order

Alan Clucas created issue - 2014-03-25 16:55

Mark Waite added a comment - 2014-03-26 11:58

Can you explain why the trailing slash does not work in your case? It works for me with gitweb. Is the problem that it is now done for all repository types?

If that is the problem, then it seems reasonable that you create a patch with a test that asserts the failing case is fixed.

Mark Waite added a comment - 2014-03-26 11:58 Can you explain why the trailing slash does not work in your case? It works for me with gitweb. Is the problem that it is now done for all repository types? If that is the problem, then it seems reasonable that you create a patch with a test that asserts the failing case is fixed.

Alan Clucas added a comment - 2014-03-26 12:38

There is no directory (or virtual directory) called gitweb, it is just a cgi script that is only invoked by gitweb with no trailing slash on our config of apache. I'm sure I could fix this on the apache side.

From a higher standpoint - the URL is getting changed unnecessarily, and used to work in version 2.0.1, hence thinking the fix is really something that should be done on the jenkins plugin side.

I'm working on getting a presentable fix, but it won't be done for a week as I'm away now.

Alan Clucas added a comment - 2014-03-26 12:38 There is no directory (or virtual directory) called gitweb, it is just a cgi script that is only invoked by gitweb with no trailing slash on our config of apache. I'm sure I could fix this on the apache side. From a higher standpoint - the URL is getting changed unnecessarily, and used to work in version 2.0.1, hence thinking the fix is really something that should be done on the jenkins plugin side. I'm working on getting a presentable fix, but it won't be done for a week as I'm away now.

Alan Clucas added a comment - 2014-03-26 16:20

Did get enough time to do this today, so have submitted a pull request for review https://github.com/jenkinsci/git-plugin/pull/217

Alan Clucas added a comment - 2014-03-26 16:20 Did get enough time to do this today, so have submitted a pull request for review https://github.com/jenkinsci/git-plugin/pull/217

Alan Clucas added a comment - 2014-04-04 09:57

Can I persuade someone to look at this please?

Alan Clucas added a comment - 2014-04-04 09:57 Can I persuade someone to look at this please?

Mark Waite added a comment - 2014-04-04 11:28

I hope to look at it within the next 7 days. I'd prefer that Nicolas be the one to evaluate it, since he made the most recent code changes in that area, but he's been busy in other areas.

Mark Waite added a comment - 2014-04-04 11:28 I hope to look at it within the next 7 days. I'd prefer that Nicolas be the one to evaluate it, since he made the most recent code changes in that area, but he's been busy in other areas.

Adam Pryce added a comment - 2014-04-07 22:00

We are having the same problem. Our gitweb (through Gerrit with an Apache proxy) fails to find the resource with the additional slash.

Adam Pryce added a comment - 2014-04-07 22:00 We are having the same problem. Our gitweb (through Gerrit with an Apache proxy) fails to find the resource with the additional slash.

SCM/JIRA link daemon added a comment - 2014-04-09 13:54

Code changed in jenkins
User: Alan Clucas
Path:
src/main/java/hudson/plugins/git/browser/BitbucketWeb.java
src/main/java/hudson/plugins/git/browser/CGit.java
src/main/java/hudson/plugins/git/browser/FisheyeGitRepositoryBrowser.java
src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java
src/main/java/hudson/plugins/git/browser/GitLab.java
src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java
src/main/java/hudson/plugins/git/browser/GitWeb.java
src/main/java/hudson/plugins/git/browser/GithubWeb.java
src/main/java/hudson/plugins/git/browser/GitoriousWeb.java
src/main/java/hudson/plugins/git/browser/KilnGit.java
src/main/java/hudson/plugins/git/browser/Phabricator.java
src/main/java/hudson/plugins/git/browser/RedmineWeb.java
src/main/java/hudson/plugins/git/browser/RhodeCode.java
src/main/java/hudson/plugins/git/browser/Stash.java
src/main/java/hudson/plugins/git/browser/ViewGitWeb.java
http://jenkins-ci.org/commit/git-plugin/962e244a72dff3a79d1643d5107faa33f0ff74d8
Log:
Fix for ~~JENKINS-22342~~

Only normalize with slash certain repository types.
Should match behaviour prior to
7abfafcf58addbce2c268dbfed1910a90d4fe2a3

SCM/JIRA link daemon added a comment - 2014-04-09 13:54 Code changed in jenkins User: Alan Clucas Path: src/main/java/hudson/plugins/git/browser/BitbucketWeb.java src/main/java/hudson/plugins/git/browser/CGit.java src/main/java/hudson/plugins/git/browser/FisheyeGitRepositoryBrowser.java src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java src/main/java/hudson/plugins/git/browser/GitLab.java src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java src/main/java/hudson/plugins/git/browser/GitWeb.java src/main/java/hudson/plugins/git/browser/GithubWeb.java src/main/java/hudson/plugins/git/browser/GitoriousWeb.java src/main/java/hudson/plugins/git/browser/KilnGit.java src/main/java/hudson/plugins/git/browser/Phabricator.java src/main/java/hudson/plugins/git/browser/RedmineWeb.java src/main/java/hudson/plugins/git/browser/RhodeCode.java src/main/java/hudson/plugins/git/browser/Stash.java src/main/java/hudson/plugins/git/browser/ViewGitWeb.java http://jenkins-ci.org/commit/git-plugin/962e244a72dff3a79d1643d5107faa33f0ff74d8 Log: Fix for JENKINS-22342 Only normalize with slash certain repository types. Should match behaviour prior to 7abfafcf58addbce2c268dbfed1910a90d4fe2a3

SCM/JIRA link daemon added a comment - 2014-04-09 13:54

Code changed in jenkins
User: Alan Clucas
Path:
src/test/java/hudson/plugins/git/browser/GitWebTest.java
http://jenkins-ci.org/commit/git-plugin/1d2d879b56c8a2a1962c53273799a31c41ae2730
Log:
Add tests for gitweb repo broswer

Add tests for gitweb, which include cover for ~~JENKINS-22342~~

SCM/JIRA link daemon added a comment - 2014-04-09 13:54 Code changed in jenkins User: Alan Clucas Path: src/test/java/hudson/plugins/git/browser/GitWebTest.java http://jenkins-ci.org/commit/git-plugin/1d2d879b56c8a2a1962c53273799a31c41ae2730 Log: Add tests for gitweb repo broswer Add tests for gitweb, which include cover for JENKINS-22342

Mark Waite added a comment - 2014-04-09 13:58

Will be available in the next git-plugin release after 2.2.0

Mark Waite added a comment - 2014-04-09 13:58 Will be available in the next git-plugin release after 2.2.0

Mark Waite made changes - 2014-04-09 13:58

Field	Original Value	New Value
Resolution		Fixed [ 1 ]
Status	Open [ 1 ]	Resolved [ 5 ]

Mark Waite added a comment - 2014-04-10 11:15

The change which is now on the tip of the git-plugin also breaks the URL for githubweb. It is important that the githubweb URL (and other URLs) work before the next release of the git-plugin.

Mark Waite added a comment - 2014-04-10 11:15 The change which is now on the tip of the git-plugin also breaks the URL for githubweb. It is important that the githubweb URL (and other URLs) work before the next release of the git-plugin.

Mark Waite made changes - 2014-04-10 11:15

Assignee	Nicolas De Loof [ ndeloof ]	Mark Waite [ markewaite ]
Resolution	Fixed [ 1 ]
Status	Resolved [ 5 ]	Reopened [ 4 ]

Alan Clucas added a comment - 2014-04-10 11:48

What kind of URLs does it break - do you have any examples? It's passing the test cases, so I'm not sure what needs fixing.

Alan Clucas added a comment - 2014-04-10 11:48 What kind of URLs does it break - do you have any examples? It's passing the test cases, so I'm not sure what needs fixing.

Mark Waite added a comment - 2014-04-10 11:58 - edited

It appears this change breaks the previously working githubweb links until the next time the job is saved. We can't release the plugin with that behavior because we can't force our users to save the job definition in order to restore something that was working previously.

The steps I took to show the problem were:

Define a job using GitHub URL https://github.com/jenkinsci/git-plugin and GitHub repository browser URL https://github.com/jenkinsci/git-plugin
Save that job
Let the job run when changes arrive on that GitHub project
Upgrade the git plugin to the latest unreleased version
Open the job and click one of the recent changes links to githubweb. A slash will be missing from the githubweb URL. Instead of https://github.com/jenkinsci/git-plugin/commit/fb12fed4c1c8af11db68ec236961401d3d11eb7b it will be https://github.com/jenkinsci/git-plugincommit/fb12fed4c1c8af11db68ec236961401d3d11eb7b
Configure the job
Save that configuration without making any changes
Click one of the recent change links to githubweb. A slash will be inserted correctly into the githubweb URL. The link fb12fed works

Can you propose a change which will fix that case? If you don't have time for that, I'll reopen the bug and revert this change so that plugin releases of other fixes and improvements are not blocked.

Mark Waite added a comment - 2014-04-10 11:58 - edited It appears this change breaks the previously working githubweb links until the next time the job is saved. We can't release the plugin with that behavior because we can't force our users to save the job definition in order to restore something that was working previously. The steps I took to show the problem were: Define a job using GitHub URL https://github.com/jenkinsci/git-plugin and GitHub repository browser URL https://github.com/jenkinsci/git-plugin Save that job Let the job run when changes arrive on that GitHub project Upgrade the git plugin to the latest unreleased version Open the job and click one of the recent changes links to githubweb. A slash will be missing from the githubweb URL. Instead of https://github.com/jenkinsci/git-plugin/commit/fb12fed4c1c8af11db68ec236961401d3d11eb7b it will be https://github.com/jenkinsci/git-plugincommit/fb12fed4c1c8af11db68ec236961401d3d11eb7b Configure the job Save that configuration without making any changes Click one of the recent change links to githubweb. A slash will be inserted correctly into the githubweb URL. The link fb12fed works Can you propose a change which will fix that case? If you don't have time for that, I'll reopen the bug and revert this change so that plugin releases of other fixes and improvements are not blocked.

Mark Waite added a comment - 2014-04-10 12:11

When I compare the job definitions, one point that may be relevant is that the jobs with the failing URL's do not have any normalizeUrl tag inserted into the job definition, while those jobs which have been edited get a normalizeUrl tag with the value "true".

Mark Waite added a comment - 2014-04-10 12:11 When I compare the job definitions, one point that may be relevant is that the jobs with the failing URL's do not have any normalizeUrl tag inserted into the job definition, while those jobs which have been edited get a normalizeUrl tag with the value "true".

Alan Clucas added a comment - 2014-04-10 12:18

Ok, that's me not understanding how all this stuff works. I can confirm that I'm sure my change breaks it. I'll have a look at fixing it today

Alan Clucas added a comment - 2014-04-10 12:18 Ok, that's me not understanding how all this stuff works. I can confirm that I'm sure my change breaks it. I'll have a look at fixing it today

Mark Waite added a comment - 2014-04-10 12:21

Thanks very much. I've alerted Nicolas to not release a new version of the plugin until we either have a fix or we have reverted the change.

Mark Waite added a comment - 2014-04-10 12:21 Thanks very much. I've alerted Nicolas to not release a new version of the plugin until we either have a fix or we have reverted the change.

Alan Clucas added a comment - 2014-04-11 09:48

Sent a pull request with a fix. Tested against Github and Gitweb. Now understand more than I want to about java serialization.

Alan Clucas added a comment - 2014-04-11 09:48 Sent a pull request with a fix. Tested against Github and Gitweb. Now understand more than I want to about java serialization.

Mark Waite added a comment - 2014-04-11 13:00

Thanks. I've started the automated evaluation of the fix in my various environments. Would you consider modifying your changes to minimize your diffs from git-2.2.0? I've submitted a pull request to your repository with my proposed changes for you to evaluate.

Mark Waite added a comment - 2014-04-11 13:00 Thanks. I've started the automated evaluation of the fix in my various environments. Would you consider modifying your changes to minimize your diffs from git-2.2.0? I've submitted a pull request to your repository with my proposed changes for you to evaluate.

Alan Clucas added a comment - 2014-04-11 13:31

I think I've done the right thing to accept them.

Alan Clucas added a comment - 2014-04-11 13:31 I think I've done the right thing to accept them.

Alan Clucas added a comment - 2014-04-11 14:54

Just wondered if you wanted them squashing down?

Alan Clucas added a comment - 2014-04-11 14:54 Just wondered if you wanted them squashing down?

Mark Waite added a comment - 2014-04-11 15:35

I would prefer if you squashed them down so that there isn't an intermediate state where the constructor is missing.

Mark Waite added a comment - 2014-04-11 15:35 I would prefer if you squashed them down so that there isn't an intermediate state where the constructor is missing.

SCM/JIRA link daemon added a comment - 2014-04-12 12:17

Code changed in jenkins
User: Alan Clucas
Path:
src/main/java/hudson/plugins/git/browser/BitbucketWeb.java
src/main/java/hudson/plugins/git/browser/CGit.java
src/main/java/hudson/plugins/git/browser/FisheyeGitRepositoryBrowser.java
src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java
src/main/java/hudson/plugins/git/browser/GitLab.java
src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java
src/main/java/hudson/plugins/git/browser/GitWeb.java
src/main/java/hudson/plugins/git/browser/GithubWeb.java
src/main/java/hudson/plugins/git/browser/GitoriousWeb.java
src/main/java/hudson/plugins/git/browser/KilnGit.java
src/main/java/hudson/plugins/git/browser/Phabricator.java
src/main/java/hudson/plugins/git/browser/RedmineWeb.java
src/main/java/hudson/plugins/git/browser/RhodeCode.java
src/main/java/hudson/plugins/git/browser/Stash.java
src/main/java/hudson/plugins/git/browser/ViewGitWeb.java
http://jenkins-ci.org/commit/git-plugin/510337b35a4665f1d938d35215801bd9d354554e
Log:
~~JENKINS-22342~~ ensure URL normalization correct

Previous attempts to fix this stored the normalization flag
in the serialized repository browser. Moved to be a function
call to avoid the deserializer getting involved

SCM/JIRA link daemon added a comment - 2014-04-12 12:17 Code changed in jenkins User: Alan Clucas Path: src/main/java/hudson/plugins/git/browser/BitbucketWeb.java src/main/java/hudson/plugins/git/browser/CGit.java src/main/java/hudson/plugins/git/browser/FisheyeGitRepositoryBrowser.java src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java src/main/java/hudson/plugins/git/browser/GitLab.java src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java src/main/java/hudson/plugins/git/browser/GitWeb.java src/main/java/hudson/plugins/git/browser/GithubWeb.java src/main/java/hudson/plugins/git/browser/GitoriousWeb.java src/main/java/hudson/plugins/git/browser/KilnGit.java src/main/java/hudson/plugins/git/browser/Phabricator.java src/main/java/hudson/plugins/git/browser/RedmineWeb.java src/main/java/hudson/plugins/git/browser/RhodeCode.java src/main/java/hudson/plugins/git/browser/Stash.java src/main/java/hudson/plugins/git/browser/ViewGitWeb.java http://jenkins-ci.org/commit/git-plugin/510337b35a4665f1d938d35215801bd9d354554e Log: JENKINS-22342 ensure URL normalization correct Previous attempts to fix this stored the normalization flag in the serialized repository browser. Moved to be a function call to avoid the deserializer getting involved

SCM/JIRA link daemon added a comment - 2014-04-12 12:17

Code changed in jenkins
User: Mark Waite
Path:
src/main/java/hudson/plugins/git/browser/BitbucketWeb.java
src/main/java/hudson/plugins/git/browser/CGit.java
src/main/java/hudson/plugins/git/browser/FisheyeGitRepositoryBrowser.java
src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java
src/main/java/hudson/plugins/git/browser/GitLab.java
src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java
src/main/java/hudson/plugins/git/browser/GitWeb.java
src/main/java/hudson/plugins/git/browser/GithubWeb.java
src/main/java/hudson/plugins/git/browser/GitoriousWeb.java
src/main/java/hudson/plugins/git/browser/KilnGit.java
src/main/java/hudson/plugins/git/browser/Phabricator.java
src/main/java/hudson/plugins/git/browser/RedmineWeb.java
src/main/java/hudson/plugins/git/browser/RhodeCode.java
src/main/java/hudson/plugins/git/browser/Stash.java
src/main/java/hudson/plugins/git/browser/ViewGitWeb.java
http://jenkins-ci.org/commit/git-plugin/737e25272e598fa3786ab2dd961cbcf0ebe2eceb
Log:
Merge pull request #224 from Joibel/master

[Fixed JENKINS-22342] ensure URL normalization correct, retain compatibility

Compare: https://github.com/jenkinsci/git-plugin/compare/743d715f2118...737e25272e59

SCM/JIRA link daemon added a comment - 2014-04-12 12:17 Code changed in jenkins User: Mark Waite Path: src/main/java/hudson/plugins/git/browser/BitbucketWeb.java src/main/java/hudson/plugins/git/browser/CGit.java src/main/java/hudson/plugins/git/browser/FisheyeGitRepositoryBrowser.java src/main/java/hudson/plugins/git/browser/GitBlitRepositoryBrowser.java src/main/java/hudson/plugins/git/browser/GitLab.java src/main/java/hudson/plugins/git/browser/GitRepositoryBrowser.java src/main/java/hudson/plugins/git/browser/GitWeb.java src/main/java/hudson/plugins/git/browser/GithubWeb.java src/main/java/hudson/plugins/git/browser/GitoriousWeb.java src/main/java/hudson/plugins/git/browser/KilnGit.java src/main/java/hudson/plugins/git/browser/Phabricator.java src/main/java/hudson/plugins/git/browser/RedmineWeb.java src/main/java/hudson/plugins/git/browser/RhodeCode.java src/main/java/hudson/plugins/git/browser/Stash.java src/main/java/hudson/plugins/git/browser/ViewGitWeb.java http://jenkins-ci.org/commit/git-plugin/737e25272e598fa3786ab2dd961cbcf0ebe2eceb Log: Merge pull request #224 from Joibel/master [Fixed JENKINS-22342] ensure URL normalization correct, retain compatibility Compare: https://github.com/jenkinsci/git-plugin/compare/743d715f2118...737e25272e59

Mark Waite added a comment - 2014-04-12 13:55

Fixed in git-plugin 2.2.1 release 12 Apr 2014

Mark Waite added a comment - 2014-04-12 13:55 Fixed in git-plugin 2.2.1 release 12 Apr 2014

Mark Waite made changes - 2014-04-12 13:55

Resolution		Fixed [ 1 ]
Status	Reopened [ 4 ]	Resolved [ 5 ]

Mark Waite made changes - 2014-04-12 13:55

Status

Resolved [ 5 ]

Closed [ 6 ]

R. Tyler Croy made changes - 2016-07-26 00:00

Workflow

JNJira [ 154433 ]

JNJira + In-Review [ 207555 ]

People

Assignee:: Mark Waite

Reporter:: Alan Clucas

Votes:: 1 Vote for this issue

Watchers:: 5 Start watching this issue

Dates

Created:: 2014-03-25 16:55

Updated:: 2014-04-12 13:55

Resolved:: 2014-04-12 13:55