Side Effects from including bootup.utf8.php

[dongilbert]

When including your bootup file, the script sanitizes global vars with UTF-8. While I'm all for what you're trying to accomplish, as a framework this isn't something we could force on our downstream users. Would it be plausible to only run the input sanitization chunk of your bootup script if a constant was defined or something similar?

defined('PATCHWORK_SANITIZE_GLOBALS') or define('PATCHWORK_SANITIZE_GLOBALS', true)

if (PATCHWORK_SANITIZE_GLOBALS)
{
    // run script here
}

[nicolas-grekas]

Rather than adding constants like you asked for, I tried to move bootup stages in several namespaced functions. This should make it easier for you to take only what you want. See commit https://github.com/nicolas-grekas/Patchwork-UTF8/commit/b58f08b2e791d45226c4d86a6ad5def1a79b9f43 That should fix your problem isn't it?

[dongilbert]

The main concern I had was that it filters the request vars and uri. The referenced commit allows me to pick which parts of the package to use, but it still automatically filters the request vars.

I'm sure there's good reason for this. I'll need to talk it out with my team again, there was some concern about pushing this default behavior onto downstream consumers of our framework without an easy way to disable it, if needed.

[nicolas-grekas]

You should check it with your team yes. Personnaly on my framework, as I decided that UTF-8 is mandatory, these filter are also mandatory. Adding a way to disable the filter is now your responsability, and very easy: instead of including bootup.utf8.php, copy past its current content and tweak it to your needs.

[dongilbert]

I would agree that would work, but if I include the package via composer, then the bootup.utf8.php file is automatically included, so we can't get around it that easily.

[dongilbert]

Since the package is meant for broad consumption, those two items really should be optional, IMO. Those using it will have to do additional setup now that you've made the parts more modular and opt-in, so, it stands to reason that it wouldn't be THAT much more of a "burden" to have to specifically enable these two additional items.

I think it would make this a viable option for a lot more people if you were to make it all optional and easy to configure instead of forcing a good but not always desired option on end users.

[nicolas-grekas]

Ok, I've just committed https://github.com/nicolas-grekas/Patchwork-UTF8/commit/e325c204eb86fca93b223b79388708aa4e0d3824 which mandates manual bootup configuration and relies on PSR-0 to load the classes.

[nicolas-grekas]

Cross posted an issue for Laravel concerning the update to v1.1 https://github.com/laravel/framework/issues/997

[dongilbert]

Very cool - thanks for this update. Makes the code much more portable. :+1: