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

DOCs: Maybe should change default filter for config uri_filter or note with warning #2131

Open
Illirgway opened this issue Oct 1, 2019 · 1 comment

Comments

@Illirgway
Copy link
Contributor

Illirgway commented Oct 1, 2019

Fuel Version: 1.8.2
https://fuelphp.com/docs/classes/security.html

If we configure as in "default" security.uri_filter = array('htmlentities') and use HTML5 for templates => configure security.htmlentities_flags as e.g. ENT_QUOTES | ENT_HTML5 (so use ENT_HTML5 for flags), due to
https://github.com/fuel/core/blob/1.9/develop/classes/security.php#L100
https://github.com/fuel/core/blob/1.9/develop/classes/security.php#L148

silently instead of htmlentities we call \Security::htmlentities, which use flags from security.htmlentities_flags
https://github.com/fuel/core/blob/1.9/develop/classes/security.php#L211

that leads complex uri path e.g. 'complex/path/for/controller' to (phpsandbox example):
http://sandbox.onlinephpfunctions.com/code/ab79a3fd6dc30023a02ae2749331b1929c9ad776

$uri = 'complex/path/for/controller';

var_dump(
    htmlentities($uri, ENT_HTML5, 'UTF-8', true)
);
// ==> string(39) "complex/path/for/controller"

Those, we get "/" instead of "/", which breaks further routing at all.

So we should either change default security.uri_filter filter to 'htmlspecialchars' or note by a warning that usage 'htmlentities' with 'ENT_HTML5' leads to unexpected behavior and totally breaks routing.

@WanWizard
Copy link
Member

The fact that a developer is using a framework doesn't mean the develop shouldn't know how PHP works, i.e. what the effect is of using the ENT_HTML5 flag, it isn't really related to Fuel perse.

I'd say add it as a note to the docs if absolutely needed.

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

No branches or pull requests

2 participants