Closed Bug 513989 Opened 15 years ago Closed 14 years ago

large search query causing internal server error (500) but valid redirect 302 returned

Categories

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

defect

Tracking

()

RESOLVED FIXED
Bugzilla 3.4

People

(Reporter: rob.elves, Assigned: dkl)

Details

Attachments

(3 files, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.2) Gecko/20090729 Firefox/3.5.2
Build Identifier: 3.4.1

When a search is posted, the code introduced on bug#15809 cleans the url and causes a redirect. In the event that the number of parameters in the search is very large, an internal server error occurs but instead of returning the error as a 500 response, the repository proceeds to send back a 302 redirect.  In this scenario where the query has a large number of parameters, the redirect header is a truncated (8k max) resulting in a non-deterministic query result.

This is problematic for the Mylyn Bugzilla connector since it can lead to hundreds of false incoming notifications when the redirect doesn't contain the date range qualifier (i.e. all tasks get returned instead of only those that have changed within the range specified in the original post).

Reproducible: Sometimes

Steps to Reproduce:
1.Set up a large query in Mylyn (i.e. > 1000 bugs)
2.Synchronize all tasks marking read
3.Re-synchronize all tasks, eventually this problem will occur and results in numerous incomings
Um, that's very strange, because it's Apache that determines the maximum URL length. Are you saying that Apache is cutting off the actual Location header being returned?
Attached file wget debug output
The attachment shows the output of the following command:

wget --debug -O out.txt --post-file=post.txt http://mylyn.eclipse.org/bugs34/buglist.cgi &> request.txt
Attached file the post request
(In reply to comment #1)
> Um, that's very strange, because it's Apache that determines the maximum URL
> length. Are you saying that Apache is cutting off the actual Location header
> being returned?

Yes, that is exactly what is happening. Below is a dump of a request that results in a redirect larger than 8kb. Buglist.cgi returns a 302 but Apache detects an error, appends the output for a 500 error and writes this to the log:

[Fri Sep 04 03:20:58 2009] [error] [client 24.80.3.22] malformed header from script. Bad header=2C132602%2C114124%2C132608%2C1: buglist.cgi

Mylyn follows the valid looking redirect which results in a different being executed than originally requested.

Wireshark dump (part of the URL truncated with [...]):

POST /bugs34/buglist.cgi HTTP/1.0
User-Agent: Wget/1.11.4
Accept: */*
Host: mylyn.eclipse.org
Connection: Keep-Alive
Content-Type: application/x-www-form-urlencoded
Content-Length: 8185

query_format=advanced&chfieldfrom=2009-08-31+15%3A03%3A32+EDT&chfieldto=Now&bug_id=247911%2C247912%2C228377%2C210433%2C186044%2C192558[...]


HTTP/1.1 302 Found
Date: Fri, 04 Sep 2009 07:20:57 GMT
Server: Apache/2.2.0 (Linux/SUSE)
Location: http://mylyn.eclipse.org/bugs34/buglist.cgi?ctype=rdf&chfieldto=Now&query_format=advanced&chfieldfrom=2009-08-31%2015%3A03%3A32%20EDT&bug_id=247911%2C247912%2C2
[...]
2C277999%2C173121%2C188524%2C106862%2C188523%2C106861%2C247900%2C277991%2C247901%2C247903%2C255948%2C192544%2C186011%
Content-Length: 0
Connection: close
Content-Type: application/x-cgi

<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>302 Found</title>
</head><body>
<h1>Found</h1>
<p>The server encountered an internal error or
misconfiguration and was unable to complete
your request.</p>
<p>Please contact the server administrator,
 [no address given] and inform them of the time the error occurred,
and anything you might have done that may have
caused the error.</p>
<p>More information about this error may be available
in the server error log.</p>
<p>Additionally, a 500 Internal Server Error
error was encountered while trying to use an ErrorDocument to handle the request.</p>
<hr>
<address>Apache/2.2.0 (Linux/SUSE) Server at mylyn.eclipse.org Port 80</address>
</body></html>
> [Fri Sep 04 03:20:58 2009] [error] [client 24.80.3.22] malformed header from
> script. Bad header=2C132602%2C114124%2C132608%2C1: buglist.cgi


Since the upgrade to 3.4.1 I've been seeing frequent errors like that one in my error_log.
Interesting. Indeed, the problem is that the entire response header is greater than Apache's limit of 8K--quite a rare situation indeed.

The solution here is for us to simply not do the redirect when the resulting URL is that large, and instead just display the results during the POST.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Hardware: x86 → All
Priority: -- → P2
Target Milestone: --- → Bugzilla 3.4
We are having issues with this as well with people submitting long queries using POST. They assume that using POST with buglist.cgi will solve the URI lenght limitation we have set on out webapps but since buglist.cgi redirects to GET then it still fails. 

Is it possible to have buglist.cgi not do the redirect if $ENV{CONTENT_LENGTH} is greater than some value?

Dave
@Dave It is indeed possible, and I'd accept a patch that does that.
Flags: blocking3.4.6+
Although I think CONTENT_LENGTH might not be the right item, since it's the length of the headers that matters. Perhaps simply checking the URL length would work. You could put a new parameter in the Advanced section, for trunk, and just make it a constant on the branch.
Assignee: query-and-buglist → dkl
Attachment #429612 - Flags: review?(mkanat)
Comment on attachment 429612 [details] [diff] [review]
Patch to not redirect in buglist.cgi if URI is too long for 3.4 (v1) 

>=== modified file 'Bugzilla/Constants.pm'
>+    my $uri_length = length(correct_urlbase() . "buglist.cgi" . $cgi->canonicalise_query());

  Here you can just check the length of $cgi->self_url, I'm pretty sure.

>+    if (!CGI_URI_LIMIT || $uri_length < CGI_URI_LIMIT) {
>+        print $cgi->redirect(-url => $cgi->url(-relative => 1, -query => 1));

  I don't think that the redirect() code needs to be changed, does it?
Attachment #429612 - Flags: review?(mkanat) → review-
Comment on attachment 429613 [details] [diff] [review]
Patch to not redirect in buglist.cgi if URI is too long for 3.6 (v1) 

  Thinking about it and looking at both these patches, I just want to go with the 3.4 implementation--I'm realizing that a parameter isn't really needful.
Attachment #429613 - Flags: review?(mkanat) → review-
(In reply to comment #12)
> (From update of attachment 429612 [details] [diff] [review])
> >=== modified file 'Bugzilla/Constants.pm'
> >+    my $uri_length = length(correct_urlbase() . "buglist.cgi" . $cgi->canonicalise_query());
> 
>   Here you can just check the length of $cgi->self_url, I'm pretty sure.

I wasn't sure without looking at the CGI.pm code if self_url had the raw query string or the current adjusted one. I know query_string() is the raw string so I fel best to use canonicalise_query() to be sure.

> >+    if (!CGI_URI_LIMIT || $uri_length < CGI_URI_LIMIT) {
> >+        print $cgi->redirect(-url => $cgi->url(-relative => 1, -query => 1));
> 
>   I don't think that the redirect() code needs to be changed, does it?

It is just indented. I didn't change the actual code there.

Dave
I'll still take this for 3.4.6 if it comes in before we do the release.
Flags: blocking3.4.6+ → blocking3.4.7+
(In reply to comment #14)
> > >+    if (!CGI_URI_LIMIT || $uri_length < CGI_URI_LIMIT) {
> > >+        print $cgi->redirect(-url => $cgi->url(-relative => 1, -query => 1));
> > 
> >   I don't think that the redirect() code needs to be changed, does it?

Ah, my bad. I see now what you meant. That was me copying the same code from the 3.6 patch which I realize the redirect() is different.

New patch coming.
Ok. Switched to using $cgi->self_url and also use same patch now for both 3.4 and 3.6 which works fine for me.
Attachment #429612 - Attachment is obsolete: true
Attachment #429613 - Attachment is obsolete: true
Attachment #429752 - Flags: review?(mkanat)
Comment on attachment 429752 [details] [diff] [review]
Patch to not redirect in buglist.cgi if URI is too long for 3.4/3.6 (v2) 

>=== modified file 'Bugzilla/Constants.pm'
>+    if (!CGI_URI_LIMIT || $uri_length < CGI_URI_LIMIT) {

  I wouldn't even check ! on the constant.

  Otherwise, this looks fine; the above can be fixed on checkin.
Attachment #429752 - Flags: review?(mkanat) → review-
Flags: blocking3.4.7+
Flags: blocking3.4.6+
Flags: approval3.6+
Flags: approval3.4+
Flags: approval+
Attachment #429752 - Flags: review- → review+
3.4:
modified buglist.cgi
modified Bugzilla/Constants.pm
Committed revision 6735.

3.6:
modified buglist.cgi
modified Bugzilla/Constants.pm
Committed revision 7010.

trunk:
modified buglist.cgi
modified Bugzilla/Constants.pm
Committed revision 7041.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Actually after applying this to our code and getting ready to push to production. I feel we should lower the limit from 10000 to 8000 since the Apache default URI limit is 8190 and most people will probably not change that. So if we have it set to 10000, then it is still possible to get the error. Thoughts?

http://httpd.apache.org/docs/2.2/mod/core.html#limitrequestline

Dave
  Yeah, reducing it to 8000 is a good idea. You can fix that with a second checkin; make sure to still set --fixes on the commit, though.
(In reply to comment #21)
>   Yeah, reducing it to 8000 is a good idea. You can fix that with a second
> checkin; make sure to still set --fixes on the commit, though.

And it keeps getting better :) Apparently it was made known to me that although buglist.cgi now works for querys over 8k, the other links on the page, such as Change Columns, CSV, Feed, etc. and probably the header links for sorting still give the Request-URI Too Large error. Should I reopen this bug and try to find a solution or open a separate bug?

Thanks
Dave
  I believe there is already a separate bug for the general idea that URLs can be too long, but if there isn't, you can file a separate bug. In any case, keep this one FIXED.
(In reply to comment #21)
>   Yeah, reducing it to 8000 is a good idea. You can fix that with a second
> checkin; make sure to still set --fixes on the commit, though.

Committed to 3.4, 3.6, and trunk.
modified Bugzilla/Constants.pm

Dave
You need to log in before you can comment on or make changes to this bug.