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

2.6 - Fix e-notice in modifier.escape.php #892

Closed
wants to merge 1 commit into from

Conversation

colemanw
Copy link

@colemanw colemanw commented Aug 4, 2023

A null value will cause e-notices in most of the esc_types. Functions like htmlspecialchars, str_replace, preg_replace etc do not strictly allow null as an argument.
Using an early-return if the value is empty() should also give a subtle performance boost, as none of the esc_types do anything meaningful to empty strings or the number 0.

@scottchiefbaker
Copy link

Could this be simplified to return "";?

@colemanw
Copy link
Author

colemanw commented Aug 4, 2023

Could this be simplified to return "";?

No because '0' is also empty().

@scottchiefbaker
Copy link

Ah good point.

@colemanw
Copy link
Author

colemanw commented Aug 4, 2023

As far as I can see there is no benefit to escaping integers either, and I'm not sure how the various escape functions that expect a string will cope with an integer (if not currently, maybe in the future they would thrown an e-notice?) so IMO this could be further improved as

if (empty($string) || is_int($string)) {
    return (string) $string;
}

But I didn't do that because this is a legacy project so I went for the minimal patch.

@wisskid
Copy link
Member

wisskid commented Aug 4, 2023

Smarty v2 really isn't supported anymore. Don't run it in production, it probably has a bunch of security issues.

@wisskid wisskid closed this Aug 4, 2023
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.

3 participants