Make WordPress Core

Opened 19 months ago

Last modified 18 months ago

#57262 new defect (bug)

force_ssl_admin can be set to return a string

Reported by: pbearne's profile pbearne Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch has-unit-tests dev-feedback
Focuses: Cc:


See these tests

$this->assertTrue( force_ssl_admin( 'a string' ), 'set true' );
$this->assertSame( 'a string', force_ssl_admin(), 'check is still true' );

I will add a check that Bool is passed in not string

Change History (3)

This ticket was mentioned in PR #3719 on WordPress/wordpress-develop by @pbearne.

19 months ago

  • Keywords has-patch has-unit-tests added

#2 @costdev
19 months ago

  • Keywords dev-feedback added

force_ssl_admin( $force = null ) is documented to accept string|bool $force and to return bool.

PR 3719 breaks BC by hard deprecating the string type for $force.

IMO, the only "BC break" as such, should be that undocumented return types are no longer supported.


  • If no value is passed, it returns the stored value.
  • If a value is passed, it updates the stored value, and returns the old value.

As the function "Determines whether to force SSL used for the Administration Screens.", it doesn't make sense for it to return anything other than bool, so I'd suggest we don't look at altering documented return types, and instead focus on fixing the code.

From what I can tell at the moment (I'm on mobile - makes it a little difficult), this should be sufficient to ensure the function's signature remains intact, and the return value is of type bool as documented:

// Change this:
$forced     = $force;

// To this:
$forced     = (bool) $force;

Here's some tests:

Adding dev-feedback to gather thoughts on the preferred approach here.

#3 @autotutorial
18 months ago

Returning $forced is useful when using the default parameter $force = null but the first time even if you call it


define('FORCE_SSL_ADMIN', true );
force_ssl_admin( (bool) FORCE_SSL_ADMIN );
$variable = force_ssl_admin(); // second call for set value true

the value is false regardless if the constant it returns evaluates to true or false. After first call is not setting but getting value. This Makes sense unless you need to toggle between true and false or vice versa for a special, perhaps multisite setup.
Use casting explicit bool into your code

define('FORCE_SSL_ADMIN', 'false' );
function force_ssl_admin( $force = null ) {
        static $forced = false;

        if ( ! is_null( $force ) ) {
                $old_forced = $forced;
                $forced     = $force; // maybe here $forced = (bool) $force
                return $old_forced;

        return $forced;

$test1 = (bool) force_ssl_admin( (bool) FORCE_SSL_ADMIN );
$test2 = (bool) force_ssl_admin();
$test3 = (bool) force_ssl_admin();
$test4 = (bool) force_ssl_admin();

/*Expected Result:
var_dump( $test1, $test2, $test3, $test4 );

I'm with preference to adding explicit bool cast for the six uses of force_ssl_admin in core, feel free to comment ( without BC)

Version 30, edited 18 months ago by autotutorial (previous) (next) (diff)
Note: See TracTickets for help on using tickets.