framework
framework copied to clipboard
[12.x] Switch Js::encode() to return HtmlString
Since it missed 11 in https://github.com/laravel/framework/pull/49641, let's try again and get it into 12. 🤞
Updating the Js::encode()
helper to return an instance of HtmlString
rather than a raw string.
The primary reason is to support using the normal escaping Blade tags: {{ Js::encode() }}
, rather than {!! Js::encode() !}}
, when using this in Blade. This will then match the behaviour already possible with Js::from()
which returns an instance of Htmlable
and supports {{ Js::from() }}
.
Since this is a breaking change, it will need to go into ~v11~ v12 - hence the master
branch.
There are no downsides to this change, it may just require minor code updates - which should be trivial to identify and perform with a search for Js::encode(
.
P.s. Is this the first PR into 12?
P.s. Is this the first PR into 12?
Think so yeah
@taylorotwell gonna mark this ready for you again if that's okay.
What would the upgrade guide need to contain?
The upgrade guide will need to advise that Js::encode()
no longer returns a raw string
, but instead an instance of Htmlable
.
I can see three scenarios that will be affected:
- All uses of
{{ Js::encode() }}
will no longer be escaped. There is a chance this could introduce an XSS vector if the JSON encoding doesn't break HTML structures within it's values. It may also introduce vulns or just break, if the app is unescaping the output - as it would no longer be escaped. - Uses of
{!! Js::encode() !!}
will continue to work as expected, although devs can update to{{ Js::encode() }}
and it will continue to work the same. - Code that relies on
Js::encode()
returning specifically astring
value will break. They will need totoString
or typecast the output.
@valorin if you need a new review, don't forget to mark the PR as ready.
Marking this as draft for a bit.
I wonder this change seems to be a big backward incompatibility.
Given:
$foo = ['bar' => 'baz'];
<div data-foo="{{ Js::encode($foo) }}">
Before:
<div data-foo="{"bar":"baz"}">
<!-- div.dataset.foo returns a valid JSON string '{"bar":"baz"}' -->
After:
<div data-foo="{"bar":"baz"}">
<!-- div.dataset.foo returns '{' and bar":"baz"} will be a garbage -->
We should add a cast to escape correctly...
<div data-foo="{{ Js::encode($foo)->toString() }}">
Force to use {!! !!}
is reasonable for me.
Example:
<script type="application/ld+json">{!! Js::encode($ld) !!}</script>