WordPress.org
Get WordPress

Plugin Directory

login-security-solution

Opened 14 years ago

Closed 14 years ago

Last modified 14 years ago

#1551 closed defect (invalid)

Class method visibility needs reviewing

Reported by: deanmarktaylor's profile deanmarktaylor's profile deanmarktaylor Owned by: convissor's profile convissor's profile convissor
Priority: normal Severity: normal
Plugin: login-security-solution Keywords: class, visibility, protected, method, function
Cc:

Description

There appears to be cases where method visibility (protected vs public) might cause unexpected runtime PHP errors/warnings, depending on PHP version, configuration and warning level.

This might cause unexpected behaviours.

Currently on reviewing version 0.15.0 the following potential problems exist:

  • login_security_solution constructor: $admin->was_pw_force_change_done() call on login_security_solution_admin class where method was_pw_force_change_done is protected.

Not 100% sure on this one, perhaps there is some exception for unit tests?

  • LoginFailTest calls protected method get_login_fail on class login_security_solution.
  • IdleTest calls protected method get_last_active on class login_security_solution.
  • There many other multiple cases of this calling of protected methods throughout the unit tests.

Attachments (1)

class_method_visibility.php​ (2.8 KB) - added by deanmarktaylor 14 years ago.
Added test code to show behaviour of callbacks in PHP

Download all attachments as: .zip

Change History (5)

@deanmarktaylor
14 years ago

Added test code to show behaviour of callbacks in PHP

#1 @deanmarktaylor
14 years ago

In reference to the class_method_visibility.php test code, please find below my output:

* PHP_VERSION: 5.3.10

* [before] trigger_notice

Notice: Undefined property: stdClass::$property_which_does_not_exist in /var/sites/g/testtesttest.com/public_html/class_method_visibility.php on line 25

* * 

* [after] trigger_notice

* [before] trigger_notice
Notice: Undefined property: stdClass::$property_which_does_not_exist in /var/sites/g/testtesttest.com/public_html/class_method_visibility.php on line 25 
* * 

* [after] trigger_notice

* [before] ::execute()

* * [before] TEST 1: do_action(cmv_public)

* * * public_function()

* * [after] TEST 1: do_action(cmv_public)

* * [before] TEST 2: do_action(cmv_protected) - should cause a "cannot access protected method" warning
Warning: call_user_func_array() expects parameter 1 to be a valid callback, cannot access protected method ClassMethodVisibility::protected_function() in /var/sites/g/testtesttest.com/public_html/wp-includes/plugin.php on line 403 
* * [after] TEST 2: do_action(cmv_protected)

* * [before] TEST 3: call_user_func_array(public_function)

* * * public_function()

* * [after] TEST 3: call_user_func_array(public_function)

* * [before] TEST 4: call_user_func_array(protected_function)

* * * protected_function()

* * [after] TEST 4: call_user_func_array(protected_function)

* [after] ::execute()

* [before] TEST 5: call_user_func_array(protected_function) - should cause a "cannot access protected method" warning
Warning: call_user_func_array() expects parameter 1 to be a valid callback, cannot access protected method ClassMethodVisibility::protected_function() in /var/sites/g/testtesttest.com/public_html/class_method_visibility.php on line 95 
* [after] TEST 5: call_user_func_array(protected_function)
Last edited 14 years ago by deanmarktaylor (previous) (diff)

#2 in reply to: ↑ description @convissor
14 years ago

Replying to deanmarktaylor:

  • login_security_solution constructor: $admin->was_pw_force_change_done() call on login_security_solution_admin class where method was_pw_force_change_done is protected.

Protected items are visible to child and parent classes, so no problem there.

Not 100% sure on this one, perhaps there is some exception for unit tests?

  • LoginFailTest calls protected method get_login_fail on class login_security_solution.

... snip ...

The class in tests/Accessor.php provides overloading methods for getting to protected elements in the plugin's classes.

#3 @convissor
14 years ago

  • Resolution set to invalid
  • Status changed from new to closed

#4 @deanmarktaylor
14 years ago

I've had this issue with other 3rd party plugins... hence my concern.

Dan - cheers for reviewing and clarifying!

Cheers,
Dean.

Note: See TracTickets for help on using tickets.