searching results in "YOUR BROWSER DOESN'T SUPPORT THIS SERVER-PUSH TECHNOLOGY" warning on brower under mod_perl

RESOLVED FIXED in Bugzilla 3.0

Status

()

defect
RESOLVED FIXED
13 years ago
13 years ago

People

(Reporter: glob, Assigned: mkanat)

Tracking

2.23
Bugzilla 3.0
Dependency tree / graph
Bug Flags:
approval +

Details

()

Attachments

(1 attachment, 1 obsolete attachment)

2.61 KB, patch
justdave
: review+
Details | Diff | Splinter Review
searching results in "YOUR BROWSER DOESN'T SUPPORT THIS SERVER-PUSH TECHNOLOGY" warning on brower that does under mod_perl.
Assignee: query-and-buglist → mkanat
Target Milestone: --- → Bugzilla 2.24
Version: unspecified → 2.23
Posted patch v1 (obsolete) — Splinter Review
This is one of the most difficult fixes I've ever done. I spent hours and hours on it today.

We can't rely on package-global variables under mod_perl. They just don't work. Also, mod_perl only sends headers once, because of some special hacks in CGI.pm, as far as I can tell.

At least in CGI.pm 3.11, multipart_start doesn't cause any quoting problems anymore, so we can use its code, and then just add in the cookies.

This fixes the bug. It works under mod_cgi and mod_perl both.
Attachment #227271 - Flags: review?(bugzilla)
Comment on attachment 227271 [details] [diff] [review]
v1


>+sub multipart_start {
>+    my $self = shift;
>+    my $headers = $self->SUPER::multipart_start(@_);
>+    # Eliminate the one extra CRLF at the end.
>+    $headers =~ s/$CGI::CRLF$//;
>+    # Add the cookies.
>+    foreach my $cookie (@{$self->{Bugzilla_cookie_list}}) {
>+        $headers .= "Set-Cookie: ${cookie}${CGI::CRLF}";
>+    }
>+    $headers .= $CGI::CRLF;
>+    return $headers;
>+}

instead of modifying the $headers after its been built, you should push the cookies onto @_ then call SUPER::multipart_start.
Attachment #227271 - Flags: review?(bugzilla) → review-
oh, and checksetup needs to be updated to require CGI 3.11
(In reply to comment #3)
> oh, and checksetup needs to be updated to require CGI 3.11

  I've done that for mod_perl only in bug 342736. Do you think we need to do it for mod_cgi installations, too?
Status: NEW → ASSIGNED
Posted patch v1.1Splinter Review
Okay, I loved your idea, and I implemented it, but then it didn't work because of a bug in CGI.pm. So I've noted here, in a comment, why we do things the way we do them.

Also, I checked, and there's completely no difference between CGI 2.93 and CGI 3.11 for multipart_start. It's just multipart_init that was causing us the problem.
Attachment #227271 - Attachment is obsolete: true
Attachment #227538 - Flags: review?
Attachment #227538 - Flags: review? → review?(bugzilla)
Attachment #227538 - Flags: review?(bugzilla) → review?(justdave)
Comment on attachment 227538 [details] [diff] [review]
v1.1

Looks less dangerous than what we were doing before at least.  We still need to just rewrite all the multipart stuff in CGI and submit it upstream someday ;)
Attachment #227538 - Flags: review?(justdave) → review+
Checking in buglist.cgi;
/cvsroot/mozilla/webtools/bugzilla/buglist.cgi,v  <--  buglist.cgi
new revision: 1.339; previous revision: 1.338
done
Checking in Bugzilla/CGI.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/CGI.pm,v  <--  CGI.pm
new revision: 1.26; previous revision: 1.25
done
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Summary: searching results in "YOUR BROWSER DOESN'T SUPPORT THIS SERVER-PUSH TECHNOLOGY" warning on brower that does under mod_perl → searching results in "YOUR BROWSER DOESN'T SUPPORT THIS SERVER-PUSH TECHNOLOGY" warning on brower under mod_perl
You need to log in before you can comment on or make changes to this bug.