fuel / core

Fuel PHP Framework - The core of the Fuel v1 framework
http://fuelphp.com
813 stars 345 forks source link

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

Open Illirgway opened 5 years ago

Illirgway commented 5 years ago

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 commented 5 years ago

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.