[5.1] Distinction between real @parent and echoed content

[dmgawel]

This PR fixes security issue: @parent from view data was treated as real @parent.
This is an idea for non-breaking approach without restructuring everything. Needs review and tests.

---

Closes #10068.

[GrahamCampbell]

Broken build.

[GrahamCampbell]

Could you add tests that failed before and now pass please?

[dmgawel]

Yes, but not until tomorrow (UTC+2).

[dmgawel]

@GrahamCampbell ?

[GrahamCampbell]

can you add another test that checks parent isn't converted anymore too please?

[GrahamCampbell]

@taylorotwell I wonder if this change should be made on 5.2 and not 5.1, since it's a massive BC break, and the security implications are only minor.

[taylorotwell]

What is the massive BC break?

[GrahamCampbell]

People might be relying on the fact that the parent processing is done at the factory level, and not at the compiler level. That's quite a big change to make.

[taylorotwell]

Can you give an example of how it would break an application?

[GrahamCampbell]

{{ '@parent' }} - that used to work, now doesn't

[taylorotwell]

Thanks <3

[GrahamCampbell]

Thank you very much @dmgawel. :)

[abhimanyu003]

Thanks very much 👍

[franzliedke]

Great job! 👏

[atmediauk]

Not sure if this is intentional and will stay like this but before if you had:

a layout file with
@yield('scripts')

a child file which extends layout with

@section('scripts')
@parent
<script>
    $(document).ready(function() {
     //foo
    });
</script>
@stop

and then the child above also had multiple @include at some point which had their own:

@section('scripts')
@parent
<script>
    $(document).ready(function() {
      //bar
    });
</script>
@stop

All the @Sections were passed along correctly but now the @section inside the @include is ignored.

I found this really helpful in scenarios where for example I had a media manager page. This media manager had multiple sections (tabs). One to upload, another to attach markup, another to embed etc. To help keep things manageable each tabs code and its related scripts section were in a separate @include file.

I think with this merge now all the scripts have to be bunched up in to the childs @section which isn't a big problem, but it previously sectioned off nicely :(

[taylorotwell]

Can you give us the full views to try out?

On Friday, September 4, 2015, atmediauk notifications@github.com wrote:

Not sure if this is intentional and will stay like this but before if you
had:

a layout file with
@yield('scripts')

a child file which extends layout with

@section('scripts')
@parent

<script> $(document).ready(function() { //foo }); </script>

@Stop

and then the child above also had multiple @include
https://github.com/include at some point which had their own:

@section('scripts')
@parent

<script> $(document).ready(function() { //bar }); </script>

@Stop

All the @Sections https://github.com/sections were passed along
correctly but now the @section https://github.com/section inside the
@include https://github.com/include is ignored.

I found this really helpful in scenarios where for example I had a media
manager page. This media manager had multiple sections (tabs). One to
upload, another to attach markup, another to embed etc. To help keep things
manageable each tabs code and its related scripts section were in a
separate @include https://github.com/include file.

I think with this merge now all the scripts have to be bunched up in to
the childs @section https://github.com/section which isn't a big
problem, but it previously sectioned off nicely :(

—
Reply to this email directly or view it on GitHub
#10122 (comment).

[taylorotwell]

This PR has been reverted due to multiple issues with existing applications. It can be attempted on 5.2. We also need to stop referring to it as a security issue since we have no known security exploit related to this bug.

[atmediauk]

Thanks for that Taylor, will put together a more complete example of what I meant.

https://gist.github.com/atmediauk/65e9bb437691c85a9560

In the gist above the @section('scripts') in upload-form.blade.php is no longer added.

Thanks again :)