Closed
Bug 236678
Opened 20 years ago
Closed 20 years ago
Clean up access to COOKIE global
Categories
(Bugzilla :: Bugzilla-General, defect)
Tracking
()
RESOLVED
FIXED
Bugzilla 2.20
People
(Reporter: kiko, Assigned: kiko)
References
Details
Attachments
(1 file, 3 obsolete files)
4.71 KB,
patch
|
bugreport
:
review+
|
Details | Diff | Splinter Review |
We should clean up access to COOKIE global. Callsites involved: attachment.cgi buglist.cgi post_bug.cgi enter_bug.cgi process_bug.cgi move.pl query.cgi show_bug.cgi userlist.cgi Note that $::COOKIE{'Bugzilla_login'}; is supposed to become either: Bugzilla->user->login or [% user.login %]
Assignee | ||
Comment 1•20 years ago
|
||
My patch for bug 226754 kills COOKIE{'Bugilla_logincookie'}.
Depends on: 226754
Comment 2•20 years ago
|
||
OK, after all the dependencies are resolved, you can as a patch to this bug remove the backward compatibility stuff from CGI.pl: 417: $::COOKIE{$name} = $::cgi->cookie($name); Bugzilla.pm: 61: # $::COOKIE{'Bugzilla_login'} from a userid to a loginname 84: $::COOKIE{'Bugzilla_login'} = $_user->login; 128: delete $::COOKIE{"Bugzilla_login"}; Remember that nothing should be using $cgi->cookie("Bugzilla_login") outside of the Auth stuff. Everyone else should be using Bugzilla->user->login. If you want, the COOKIE stuff is few and far enough between that it would probably work as a single patch to this bug to wipe them all out at once. If you do that, remove %COOKIE from the summary on the rest of the dependent bugs, and move their dependencies back the main %FORM bug once this one is ready to go in.
Comment 3•20 years ago
|
||
The only file with %COOKIE in it that's not mentioned on one of the dependent bugs is Bugzilla/Bug.pm. 394: && (defined $::COOKIE{"Bugzilla_login"}) 395: && ($::COOKIE{"Bugzilla_login"} =~ /$movers/); It'd be a waste to file a separate bug for it if you wind up rolling it all into one patch anyway. :)
Assignee | ||
Comment 4•20 years ago
|
||
The final cleanup should also apply to Bugzilla.pm and globals.pl, which have the backwards compatibility code that provides COOKIE in the first place.
Assignee | ||
Updated•20 years ago
|
Assignee | ||
Comment 5•20 years ago
|
||
I've started the COOKIE revolution separately so we get at least some traction here.
Assignee | ||
Comment 6•20 years ago
|
||
Dave, it seems that the code in Bugzilla::Bug->user isn't being used anywhere. Or is it?
Assignee | ||
Comment 7•20 years ago
|
||
Hmm, yes it is: in knob.html.tmpl. So that means I have to actually understand it :-(
Assignee | ||
Comment 8•20 years ago
|
||
This eliminates the last traces of cookie off the map. It got a bit grotesque because there was a weird logic error in Bug->user that required doing some groovy changes there and I decided to de-$::userid-ify it while I was at it (in part because the old code was hard to read). I think it's much nicer of course.
Assignee: nobody → kiko
Status: NEW → ASSIGNED
Assignee | ||
Updated•20 years ago
|
Attachment #153960 -
Flags: review?(bugreport)
Comment 9•20 years ago
|
||
Comment on attachment 153960 [details] [diff] [review] kiko_v1: COOKIE KILLER >+ my $canedit = (!Bugzilla->user) >+ || $isreporter >+ || &::UserInGroup("editbugs") ewww... I see :: still ;) Try Bugzilla->user->in_group("editbugs") >- # NB - Can't delete from $cgi->cookie, so the logincookie data will >- # remain there; it's only used in Bugzilla::Auth::CGI->logout anyway >- # People shouldn't rely on the cookie param for the username >- # - use Bugzilla->user instead! >+ # NB - Won't delete from $cgi->cookie, so the logincookie data will >+ # and should remain there. Don't rely on it: use Bugzilla->user >+ # instead! Can't or won't? If it's possible, why don't we?
Assignee | ||
Comment 10•20 years ago
|
||
Attachment #153960 -
Attachment is obsolete: true
Assignee | ||
Updated•20 years ago
|
Attachment #154052 -
Flags: review?(bugreport)
Assignee | ||
Updated•20 years ago
|
Attachment #153960 -
Flags: review?(bugreport)
Assignee | ||
Updated•20 years ago
|
Attachment #154052 -
Attachment is obsolete: true
Attachment #154052 -
Flags: review?(bugreport)
Assignee | ||
Comment 11•20 years ago
|
||
Assignee | ||
Updated•20 years ago
|
Attachment #154053 -
Flags: review?(bugreport)
Comment 12•20 years ago
|
||
Comment on attachment 154053 [details] [diff] [review] kiko_v2: COOKIE KILLER 2.0 (the real one) I think the logic is wrong for cancinfirm, If a user is both the reporter and the assignee, they used to be able to confirm. Now they would no longer be able to.
Attachment #154053 -
Flags: review?(bugreport) → review-
Assignee | ||
Comment 13•20 years ago
|
||
You're right, but only because the code is now understandable <wink>. I'll have it fixed in the afternoon.
Assignee | ||
Comment 14•20 years ago
|
||
Assignee | ||
Updated•20 years ago
|
Attachment #154053 -
Attachment is obsolete: true
Assignee | ||
Updated•20 years ago
|
Attachment #154325 -
Flags: review?(bugreport)
Comment 15•20 years ago
|
||
Comment on attachment 154325 [details] [diff] [review] kiko_v3: okidoki r=joel Interestingly, a user without canconfirm could assign a bug to himself, confirm it, and then reassign it. If they figure that out, though, they deserve confirm.
Attachment #154325 -
Flags: review?(bugreport) → review+
Assignee | ||
Comment 16•20 years ago
|
||
Well, if you wanted to revoke that right, you should never have brought up the issue in the first place <wink>. Thanks.
Flags: approval?
Updated•20 years ago
|
Flags: approval? → approval+
OS: Linux → All
Hardware: PC → All
Target Milestone: --- → Bugzilla 2.20
Assignee | ||
Comment 17•20 years ago
|
||
Checked in: /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v <-- Bug.pm new revision: 1.40; previous revision: 1.39 /cvsroot/mozilla/webtools/bugzilla/Bugzilla.pm,v <-- Bugzilla.pm new revision: 1.14; previous revision: 1.13 /cvsroot/mozilla/webtools/bugzilla/CGI.pl,v <-- CGI.pl new revision: 1.212; previous revision: 1.211 /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Auth/README,v <-- README new revision: 1.2; previous revision: 1.1 I should have done this long ago. As it stands, thanks!
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 18•20 years ago
|
||
Oops. Some bits were still left in Auth/Login/WWW.pm -- just murdered them too.
Updated•12 years ago
|
QA Contact: matty_is_a_geek → default-qa
You need to log in
before you can comment on or make changes to this bug.
Description
•