Skip to content

Conversation

jjanusch
Copy link

@jjanusch jjanusch commented Jul 20, 2022

This PR addresses an issue we were experiencing with Facades. We're upgrading an old project from Laravel 5.5 to 9.x that utilizes a few facades to generate code, specifically Laravel Collective's Form Builder. If you run this:

{{ Form.open() }}

it will autoescape the tag. This had previously not escaped the HTML for years on older releases, producing forms as expected.

I did a few hours of troubleshooting and determined the cause was this line:

$is_safe = ($is_safe && (is_string($result) || (is_callable($result) && method_exists($result, '__toString'))));

Form.open() returns an HtmlString object which has a __toString() method. The expectation is it would use that method to convert it to a String, but that doesn't happen because is_callable($result) should never return true on an object. Instead, it is intended to be run as:

is_callable([$result, '__toString']);

Unfortunately, switching this one line to just use that instead of the combined is_callable() && method_exists() resulted in some unexpected behavior. The commit that added this check said it was to fix a PHP 8 TypeError, and my change to usuing is_object() instead of is_callable() should satisfy the type errors, fix the bugs we're seeing, and maintain expected behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant