Volt Replacing Single Quote with Double Quote

[kgrammer]

I am setting the following variables in my VOLT file:

 {% set login_gtm_class = 'class="bl_ga_tag"' %}

This was working as expected in RC2 and produced:

    <?php $login_gtm_class = 'class="bl_ga_tag"'; ?>

But now in RC3 I'm getting the following:

    <?php $login_gtm_class = "class="bl_ga_tag""; ?>

The code is replacing the single quotes with double quotes which is breaking everything.

The original behavior needs to be returned.

Details

• Phalcon version: 5.0.0RC3
• PHP Version: 8.1.8 (Zend Engine v4.1.8)
• Operating System: Ubuntu 22.04
• Installation type: Compiling from source
• Zephir version (if any): 0.16.0
• Server: Apache
• Other related info (Database, table schema): N/A

Additional context Nothing more is required.

[Deathamns]

Also double quotes now interpret PHP variables. I had to put (external) JSON data to an attribute, but one of my variable name was the part of a JSON property name. Maybe single quotes would be safer in Volt.

// oversimplified example
{% set today = date('Y-m-d') %}
<div{{ ' data-json=\"{_$today: 1}\"' }}"></div>

// translated to (which obviously leaked the value of $today, and caused parsing error in JS):
<div data-json="<?= "{_$today: 1}" ?>"></div>

[borisdelev]

... one more thing: {% set divider = '<i class="right chevron icon divider"></i>' %} - this return error. When switch quotes works, but need to be fixed.

[niden]

Resolved in https://github.com/phalcon/cphalcon/pull/16024

[fichtner]

Personally having this completely destroy volt templates generated HTML with PHP parser errors I'd think "status: medium" seems a little shy of how bad this actually was especially during RC phase.

Cheers, Franco

[niden]

Personally having this completely destroy volt templates generated HTML with PHP parser errors I'd think "status: medium" seems a little shy of how bad this actually was especially during RC phase.

Cheers, Franco

Actually it will not. The latest commit reverted everything to what it was before, therefore existing templates will work as expected.

The only difference is that if one is using the TagFactory will use double quotes and only that

{{ tag.title("\t", "\n\n") }}

<?= $this->tag->title("\t", "\n\n"); >

The reason it was set to medium was because of that - it does not break backwards compatibility.

[fichtner]

@niden

Our experience differs from that:

[27-Jul-2022 07:37:17 Etc/UTC] ParseError: syntax error, unexpected identifier "readonly" in /usr/local/opnsense/mvc/app/cache/_usr_local_opnsense_mvc_app_views_layout_partials_form_input_tr.volt.php:20

The code generated was definitely wrong:

<?= ((empty($readonly) ? (false) : ($readonly)) ? "readonly="readonly"" : "") ?>

Now I don't know if that's a different issue but we reverted to RC2 and there it's fine. I guess we will see after RC4 or the release is out.

[kgrammer]

I recently updated to the latest RC3 and the original single quote issue has been resolved in RC3 now.

[kgrammer]

@fichtner

I meant to add that I think in your case, the issue is a PHP parsing error and not a Phalcon bug. PHP can't handle quotes inside of quotes. The PHP parsing is ending with your second double quote.

I would change the code to the following (note single quote around 'readonly'):

<?= ((empty($readonly) ? (false) : ($readonly)) ? "readonly='readonly'" : "") ?>

I believe that will solve your error.

[fichtner]

Err, but all versions of Phalcon up until RC2 did it in a way that it didn't break the code Phalcon generated: <?= ((empty($readonly) ? (false) : ($readonly)) ? 'readonly="readonly"' : '') ?> and I don't see how this won't break in the future changing doubles to singles if nobody will implement proper escaping such as "readonly=\"readonly\"". It's not magic.

[niden]

@fichtner Let me run a test on this. Maybe something was missed.

What is the exact volt code if you don't mind?

[fichtner]

@niden https://github.com/opnsense/core/blob/da562090dfc80c41b1eb35da7dca358f2fcf939e/src/opnsense/mvc/app/views/layout_partials/form_input_tr.volt#L69

[niden]

Thank you bud. I will get that test going today and report back.

[niden]

@fichtner I run the whole template using RC3 and this is what I got

<tr id="row_<?= $id ?>" <?php if ((empty($advanced) ? (false) : ($advanced)) == 'true') { ?> data-advanced="true"<?php } ?>>
    <td>
        <div class="control-label" id="control_label_<?= $id ?>">
            <?php if ((empty($help) ? (false) : ($help))) { ?>
                <a id="help_for_<?= $id ?>" href="#" class="showhelp"><i class="fa fa-info-circle"></i></a>
            <?php } elseif ((empty($help) ? (false) : ($help)) == false) { ?>
                <i class="fa fa-info-circle text-muted"></i>
            <?php } ?>
            <b><?= $label ?></b>
        </div>
    </td>
    <td>
        <?php if ($type == 'text') { ?>
            <input  type="text"
                    class="form-control <?= (empty($style) ? ('') : ($style)) ?>"
                    size="<?= (empty($size) ? ('50') : ($size)) ?>"
                    id="<?= $id ?>"
                <?= ((empty($readonly) ? (false) : ($readonly)) ? 'readonly="readonly"' : '') ?>
                <?php if ((empty($hint) ? (false) : ($hint))) { ?>placeholder="<?= $hint ?>"<?php } ?>
            >
        <?php } elseif ($type == 'hidden') { ?>
            <input type="hidden" id="<?= $id ?>" class="<?= (empty($style) ? ('') : ($style)) ?>" >
        <?php } elseif ($type == 'checkbox') { ?>
            <input type="checkbox"  class="<?= (empty($style) ? ('') : ($style)) ?>" id="<?= $id ?>">
        <?php } elseif ($this->isIncluded($type, ['select_multiple', 'dropdown'])) { ?>
        <select <?php if ($type == 'select_multiple') { ?>multiple="multiple"<?php } ?>
            <?php if ((empty($size) ? (false) : ($size))) { ?>data-size="<?= $size ?>"<?php } ?>
                id="<?= $id ?>"
                class="<?= (empty($style) ? ('selectpicker') : ($style)) ?>"
            <?php if ((empty($hint) ? (false) : ($hint))) { ?>data-hint="<?= $hint ?>"<?php } ?>
                data-width="<?= (empty($width) ? ('334px') : ($width)) ?>"
                data-allownew="<?= (empty($allownew) ? ('false') : ($allownew)) ?>"
                data-sortable="<?= (empty($sortable) ? ('false') : ($sortable)) ?>"
                data-live-search="true"
            <?php if ((empty($separator) ? (false) : ($separator))) { ?>data-separator="<?= $separator ?>"<?php } ?>
        ></select><?php if ((empty($style) ? ('selectpicker') : ($style)) != 'tokenize') { ?><br /><?php } ?>
            <a href="#" class="text-danger" id="clear-options_<?= $id ?>"><i class="fa fa-times-circle"></i> <small><?= $lang->_('Clear All') ?></small></a>
            <?php if ((empty($style) ? ('selectpicker') : ($style)) == 'tokenize') { ?>&nbsp;&nbsp;<a href="#" class="text-danger" id="copy-options_<?= $id ?>"><i class="fa fa-copy"></i> <small><?= $lang->_('Copy') ?></small></a>
                &nbsp;&nbsp;<a href="#" class="text-danger" id="paste-options_<?= $id ?>" style="display:none"><i class="fa fa-paste"></i> <small><?= $lang->_('Paste') ?></small></a>
            <?php } ?>
        <?php } elseif ($type == 'password') { ?>
            <input type="password" autocomplete="new-password" class="form-control <?= (empty($style) ? ('') : ($style)) ?>" size="<?= (empty($size) ? ('50') : ($size)) ?>" id="<?= $id ?>" <?= ((empty($readonly) ? (false) : ($readonly)) ? 'readonly="readonly"' : '') ?> >
        <?php } elseif ($type == 'textbox') { ?>
            <textarea class="<?= (empty($style) ? ('') : ($style)) ?>" rows="<?= (empty($height) ? ('5') : ($height)) ?>" id="<?= $id ?>" <?= ((empty($readonly) ? (false) : ($readonly)) ? 'readonly="readonly"' : '') ?>></textarea>
        <?php } elseif ($type == 'info') { ?>
            <span  class="<?= (empty($style) ? ('') : ($style)) ?>" id="<?= $id ?>"></span>
        <?php } ?>
        <?php if ((empty($help) ? (false) : ($help))) { ?>
            <div class="hidden" data-for="help_for_<?= $id ?>">
                <small><?= $help ?></small>
            </div>
        <?php } ?>
    </td>
    <td>
        <span class="help-block" id="help_block_<?= $id ?>"></span>
    </td>
</tr>

[niden]

Note the line:

<?= ((empty($readonly) ? (false) : ($readonly)) ? 'readonly="readonly"' : '') ?>

I also wrote a test just for that and it passes just fine here:

https://github.com/niden/cphalcon/commit/99925b1ce1da9e3f7138bb006149a0dbf1ab3d28

[niden]

I am going to release a new RC today, so if you don't mind let's check this again with the new version so as not to have issues like this. Changing and refactoring volt templates is not something I want developers to do.

[fichtner]

@niden I'll give it a try as soon as the next RC is out. Thanks!

[kgrammer]

@niden Likewise, I'll also update and test when the new RC release is available!

[fichtner]

@niden RC4 looks fine from here. Will continue to monitor for the rest of the week. Thanks for your help!

[niden]

Thank you gents. I am closing this one for now but please report back if anything is not as it should be.

[kgrammer]

Just wanted to close the loop by adding that my initial error has indeed been resolved in 5.0.0RC4.

As always, I will open a new ticket if anything else pops up.