Skip to content

Conversation

HeahDude
Copy link
Contributor

Q A
Branch? 3.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets ~
License MIT
Doc PR ~

@chalasr
Copy link
Member

chalasr commented Mar 2, 2020

Good catch, thanks @HeahDude.

@chalasr chalasr merged commit dcf3da8 into symfony:3.4 Mar 2, 2020
@HeahDude HeahDude deleted the security-fix-xsd-34 branch March 2, 2020 12:37
nicolas-grekas added a commit that referenced this pull request Mar 12, 2020
…ahDude)

This PR was merged into the 4.4 branch.

Discussion
----------

[SecurityBundle] Minor fix in LDAP config tree builder

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | ~
| License       | MIT
| Doc PR        | ~

Continuation of #35910 for 4.4.

Commits
-------

468a201 [SecurityBundle] Minor fix in LDAP config tree builder
This was referenced Mar 27, 2020
nicolas-grekas added a commit that referenced this pull request Apr 18, 2020
…figurations (zek)

This PR was merged into the 3.4 branch.

Discussion
----------

[SecurityBundle] fix accepting env vars in remember-me configurations

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #36271
| License       | MIT
| Doc PR        | -

As @wouterj explained we cannot use env variables after #35910 merged.

> Hmm, so I'm guessing this is what happens:
>
> * `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)

Commits
-------

46c2783 [SecurityBundle] fix accepting env vars in remember-me configurations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants