New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FrameworkBundle] Add sid_length and sid_bits_per_character session ini options in session configuration #30027

Merged
merged 1 commit into from Feb 8, 2019

Conversation

@XuruDragon
Copy link
Contributor

XuruDragon commented Jan 29, 2019

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #29830
License MIT
Doc PR n/a

this a fix for the issue #29830

After deliberation, we estimate that only sid_length and sid_bits_per_character session options should be exposed. These options à optional.

For others, we recommend changing your php.ini file

We can now configure the session like this :

framwork:
  session:
    sid_length: 64 //optional, recommended value is 32
    sid_bits_per_character: 6 //optional, recommended value is 5
@derrabus

This comment has been minimized.

Copy link
Contributor

derrabus commented Jan 29, 2019

In #29830, @pravdinalex reported that the 3.4 branch lacks support for those options as well. Was this PR intentionally opened against the master branch?

@OskarStark

This comment has been minimized.

Copy link
Contributor

OskarStark commented Jan 29, 2019

As this is a new "feature" it must be against master per definition, right?

@derrabus

This comment has been minimized.

Copy link
Contributor

derrabus commented Jan 29, 2019

As this is a new "feature"

Is it? The new feature has been implemented in php itself and the HTTP Foundation component already supports it. One could argue that this PR merely solves a configuration issue of the Framework Bundle.

I don't have a clear opinion on this. I've just noticed a contradiction between the linked issue and the PR and thought I should mention it. 😃

@XuruDragon

This comment has been minimized.

Copy link
Contributor Author

XuruDragon commented Jan 30, 2019

yes indeed, my first intention was to make it a bugfix for the 3.4 branch.

But as it can change the behavior of the application and can be set in the application, it sounds like a new feature. But I'm open to change it.

@XuruDragon XuruDragon changed the title [FrameworkBundle] Add sid_length and sid_bits_per_character session ini options in session configuration [WIP] [FrameworkBundle] Add sid_length and sid_bits_per_character session ini options in session configuration Jan 30, 2019

@XuruDragon XuruDragon force-pushed the XuruDragon:issue-29830 branch from 34f0d92 to 69a8c7b Jan 30, 2019

@XuruDragon XuruDragon changed the title [WIP] [FrameworkBundle] Add sid_length and sid_bits_per_character session ini options in session configuration [FrameworkBundle] Add sid_length and sid_bits_per_character session ini options in session configuration Jan 30, 2019

@XuruDragon XuruDragon force-pushed the XuruDragon:issue-29830 branch from 69a8c7b to 73ca6f1 Jan 30, 2019

@XuruDragon XuruDragon force-pushed the XuruDragon:issue-29830 branch 2 times, most recently from eb91f57 to 32ad132 Jan 30, 2019

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Jan 30, 2019

That's a new feature to me. Reports that ask for bug fixes but really ask for feature are not uncommon :)

XuruDragon added a commit to XuruDragon/symfony-docs that referenced this pull request Jan 30, 2019

@XuruDragon XuruDragon force-pushed the XuruDragon:issue-29830 branch 2 times, most recently from dd20abf to 0ea374c Feb 1, 2019

@XuruDragon XuruDragon force-pushed the XuruDragon:issue-29830 branch from 0ea374c to 23888fa Feb 7, 2019

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Feb 8, 2019

rebase needed

[FrameworkBundle] Add sid_length and sid_bits_per_character session i…
…ni options in configuration

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #29830
| License       | MIT
| Doc PR        | n/a

this a fix for the issue #29830

After deliberation, we estimate that only `sid_length` and` sid_bits_per_character` session options should be exposed. These options à optional.

For others, we recommend changing your php.ini file

We can now configure the session like this :

```yaml
framwork:
  session:
    sid_length: 64 //optional, recommended value is 32
    sid_bits_per_character: 6 //optional, recommended value is 5
```

@XuruDragon XuruDragon force-pushed the XuruDragon:issue-29830 branch from 23888fa to 0403e4a Feb 8, 2019

@XuruDragon

This comment has been minimized.

Copy link
Contributor Author

XuruDragon commented Feb 8, 2019

rebase needed

done

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Feb 8, 2019

Thank you @XuruDragon.

@nicolas-grekas nicolas-grekas merged commit 0403e4a into symfony:master Feb 8, 2019

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
fabbot.io Your code looks good.
Details

nicolas-grekas added a commit that referenced this pull request Feb 8, 2019

feature #30027 [FrameworkBundle] Add sid_length and sid_bits_per_char…
…acter session ini options in session configuration (XuruDragon)

This PR was merged into the 4.3-dev branch.

Discussion
----------

[FrameworkBundle] Add sid_length and sid_bits_per_character session ini options in session configuration

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #29830
| License       | MIT
| Doc PR        | n/a

this a fix for the issue #29830

After deliberation, we estimate that only `sid_length` and` sid_bits_per_character` session options should be exposed. These options à optional.

For others, we recommend changing your php.ini file

We can now configure the session like this :

```yaml
framwork:
  session:
    sid_length: 64 //optional, recommended value is 32
    sid_bits_per_character: 6 //optional, recommended value is 5
```

Commits
-------

0403e4a [FrameworkBundle] Add sid_length and sid_bits_per_character session ini options in configuration

@XuruDragon XuruDragon deleted the XuruDragon:issue-29830 branch Feb 8, 2019

javiereguiluz added a commit to symfony/symfony-docs that referenced this pull request Feb 11, 2019

minor #10944 Update the documentation for the PR #30027 (XuruDragon)
This PR was merged into the master branch.

Discussion
----------

Update the documentation for the PR #30027

See symfony/symfony#30027

Added `sid_length` and `sid_vits_per_character` options

Commits
-------

8e44f8e Update the documentation for the PR #30027
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment