Scalar fixes

[Cheddam]

PHP 8 limits method_exists() to accepting strings and objects, which was in particular impacting the /Security/ping endpoint (which returns an integer.)

I've included two changes:

• A minor improvement to Controller::prepareResponse() that handles scalar values directly, rather than passing them through checks only relevant to objects
• A fix to ClassInfo::hasMethod() that will enforce a false outcome if the provided value isn't a valid argument for method_exists(). I initially considered putting the same blanket is_scalar() check in place here, but that changes the behaviour when passing in FQCNs, which may be relied upon (despite the docblock clearly calling for an object.)

[emteknetnz]

Looks good, could just use some unit tests on the public method for integers, bools, etc

[sminnee]

If the functionality of ClassInfo::hasMethod() is broader than the docblock, and those extra uses are known in the wild, then it might be more honest to update the docblock go bless that extra case. Otherwise someone might break it in the future.

[sminnee]

It feels like there's a deeper issue here, though: why was the result of ping ending up in a method-exists check? That feels like there's an unguarded method execution that could potentially be tied to user input (ie security flaw) in there. It would be good to explore that.

[emteknetnz]

@sminnee It doesn't look like a security thing, this is where the call to the ClassInfo::methodExists comes from on a ping.

If we wanted to make it 'more nicer', then probably change the ping method to return a HTTPResponse::create()->setBody('1') rather than simply 1

[sminnee]

Right. We can probably add an is_object check to that screen cap'd ClassInfo::methodExists call. If a class name ends up in the response it would be erroneous to check it for a method.

[sminnee]

Returning a string '1' would also be nicer than the integer its currently returning, but you're right that returning an HTTPResponse object would be clearer in intent.

[emteknetnz]

PR to update ping to return an HTTPResponse #9745

I'm not sure that if (is_object($response) && ClassInfo::hasMethod($response, 'getViewer')) { is enough of an improvement, since $response being an instance (e.g. a controller) doesn't really seem that much better than $response being a string of the a controller class. Neither of them are a "response" :-) I'm not sure the upside here is worth the risk of introducing regressions since there will be a lot of old code paths that go through here, some which may have a classname as a response.

[Cheddam]

I've added some unit tests for the ClassInfo::hasMethod() method, and revised the new logic in Controller::prepareResponse() to use !is_object($response) over is_scalar($response), as this makes the intention a bit clearer (and covers the edge-case of arrays.) I could also loosen the docblock of both methods, but I'm a little torn. In general I'd rather we shift towards tighter interfaces over time, but these types of method do lend themselves to wider input (and the additional logic I'm adding here to harden it doesn't increase complexity too much.)