[5.1] Fix for issue #10068

[mcgrogan91]

This merge fixes the issue (#10068) by adding a blade-specific version of the e() htmlentities shortcut. If we are echoing content in a blade file, we should likely be escaping any '@' characters we are echoing, as that character has special meaning to the Blade Interpreter. This change adds a blade escape helper function that replaces '@' with '@' - the htmlentities version of the character when it is inside of blade echo tags.

There is likely a more Blade-centric way to do this, but as the issue has existed for a while now I figured I would at least submit a fairly straightforward fix for this.

[GrahamCampbell]

Please squash to one commit.

[GrahamCampbell]

Also, the email address you used on git needs adding to your GitHub so the commits get associated with you.

[mcgrogan91]

Alright, think I squashed it and linked my account correctly.

[rentalhost]

Can you add tests that should fail without your fix?

[taylorotwell]

I agree I would like to see a test or two that fails without the fix.

[mcgrogan91]

I'm not sure where I would put an overall test for this fix, as my implementation changes the BladeCompiler. The full test won't show a fix until the Factory processes the compiled output and no longer sees '@parent'. I'm not sure where to write a single test that covers that process, so I wrote a couple tests for the various steps of it.

I'm open to any input on where to write a full test for this.

[vlakoff]

I think be() is too easily confusable with the verb "to be"… Also, this name might be wanted for something else in the future (or in user code)? How about blade_e() or e_blade()?

[vlakoff]

btw, to reduce verbosity, maybe it's time to replace <?php echo to <?=. The package itself requires PHP 5.5, and uses PHP 5.4 array notation, so…

[rentalhost]

I think that be() should not exists, but a static function can be created to do that service. Except if be() can be useful directly to user like e() does.

@vlakoff I think that use full php tag is better. Else, should be need replace it in all Laravel.

[vlakoff]

• No opinion about helper vs. static method, haven't looked into it enough.
• Searched for <?php echo in laravel/framework codebase. Only BladeCompiler.php and a few test fixtures contain it.

[taylorotwell]

Yeah I wouldn't call this be I would also suggest making a static method. This isn't really a user-land function we want to make available globally.

[mcgrogan91]

I like pulling it out into the static function rather than have it be user land global. It makes more sense here, since it's specific functionality related to an internal tool. Thanks for the PR, @rentalhost! I can squash these down again if this is a solution that might be accepted.

[rkgrep]

Shouldn't this be implemented in 5.2?
It may break existing apps that are expecting raw @ in views. For instance, in mailto links.

[GrahamCampbell]

I agree this belongs on 5.2, if at all.

[rentalhost]

mailto shouldn't be affected, is very important to test it.
I think that it it is happening, should be avoided, instead of just wait to implement it on 5.2, because it acts like a bug (as in #10068).

[rkgrep]

mailto is just an example. It may be some JS code that depends on @ symbol either.

[rentalhost]

Exactly. In this case, I suggests make more tests avoiding 'hacks'. Maybe it causes more problem that solve others.

[GrahamCampbell]

👎 for ths.

[GrahamCampbell]

We need to just fix the actual issue instead of a workaround that's a bit dodgie.