Bugzilla – Attachment 34355 Details for
Bug 10900
Incorrect calling conventions accessing C4::Context
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 10900 - Incorrect calling conventions accessing C4::Context
Bug-10900---Incorrect-calling-conventions-accessin.patch (text/plain), 9.54 KB, created by
Mark Tompsett
on 2014-12-12 15:06:43 UTC
(
hide
)
Description:
Bug 10900 - Incorrect calling conventions accessing C4::Context
Filename:
MIME Type:
Creator:
Mark Tompsett
Created:
2014-12-12 15:06:43 UTC
Size:
9.54 KB
patch
obsolete
>From 21661389544072221157ca6767ec68d9d8f90cb0 Mon Sep 17 00:00:00 2001 >From: Mark Tompsett <mtompset@hotmail.com> >Date: Fri, 20 Sep 2013 21:36:46 -0400 >Subject: [PATCH] Bug 10900 - Incorrect calling conventions accessing > C4::Context > >There were multiple calling conventions for C4::Context's >set_userenv routine. So the following commands were used to >find discrepancies: > grep "::set_userenv" `find .` > grep "\->set_userenv" `find .` > >The first grep demonstrated that the smaller change is from >:: to -> as only C4/Auth.pm, installer/InstallAuth.pm, and >t/db_dependent/Circulation.t would need to be modified. This >patch corrects C4::Context's set_userenv routine to be object >call based (use ->) by using a shift to ignore the first >parameter, and modify the three files found with :: calls. > >As the result of trying to roll a distribution, >t/Circulation_barcodedecode.t was discovered to be faulty. The >cause being incorrect parameters! This was hidden when there >was no shift in the set_userenv routine. However, with its >correction, the test broke. > >This led me to read the POD documentation for the function >set_userenv in C4::Context and realize it was outdated as >well. It has been revised to match the current version of >the function. > >Then intentionally bad parameters passed to the set_userenv >routine in C4::Context were hunted down. The biggest problems >were missing surnames or branch names. > >Rebase required because of shibboleth change in C4/Context.pm > >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> >Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> > >Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> >--- > C4/Auth.pm | 22 +++++++++++----------- > C4/Context.pm | 8 +++++--- > C4/InstallAuth.pm | 6 +++--- > t/Circulation_barcodedecode.t | 2 +- > t/db_dependent/Circulation.t | 2 +- > 5 files changed, 21 insertions(+), 19 deletions(-) > >diff --git a/C4/Auth.pm b/C4/Auth.pm >index b808c66..f1ed568 100644 >--- a/C4/Auth.pm >+++ b/C4/Auth.pm >@@ -728,7 +728,7 @@ sub checkauth { > my $s_userid = ''; > if ($session){ > $s_userid = $session->param('id') // ''; >- C4::Context::set_userenv( >+ C4::Context->set_userenv( > $session->param('number'), $s_userid, > $session->param('cardnumber'), $session->param('firstname'), > $session->param('surname'), $session->param('branch'), >@@ -1042,7 +1042,7 @@ sub checkauth { > if ($persona){ > $session->param('persona',1); > } >- C4::Context::set_userenv( >+ C4::Context->set_userenv( > $session->param('number'), $session->param('id'), > $session->param('cardnumber'), $session->param('firstname'), > $session->param('surname'), $session->param('branch'), >@@ -1282,7 +1282,7 @@ sub check_api_auth { > my $session = get_session($sessionID); > C4::Context->_new_userenv($sessionID); > if ($session) { >- C4::Context::set_userenv( >+ C4::Context->set_userenv( > $session->param('number'), $session->param('id'), > $session->param('cardnumber'), $session->param('firstname'), > $session->param('surname'), $session->param('branch'), >@@ -1445,7 +1445,7 @@ sub check_api_auth { > $session->param('ip',$session->remote_addr()); > $session->param('lasttime',time()); > } >- C4::Context::set_userenv( >+ C4::Context->set_userenv( > $session->param('number'), $session->param('id'), > $session->param('cardnumber'), $session->param('firstname'), > $session->param('surname'), $session->param('branch'), >@@ -1524,7 +1524,7 @@ sub check_cookie_auth { > my $session = get_session($sessionID); > C4::Context->_new_userenv($sessionID); > if ($session) { >- C4::Context::set_userenv( >+ C4::Context->set_userenv( > $session->param('number'), $session->param('id'), > $session->param('cardnumber'), $session->param('firstname'), > $session->param('surname'), $session->param('branch'), >@@ -1663,35 +1663,35 @@ sub checkpw_internal { > > my $sth = > $dbh->prepare( >-"select password,cardnumber,borrowernumber,userid,firstname,surname,branchcode,flags from borrowers where userid=?" >+"select password,cardnumber,borrowernumber,userid,firstname,surname,borrowers.branchcode,branches.branchname,flags from borrowers join branches on borrowers.branchcode=branches.branchcode where userid=?" > ); > $sth->execute($userid); > if ( $sth->rows ) { > my ( $stored_hash, $cardnumber, $borrowernumber, $userid, $firstname, >- $surname, $branchcode, $flags ) >+ $surname, $branchcode, $branchname, $flags ) > = $sth->fetchrow; > > if ( checkpw_hash($password, $stored_hash) ) { > > C4::Context->set_userenv( "$borrowernumber", $userid, $cardnumber, >- $firstname, $surname, $branchcode, $flags ); >+ $firstname, $surname, $branchcode, $branchname, $flags ); > return 1, $cardnumber, $userid; > } > } > $sth = > $dbh->prepare( >-"select password,cardnumber,borrowernumber,userid, firstname,surname,branchcode,flags from borrowers where cardnumber=?" >+"select password,cardnumber,borrowernumber,userid,firstname,surname,borrowers.branchcode,branches.branchname,flags from borrowers join branches on borrowers.branchcode=branches.branchcode where cardnumber=?" > ); > $sth->execute($userid); > if ( $sth->rows ) { > my ( $stored_hash, $cardnumber, $borrowernumber, $userid, $firstname, >- $surname, $branchcode, $flags ) >+ $surname, $branchcode, $branchname, $flags ) > = $sth->fetchrow; > > if ( checkpw_hash($password, $stored_hash) ) { > > C4::Context->set_userenv( $borrowernumber, $userid, $cardnumber, >- $firstname, $surname, $branchcode, $flags ); >+ $firstname, $surname, $branchcode, $branchname, $flags ); > return 1, $cardnumber, $userid; > } > } >diff --git a/C4/Context.pm b/C4/Context.pm >index c5f92ee..1bda03d 100644 >--- a/C4/Context.pm >+++ b/C4/Context.pm >@@ -1085,9 +1085,10 @@ sub userenv { > > =head2 set_userenv > >- C4::Context->set_userenv($usernum, $userid, $usercnum, $userfirstname, >- $usersurname, $userbranch, $userflags, $emailaddress, $branchprinter, >- $persona); >+ C4::Context->set_userenv($usernum, $userid, $usercnum, >+ $userfirstname, $usersurname, >+ $userbranch, $branchname, $userflags, >+ $emailaddress, $branchprinter, $persona); > > Establish a hash of user environment variables. > >@@ -1097,6 +1098,7 @@ set_userenv is called in Auth.pm > > #' > sub set_userenv { >+ shift @_; > my ($usernum, $userid, $usercnum, $userfirstname, $usersurname, $userbranch, $branchname, $userflags, $emailaddress, $branchprinter, $persona, $shibboleth)= @_; > my $var=$context->{"activeuser"} || ''; > my $cell = { >diff --git a/C4/InstallAuth.pm b/C4/InstallAuth.pm >index 35ac2e1..97b45a0 100644 >--- a/C4/InstallAuth.pm >+++ b/C4/InstallAuth.pm >@@ -246,7 +246,7 @@ sub checkauth { > new CGI::Session( "driver:File;serializer:yaml", $sessionID, > { Directory => '/tmp' } ); > if ( $session->param('cardnumber') ) { >- C4::Context::set_userenv( >+ C4::Context->set_userenv( > $session->param('number'), > $session->param('id'), > $session->param('cardnumber'), >@@ -308,7 +308,7 @@ sub checkauth { > #Only superlibrarian should have access to this page. > #Since if it is a user, it is supposed that there is a borrower table > #And thus that data structure is loaded. >- my $hash = C4::Context::set_userenv( >+ my $hash = C4::Context->set_userenv( > 0, 0, > C4::Context->config('user'), C4::Context->config('user'), > C4::Context->config('user'), "", >@@ -412,7 +412,7 @@ sub checkpw { > C4::Context->config('user'), > C4::Context->config('user'), > C4::Context->config('user'), >- "", 1 >+ "", "NO_LIBRARY_SET", 1 > ); > return 2; > } >diff --git a/t/Circulation_barcodedecode.t b/t/Circulation_barcodedecode.t >index 5090e9b..f397cc9 100644 >--- a/t/Circulation_barcodedecode.t >+++ b/t/Circulation_barcodedecode.t >@@ -21,7 +21,7 @@ use Test::More tests => 26; > use t::lib::Mocks; > > C4::Context->_new_userenv(123456); >-C4::Context->set_userenv(1,'kmkale' , 1, 'kk1' , 'IMS', 0, 'kmkale@anantcorp.com'); >+C4::Context->set_userenv(1,'kmkale' , 1, 'km', 'kale' , 'IMS', 'IMS Branch DEscription', 0, 'kmkale@anantcorp.com'); > > BEGIN { > # Mock the DB connexion and C4::Context >diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t >index 58dbc2b..d020d24 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -95,7 +95,7 @@ is( > > # Now, set a userenv > C4::Context->_new_userenv('xxx'); >-C4::Context::set_userenv(0,0,0,'firstname','surname', 'MPL', 'Midway Public Library', '', '', ''); >+C4::Context->set_userenv(0,0,0,'firstname','surname', 'MPL', 'Midway Public Library', '', '', ''); > is(C4::Context->userenv->{branch}, 'MPL', 'userenv set'); > > # Userenv set, PickupLibrary >-- >1.7.9.5
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 10900
:
21152
|
21263
|
21290
|
21291
|
21292
|
22037
|
22038
|
22585
|
22937
|
27729
|
28183
|
30243
|
30244
|
31297
|
31298
|
31719
|
31720
|
31721
|
34355
|
34356
|
34357
|
35542
|
35543
|
35544