96003 - buglist.cgi shouldn't find all bugs if loaded without parameters

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, defect, P3)

Description

[Myk Melez [:myk] [@mykmelez]]

buglist.cgi got into my URL history somehow, and now I sometimes accidentally
select it from the auto-completion list as I am typing in a Bugzilla URL.  When
I do, it queries the database for all bugs and tries to display the list (until
I cancel the request).  Instead of doing this, it should display an error or
forward me to query.cgi.

Comment 1

[Jacob Steenhagen]

I don't think I like this one.  I have, on occation, bookmarked a query with the
intention of being able to run it directly without stopping at query.cgi.

In fact, I have a bookmarked Bugzilla 2.14 query (which runs my stored query).

Comment 2

[Myk Melez [:myk] [@mykmelez]]

Correctly misleading summary.  What I meant to say is only that the following
URL shouldn't work:

http://bugzilla.mycompany.com/buglist.cgi

Comment 3

[Dave Miller [:justdave]]

I don't think that's what he meant, Jake.  I think he meant if you ran it
without any query parms.

Comment 4

[Dave Miller [:justdave]]

grrr

Comment 5

[Jacob Steenhagen]

OK, that makes much more sense :)

Comment 6

[Matthew Tuck [:CodeMachine]]

Moving to new Bugzilla product ...

Comment 7

[Dave Miller [:justdave]]

We are currently trying to wrap up Bugzilla 2.16.  We are now close enough to
release time that anything that wasn't already ranked at P1 isn't going to make
the cut.  Thus this is being retargetted at 2.18.  If you strongly disagree with
this retargetting, please comment, however, be aware that we only have about 2
weeks left to review and test anything at this point, and we intend to devote
this time to the remaining bugs that were designated as release blockers.

Comment 8

[Gervase Markham [:gerv]]

I have a patch for this.

Gerv

Comment 9

[Gervase Markham [:gerv]]

Attachment: Patch v.1

This redirects to query.cgi if no parameters are given.

Gerv

Comment 10

[Myk Melez [:myk] [@mykmelez]]

We need to also test for the case where there are parameters but no search
criteria.  If you unselect "NEW", "ASSIGNED", and "REOPENED" on the query page
and submit the form there will be a bunch of parameters in the URL but no actual
search criteria.  This should also be prevented.

Comment 11

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 92144 [details] [diff] [review]
Patch v.1

In the meantime, this is a good start.	r=myk

Comment 12

[Gervase Markham [:gerv]]

We can go some way towards that by doing an

s/[^&\?]*=&//g;
- i.e. cut out every param of the form &foo_param=&bar=&baz...

But that really doesn't cut it because there are a lot of params with default
values which nevertheless don't restrict the search.

Perhaps we need a URL-optimising function; that would be useful in other
situations too.

Myk: does this need a second review? Is it right that the default now is not,
unless the reviewer specifically says it needs one?

Gerv

Comment 13

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 92144 [details] [diff] [review]
Patch v.1

Yes, that's right, only one review is required by default for 2.18 bugs unless
otherwise indicated by the first reviewer.

However, I took another look at the patch and have some nits...

>Index: buglist.cgi

>@@ -61,6 +61,17 @@

>+    $vars->{'message'} = "This script is not meant for invoking without any 
>+                          parameters. Please search using the search form.";

"for invoking" -> "to be invoked"

"parameters" -> "search criteria", "search terms", or "search conditions"

"Please search" -> "Please specify some search criteria"


>+    print "Refresh: 0; URL=query.cgi\n";

We should give them some time to read the message.  I suggest refreshing after
10 seconds but providing a link for them to go to the search page more quickly
via the "url" and "link" parameters to message.html.tmpl.

Comment 14

[Gervase Markham [:gerv]]

Attachment: Patch v.2

Patch that I checked in, with myk's changes made.

Gerv

Comment 15

[Gervase Markham [:gerv]]

Fixed.

Checking in buglist.cgi;
/cvsroot/mozilla/webtools/bugzilla/buglist.cgi,v  <--  buglist.cgi
new revision: 1.182; previous revision: 1.181
done

Gerv

Comment 16

[Matthew Tuck [:CodeMachine]]

Myk, I don't think we need to prevent returning all bugs.  Smaller installations
could find this quite useful.  I think the accidental case fixed here is the
real killer.

Deliberate attacks are certainly an issue, but I think a better solution is a
bmo hack or a limit to the number of results that can get returned.