[9.x] Fixes blade tags issue #45424 🔧

[imanghafoori1]

solves #45424

The solution seems a bit hacky, but the idea, however, is simple:

• Without changing the regex pattern, we perform our first search to get matches.
• Then by using PHP tokenizer, we check if we are in trouble or not.
• If we are in trouble, we try to fix it by adding the next candidate section to the matched string to see if we can reach the proper closing parenthesis or not.
• We try the same process until we find the proper match for the closing parenthesis of the blade tag.

@include ( 
  'partial1', ['hell)o' => ')-)--)']  
)

in this example, regex finds the :
@include ( 'partial1', ['hell) first and the loop adds these portions until it reaches the closing:
)o' => '
)-
)--
'] ) <=== the closing one.

Performance:

• Let me note that this part of the code is NOT run during each and every request/response life cycle since the compiled versions of blade files get cached.

• Some tests are included to prove the fix.

As I know, Taylor likes to find a proper regex for it and does not like such solutions, maybe the regex can be found somewhere in open-source IDE syntax highlighters like VS code, or people from Jetbrain can help since they can highlight the syntax properly.

[taylorotwell]

If we are going to attempt to solve that issue should we not add a new test ensuring that issue is actually fixed?

[imanghafoori1]

Yeah, I will add more tests, currently, I have changed the previous test which proves the bug to indicate the fix.
But for now, it is a WIP. I should mark this as a draft. Slight tweaks are needed.

[imanghafoori1]

@taylorotwell I think I am done with this PR for now.

[imanghafoori1]

The first commit solves the problem for closing chars (without touching the original regex)
like: @include('a', [' :)))) '])

The second commit solves the problem for opening chars (but tweaks the original regex)
like: @include('a', [' :(((( '])

So if you want to stay really safe and avoid touching the regex, you can keep the first and discard the second one.

• Changing regex causes it to stop as soon as it reaches the first closing parenthesis, and do not match the opening and closing ones.

[taylorotwell]

Can you provide some insight about what happens if it never finds a proper closing parenthesis?

Also, do you have some before / after benchmarks of running view:cache with this change on a code base with a fairly large amount of templates?

[imanghafoori1]

Since the user has to provide valid PHP syntax as input for the blade tags, it means that the closing parenthesis should be found or there is a syntax error in the first place, but yeah it is good to have a test about the result and the thrown error.

• Performance:
  I do not know how I should demo performance difference, but let's keep in mind:
  The new code is only executed if the regex fails to find a good match, so if you have an application with a lot of blade files that do not step over the bug, the tokenizer and all the added logic never get run even when you build the cache.

From experience, I know that the tokenizer is extremely fast. My laravel-microscope package runs the entire application code through tokenizer and all processes the tokens and this usually happens in a fraction of a second on a 10-years-old home PC.

[taylorotwell]

Alright. Can you maybe add some sort of test case or behavior description for what happens if there is never a closing parenthesis found?

[imanghafoori1]

I added the test for the unclosed blade tag. It seems that they are considered a tag with no parenthesis.

[timacdonald]

I spent some time trying to break this today, but it seems to be holding up to everything I've thrown at it.

I also booted up a decently sized Blade only project from a few years ago that uses all the older style blade bits (i.e. not <x-whatever) and it seemed happy.

I ran php artisan view:cache on a loop for 100 iterations against the project with the compiler before and after this PR. The results:

Before: 11.34 seconds
After: 11.98 seconds

Don't think there is anything performance-wise to worry about.

[rubenvanerk]

I think I may have found an edge case. The code below is broken on v9.47+, but works on v9.46.

<div>
    <span @class([
        'lt-text-xs lt-inline-block lt-py-1 lt-px-2 lt-rounded',
        'lt-text-green-600 lt-bg-green-50' => !@empty($success),
        'lt-text-yellow-600 lt-bg-yellow-50' => !@empty(
            $warning
        ),
        'lt-text-red-600 lt-bg-red-50' => !@empty($danger),
    ])>
        {{ $value }}
    </span>
</div>

This gets compiled to:

<span class="<?php echo \Illuminate\Support\Arr::toCssClasses([
        'lt-text-xs lt-inline-block lt-py-1 lt-px-2 lt-rounded',
        'lt-text-green-600 lt-bg-green-50' => !@empty($success),
        'lt-text-yellow-600 lt-bg-yellow-50' => !<?php if(empty(
            $warning
        )): ?>,
        'lt-text-red-600 lt-bg-red-50' => !<?php if(empty($danger)): ?>,
    ]) ?>">

And throws the error syntax error, unexpected token "<"

Context: lunarphp/lunar#822

[imanghafoori1]

@rubenvanerk I think the fix is somewhat easy.
Thanks for your report.