Which version were you upgrading from? Can you provide a small reproducing app that worked before and doesn't anymore please? That'd help ppl that want to debug this (could be you btw, up to?)

Which version were you upgrading from?

I was upgrading from 3.4.38.

Can you provide a small reproducing app that worked before and doesn't anymore please?

OK I can try.

Here is the repo https://github.com/zek/symfony-bug

I didn't complete rest of the reproduction.
I've just added the configuration which causes error.

What I'm trying to do is
use env variables in security.yaml remember_me config. So there is no special config actually.

Hmm, so I'm guessing this is what happends:

• lifetime is now an integerNode()
• For the Config component (which IIRC doesn't know anything about env variables), you're passing a string: "%env(int:REMEMBER_ME_COOKIE_LIFETIME)%"
• This throws an error, although if it wouldn't, the DI component would sucessfully process the string into a integer before it's used by any PHP class.

So we either make Config aware of environment variables (that's probably a huge feature) or we revert the integerNode() changes (as you suggested).

@HeahDude am I mislooking something, or would reverting these 2 lines not result in much harm? (only a little less strict config processor)

Yes we should revert.

Also as @wouterj explained the Config component does not know about env processors...

It means that any node, excepted for ScalarNode, has potentially an issue with env variables which will be always read as strings by the loaders, an array could be %env(json:file:...)% etc.
Sounds very complex to try to handle this from any component, and probably too much to allow strings on every nodes...

@zek as you were the one reporting and discovering the lines that broke this, are you maybe up for a PR to revert the change on the 3.4 branch? That would give you all credits you deserve :)

It means that any node, excepted for ScalarNode, has potentially an issue with env variables which will be always read as strings by the loaders, an array could be %env(json:file:...)% etc.
Sounds very complex to try to handle this from any component, and probably too much to allow strings on every nodes...

Yes, this is also what I was thinking about. I think it might be possible to "solve" it a little bit if Config would recognize that it is an env string and discover the env processor. There are env processors for each type. E.g. if something is %env(int:...)%, we can be sure it'll be an integer when it is processed by DI.

This would make the Config component aware of the fullstack framework though. I'm not sure how to fix that, but this could be a way of solving it for all config nodes.

4.4 is already able to know about these, but not 3.4 indeed.

Oh, so in that case, the revert should only happen in 3.4?

I suppose yes, can anyone confirm on 4.4?

I can confirm that I no longer have this error when I update the reproducer to Symfony 4.4 (thanks for providing a reproducer, that makes it very easy to confirm/test!)

@zek as you were the one reporting and discovering the lines that broke this, are you maybe up for a PR to revert the change on the 3.4 branch? That would give you all credits you deserve :)

Ok. I'll send a PR today. Thank you all @wouterj @HeahDude @nicolas-grekas 👍

friendly ping @zek

I can see this was fixed for 3.4, but it exists on 4.4+. How come it's not a problem on newer versions? Because we have exactly the same problem on Symfony 4.4 (mautic/mautic#9011)

Please don't comment on closed issues/PRs, you might get no follow up.
Open an issue instead, or a PR maybe?