Why do forms need this specifically, forms should have full support for the symfony request class (which we use)? And if sites need this for integration, they can enable them themselves.

I fail to see reasoning why we should do this as your average novice php developer or a lot of mod authors will likely start using them if they are enabled (as it's what people do will by default if they don't know how about how use phpBB) which is a bad thing. If you don't know how to enable super globals, you shouldn't be enabling them.

Formerly known as Unknown Bliss

psoTFX wrote:
I went with Olympus because as I said to the teams ... "It's been one hell of a hill to climb"

There was a specific reason EXreaction pointed out to me on IRC, but I don't remember exactly how long ago it was (it's been at least a week, I think) and I don't feel like digging.

I cannot remember specifically which class had the issue, but the issue is that there is a class in Symfony Forms component that is ignorant of the Symfony request object and therefore uses $_SERVER to access a specific server variable. The method that causes the issue is at the end of a call stack with several functions in the list; to overwrite that class with one of our own would require overwriting several classes the stack. This is impractical and defeats the purpose of using Symfony in the first place.

Unless the Symfony class is somehow made aware of the Symfony request object, or we remove the whole disabling of super globals, the only way around this for us is to do:

This, in my opinion, is very hackish and ugly, and takes up unnecessary processing time.

The only real benefit of which I'm aware that comes from the current system is to allow us to force people to not use the PHP globals and instead use our own sanitized versions. If we remove the disabled super global restriction, sure some less experienced developers may use them, but we can still require use of the $request object in Extensions (and, of course, core code), and we don't have to do hackish things like above.

Note that doing what I showed in the example above was an unacceptable solution for a similar issue I encountered when I had to add the ability to create a Symfony Request object (because it, by default, creates it directly using the globals). However, in that case, there was an alternate way of creating the Symfony Request object (using the static create() method and supplying the values myself using $request->get_super_global(); reference). In this case, there really isn't a way around it that I can see.

EDIT: And even if we find a solution that does not include removing the "disable super globals" functionality, I guarantee you this issue will continue to pop up in the future as we try to integrate more and more 3rd party stuff. I doubt it's worth the headache.

I do custom MODs. PM for a quote!View My: MODs | PortfolioPlease do NOT contact for support via PM or email.
Remember, the enemy's gate is down.

There is a part in the Symfony Validator which checks $_SERVER directly (get content type or something like that) and causes this issue to come up. Many classes would need to be overwritten to be able to use the validator and it would take several hours of work. Basically the only way to get it to work now is to disable then re-enable super globals during the validation step for forms.

This isn't the first time we've run into this with libraries and certainly isn't going to be the last. This could be worked around by hours of work and a lot of ugly code, but sometime we're going to find it isn't possible at all.

So the only reason to cause all these issues is to force people to use best practices? Doesn't seem worth it to me. If they write insecure code and it's not caught during a review process, it's gonna happen anyway.

imkingdavid wrote:For reference, I think NativeRequestHandler is the class in question. The use of $_SERVER is intended. I don't think there's a way to easily use a different Request Handler instead.

The description indicates to me that you should not be using this handler at all. Use another handler or implement your own?

Frug wrote:So the only reason to cause all these issues is to force people to use best practices? Doesn't seem worth it to me. If they write insecure code and it's not caught during a review process, it's gonna happen anyway.

I strongly disagree. This is not about consciously producing insecure code. Good developers know best practices and bad developers (usually just novice people with little experience) will have to at least consciously enable super globals in order to use them. Ideally, they would get a link to best practices documentation when trying to use super globals. They can then decide whether they want to continue on the bad route by enabling super globals or do it properly. The request framework also enforces the type, so they will have to think about that too.