Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 349633

Summary:

BugzillaRepositoryConnector.getTaskData() fails when repository URL has trailing slash and no path

Product:

z_Archived

Reporter:

Alex Bradley <a.bradley>

Component:

Mylyn

Assignee:

Frank Becker <eclipse>

Status:

RESOLVED FIXED

QA Contact:

Severity:

minor

Priority:

CC:

eclipse, robert.elves, steffen.pingel

Version:

unspecified

Target Milestone:

3.7

Hardware:

OS:

Linux

Whiteboard:

Attachments:

Description	Flags
Test case	none
Test case output and stack traces	none
mylyn/context/zip	none

Description Alex Bradley

2011-06-16 18:20:16 EDT

Build Identifier: M20110210-1200

If you use BugzillaRepositoryConnector.getTaskData to attempt to retrieve a bug from a TaskRepository that was created with a URL that has a trailing slash and no path (e.g., https://bugzlla.mozilla.org/), an exception is thrown. It seems that in BugzillaClient.postFormData, in the line 

postMethod = new GzipPostMethod(WebUtil.getRequestPath(repositoryUrl.toString() + formUrl), true);

the getRequestPath call ends up returning "//show_bug.cgi", which (via GzipPostMethod superclass org.apache.commons.httpclient.HttpMethodBase) gets fed to the org.apache.commons.httpclient.URI constructor, which interprets show_bug.cgi as the authority section of the URI and therefore creates a URI with a path of null. (Per RFC 1808, I think the URI class is handling the relative URI correctly.) GzipPostMethod therefore tries to post to the webserver root, the HTML main page is returned and an exception results.

Fixing bug#211938 might avoid this problem.

See also: bug#249221

Reproducible: Always

Steps to Reproduce:
1. Run attached test case.
2. Tests testMozillaSlash and testGnomeSlash fail.

Comment 1 Alex Bradley

2011-06-16 18:22:08 EDT

Created attachment 198153 [details]
Test case

Comment 2 Alex Bradley

2011-06-16 18:27:19 EDT

Created attachment 198154 [details]
Test case output and stack traces

Comment 3 Alex Bradley

2011-06-16 18:38:24 EDT

(In reply to comment #0)
> no path (e.g., https://bugzlla.mozilla.org/), an exception is thrown. It seems

Sorry, URL should read "https://bugzilla.mozilla.org/".

Also, my version of org.eclipse.mylyn.bugzilla.core is 3.5.0.v20110316-0100.

Comment 4 Frank Becker

2011-06-24 11:34:29 EDT

How did you set the URL?

If you use the EditRepositoryWizard the Slashes get removed by AbstractRepositorySettingsPage.getRepositoryURL.

Comment 5 Alex Bradley

2011-06-24 17:34:27 EDT

(In reply to comment #4)
> How did you set the URL?
> 
> If you use the EditRepositoryWizard the Slashes get removed by
> AbstractRepositorySettingsPage.getRepositoryURL.

I set the URL by calling the TaskRepository constructor, as shown in the test case. I'm trying to use the Bugzilla connector through the Mylyn API (for a plugin of my own); I'm not accessing it through the Mylyn UI.

Comment 6 Frank Becker

2011-06-26 15:03:49 EDT

(In reply to comment #5)
> (In reply to comment #4)
> > How did you set the URL?
> >
> > If you use the EditRepositoryWizard the Slashes get removed by
> > AbstractRepositorySettingsPage.getRepositoryURL.
> 
> I set the URL by calling the TaskRepository constructor, as shown in the test
> case. I'm trying to use the Bugzilla connector through the Mylyn API (for a
> plugin of my own); I'm not accessing it through the Mylyn UI.

Sorry I see no way how we can change this. The reason is that TaskRepository Constructor is not Bugzilla specific so we can not add code to remove tailing slashes.
If you not use the UI you must do this your own.

Comment 7 Alex Bradley

2011-06-27 17:57:22 EDT

(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > How did you set the URL?
> > >
> > > If you use the EditRepositoryWizard the Slashes get removed by
> > > AbstractRepositorySettingsPage.getRepositoryURL.
> > 
> > I set the URL by calling the TaskRepository constructor, as shown in the test
> > case. I'm trying to use the Bugzilla connector through the Mylyn API (for a
> > plugin of my own); I'm not accessing it through the Mylyn UI.
> 
> Sorry I see no way how we can change this. The reason is that TaskRepository
> Constructor is not Bugzilla specific so we can not add code to remove tailing
> slashes.

I don't see how the generality of TaskRepository necessarily implies that conclusion. I tend to agree with bug#211938 that "[the stripping of trailing slash in repository urls] should be encapsulated in the TaskRepository object itself". Stripping the trailing slash from a URL doesn't change its meaning (and therefore shouldn't break any other connector), and doing it centrally in the TaskRepository seems better than doing ad hoc slash removal in every UI that uses TaskRepository. 

If you don't want to change TaskRepository, can't you at least modify BugzillaClient so it doesn't handle URLs with trailing slashes in a way that causes an exception? What about modifying WebUtil.getRequestPath so it collapses "//" at the beginning of resulting request paths into a single "/"?

Fundamentally, an API that claims to accept URLs shouldn't crash when you pass it perfectly valid URLs.

Comment 8 Steffen Pingel

2011-06-27 18:12:51 EDT

Agreed, we should evaluate if BugzillaClient to handle that case more gracefully or update the documentation.

Comment 9 Frank Becker

2011-07-25 16:30:51 EDT

Here is what I have commit, can you please verify?

(http://git.eclipse.org/c/mylyn/org.eclipse.mylyn.tasks.git/commit/?id=9e71937c527fc44eb9c2eb7d00d7357dcbd11898)

Comment 10 Frank Becker

2011-07-25 16:30:54 EDT

Created attachment 200311 [details]
mylyn/context/zip

Comment 11 Alex Bradley

2011-07-27 18:26:38 EDT

(In reply to comment #9)
> Here is what I have commit, can you please verify?
> 
> (http://git.eclipse.org/c/mylyn/org.eclipse.mylyn.tasks.git/commit/?id=9e71937c527fc44eb9c2eb7d00d7357dcbd11898)

Looks OK to me, and I can confirm that my test cases pass with this fix.

Comment 12 Frank Becker

2011-08-01 15:03:08 EDT

We can close this see comment#11