Make Bugzilla use CGI.pm to send Cookie headers

RESOLVED FIXED in Bugzilla 2.18

Status

()

Bugzilla
Bugzilla-General
RESOLVED FIXED
14 years ago
5 years ago

People

(Reporter: justdave, Assigned: justdave)

Tracking

2.17.6
Bugzilla 2.18
Bug Flags:
approval +

Details

Attachments

(1 attachment, 2 obsolete attachments)

This is needed for mod_perl.  There's an XXX - mod_perl comment on our existing
Cookie code which describes this.  Patch coming momentarily.
Comment on attachment 135767 [details] [diff] [review]
Patch

This fixes the situation by stashing the cookies (as the comment indicates it
should have been) instead of printing them, and then overriding the
CGI->header() method to add the cookie parameter containing our cookie list
before calling the real CGI->header.
Attachment #135767 - Flags: review?(bbaetz)
Comment on attachment 135767 [details] [diff] [review]
Patch

After further testing, this is broken.	Replacement patch coming up shortly
Attachment #135767 - Attachment is obsolete: true
Attachment #135767 - Flags: review?(bbaetz)
Created attachment 135775 [details] [diff] [review]
Patch v2

Previous iteration didn't take into account that the caller might have passed a
non-qualified content-type as a parameter (like the new js buglist which does
$cgi->header("text/javascript"); ).  This one fixes that and also only adds the
Cookie part if there's cookies to add, instead of passing an empty array of
cookies.
Attachment #135775 - Flags: review?(bbaetz)

Comment 5

14 years ago
If we're storing a Content-Type in it, is Bugzilla_cookie_list still an
appropriate name?
We store nothing but cookies in Bugzilla_cookie_list.  The content-type is being
passed in, not being read from Bugzilla_cookie_list.
Created attachment 136093 [details] [diff] [review]
Patch v3

Oops, cookies didn't work in buglist.cgi.  Fixed that with this rev.
Attachment #135775 - Attachment is obsolete: true
Attachment #135775 - Flags: review?(bbaetz)
Attachment #136093 - Flags: review?(bbaetz)
Comment on attachment 136093 [details] [diff] [review]
Patch v3

I guess. How about fixing multipart_start via an upstream patch?

r=bbaetz anyway - given the limited requirements we have for multipart_start,
this is OK
Attachment #136093 - Flags: review?(bbaetz) → review+
Checking in Bugzilla/CGI.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/CGI.pm,v  <--  CGI.pm
new revision: 1.7; previous revision: 1.6
done
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Flags: approval+
Resolution: --- → FIXED
Target Milestone: --- → Bugzilla 2.18
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.