Fixed Bug in Response::charset() method

[Emanuele]

Affected version

2.1.0 - Release Candidate 2

Class Response (src/XF/Http/Response.php), method charset(), around line 66:

    public function charset($charset = null)
    {
        if ($charset === null)
        {
            return $charset;
        }

        $this->charset = $charset;

        return $this;
    }

I suppose that calling charset() without arguments should return the current charset, so the correct code should be:

    public function charset($charset = null)
    {
        if ($charset === null)
        {
            return $this->charset;
        }

        $this->charset = $charset;

        return $this;
    }

 

[DragonByte Tech]

If anything, I’d say the return should always be $this so as to not break method chaining, usually there’s a dedicated getter if you want to read protected properties.

 

[Emanuele]

the return should always be $this so as to not break method chaining

I don't see any getter method for charset information. And moreover, If you look at the class, you can see that in this case the behaviour is different, as methods act as setters if called with a parameter, and getters if called without parameters.
Lastly, if $charset variable is null, it doesn't make sense to return it as a variable, you can just return null.

 

[XF Bug Bot]

Thank you for reporting this issue. It has now been resolved and we are aiming to include it in a future XF release (2.1.0 RC3).

Change log:

Fix incorrect return value for Http\Response::charset() when no arguments are passed in.

Any changes made as a result of this issue being resolved may not be rolled out here until later.