Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.5] Add new json blade directive #21004

Merged
merged 2 commits into from
Sep 5, 2017
Merged

[5.5] Add new json blade directive #21004

merged 2 commits into from
Sep 5, 2017

Conversation

luiz-brandao
Copy link
Contributor

This PR introduces a new @json directive for Blade. The purpose behind this new directive is to make it easier to inject PHP data structures into JS code while being IDE error free.

Alternatives to this approach like {!! $json !!} or {!! json_encode($data) !!} raise syntax errors in some IDEs (PhpStorm) but @json($data) doesn't.

Usage example:
let foo = @json($foo);

Create a new concern for a new json statement
Use new CompilesJson concern
@m1guelpf
Copy link
Contributor

m1guelpf commented Sep 5, 2017

👍 for making easier to interact with Vue components

@mateusjatenee
Copy link
Contributor

I'd very much like this too.

@GrahamCampbell
Copy link
Member

What use case do you have for writing JSON in HTML btw?

@taylorotwell taylorotwell merged commit 264845d into laravel:5.5 Sep 5, 2017
@luiz-brandao
Copy link
Contributor Author

luiz-brandao commented Sep 5, 2017

@GrahamCampbell often, you'll find yourself in situations where you want to pass some server-side string/array/collection/whatever to your JavaScript. There are a few different ways to achieve that and outputing JSON is the most straightforward way.

@hackel
Copy link
Contributor

hackel commented Sep 11, 2017

It would be rather nice if this method automatically called jsonSerialize on Collections and Models.

@eberkund
Copy link

So this is like a built in replacement for this package?
https://github.com/laracasts/PHP-Vars-To-Js-Transformer

@luiz-brandao
Copy link
Contributor Author

@eberkund no, this blade directive is just a handy way to output json; the package you mention has other functionalities.

@luiz-brandao
Copy link
Contributor Author

luiz-brandao commented Sep 11, 2017

@hackel Eloquent models and Collections implement JsonSerializable, so jsonSerialize is implicitly called when we use json_encode.

@ryantology
Copy link
Contributor

This should be used with extreme caution due to XSS exploits. The previous method of putting json on the page is:
{!! $json !!} When you see someone use {!! !!} this is a red flag to check the input for XSS. Because the default version of @json() does not use any of the escaping options, this leaves the method open to XSS.

I would suggest that the default value be 15 as done in symfony JsonResponse:

    // Encode <, >, ', &, and " characters in the JSON, making it also safe to be embedded into HTML.
    // 15 === JSON_HEX_TAG | JSON_HEX_APOS | JSON_HEX_AMP | JSON_HEX_QUOT
    const DEFAULT_ENCODING_OPTIONS = 15;

@luiz-brandao
Copy link
Contributor Author

luiz-brandao commented Sep 19, 2017

I don't think the new default would be an issue. I could do a pull request if @taylorotwell agrees.

@franzliedke
Copy link
Contributor

@taylorotwell @themsaid Please consider @ryantology's warning above. IMO, this is the only safe default here. Right now, the @json directive is very much open for XSS exploits.

As with most other Blade features, this feature should offer a safe default, and offer the flexibility to employ less-safe variants to people who explicitly want that (and thus probably know what they are doing). This is already possible with the second $options argument.

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.

None yet

9 participants