Closed Bug 274640 Opened 17 years ago Closed 17 years ago

Custom sort cookies which include LIMIT are now forbidden; scary error appears

Categories

(Bugzilla :: Query/Bug List, defect)

2.19.1
defect
Not set
major

Tracking

()

RESOLVED FIXED
Bugzilla 2.18

People

(Reporter: justdave, Assigned: gerv)

References

Details

Attachments

(1 file)

We have a FREQUENTLY ocurring ThrowCodeError invocation which I've now received
several copies of on the bugzilla-admin alias, which gives the error:

The custom sort order specified in your cookie contains an invalid column name
/relevance DESC LIMIT 200/. The cookie has been cleared.

And asks the user to mail it to the admin.

This kind of thing shouldn't even be throwing an error.  This should put up a
little green box at the top of the query results saying it's using a default
sort order because their sort order stored in their cookie was invalid, and run
the query anyway.
Is 274423 a dupe of this?
Is bug 274423 a dupe of this?
sort of, but not quite...  see my comment there about this bug.
*** Bug 274423 has been marked as a duplicate of this bug. ***
Comment from bug 274423:

OK, I found the cause of this...

The checkin from bug 237176 removed the following chunk of code from buglist.cgi:

-# Even more disgusting hack: if we are doing a full text search,
-# order by relevance instead of anything else, and limit to 200 results.
-if ($search->{'sorted_by_relevance'}) {
-    $db_order = $order = "relevance DESC LIMIT 200";
-    $vars->{'sorted_by_relevance'} = 1;
-}

Note that the code that *used* to be there added the "relevance DESC LIMIT 200"
to the sort order variable instead of tacking it directly onto the query itself
(which is what the current code does).  This would have resulted in a cookie
getting set with that as their sort order once the page is displayed.  When this
was removed, the checks to insure a valid sort order were also beefed up.  That
chunk no longer passes the test, because "LIMIT 200" isn't a legal part of the
sort order (and never was - that's a different portion of the SQL query).

So the people who are hitting this are people who have made use of the Specific
search prior to our upgrade, and are getting this on the first query they make
after using Bugzilla for the first time after the upgrade.

There is, then, nothing we can really do to prevent this from happening, and the
only thing we need to do is deal with it in a less threatening way.
Bug 163114 seems to be the one responsible for turning this into a ThrowCodeError.
Given that this was our mistake, should we instead add code to check for this
particular condition and silently fix things up?

Gerv
*** Bug 275602 has been marked as a duplicate of this bug. ***
I agree with Gerv - fixing this up silently is the best way to handle it.  The
impression I got at first blush was based on the error message I got.  To quote
Bugzilla:

<tt>
  <p>Bugzilla has suffered an internal error. Please save this page and send
    it to bugzilla-admin@mozilla.org with details of what you were doing at
    the time this message appeared.
  </p>
  <script type="text/javascript"> <!--
    document.write("<p>URL: " + document.location + "</p>");
  // -->
  </script>
</tt>

That says that the error was severe enough that an administrator needs to get
involved while in reality, simply hitting refresh fixes the problem.  As an
administrator, that stinks.  It will mean that I will have more work to do
because users will stop dead in their tracks and call me to report this error. 
I will need to take time to explain each time that error message comes up. 
IMNSHO (agreeing with Gerv), BZ ought to be able to fix that by itself by fixing
the cookie and displaying appropriate results as if the LIMIT had not been
specified in the cookie.

Also, please consider changing the summary for this bug.  I didn't find it when
I looked for "custom sort cookie".
Summary: We tell the user to email the admin for an "informational" error message that doesn't mean Bugzilla is broken. → Sort cookies which include LIMIT are now forbidden; scary error appears
Target Milestone: --- → Bugzilla 2.20
Summary: Sort cookies which include LIMIT are now forbidden; scary error appears → Custom sort cookies which include LIMIT are now forbidden; scary error appears
Flags: blocking2.20+
Flags: blocking2.18+
Target Milestone: Bugzilla 2.20 → Bugzilla 2.18
Attached patch Patch v.1Splinter Review
This one-liner should do the trick. It seems to work on my system (although I
have to edit cookies.txt manually to add "relevance DESC LIMIT 200" to the
LASTORDER cookie).

Gerv
Assignee: justdave → gerv
Status: NEW → ASSIGNED
Attachment #170026 - Flags: review?(justdave)
Attachment #170026 - Flags: review+
Flags: approval?
Flags: approval2.18?
Attachment #170026 - Flags: review?(justdave) → review+
Flags: approval?
Flags: approval2.18?
Flags: approval2.18+
Flags: approval+
I've filed bug 276728 to pick up the parts of this issue that weren't resolved
with this patch.
Fixed on trunk and branch.

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

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

Gerv
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.