8000 [HttpFoundation] Deprecate session.upload_progress.* and url_rewriter.tags as valid configuration options. · Issue #43284 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[HttpFoundation] Deprecate session.upload_progress.* and url_rewriter.tags as valid configuration options. #43284

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

Closed
kafene opened this issue Oct 1, 2021 · 2 comments · Fixed by #43378

Comments

@kafene
Copy link
kafene commented Oct 1, 2021

Description
While perusing the source of HttpFoundation on this beautiful Friday evening, I noticed that it permits configuration of the session.upload_progress.* and url_rewriter.tags ini settings.

In the case of session.upload_progress.* - these are PHP_INI_PERDIR options and can not be configured at runtime with ini_set. Future Symfony versions should drop them as valid session options.

In the case of url_rewriter.tags - the PHP docs note that as of PHP 7.1.0, it is no longer used for session configuration and that setting is now session.trans_sid_tags.

Both the upload_progress and url_rewriter.tags options are still present in the 6.0 branch, so I'm suggesting that they be deprecated.

Example

> php -n -d'extension=readline.so' -d'session.upload_progress.enabled=On' -a
Interactive mode enabled

php > var_dump(ini_get('session.upload_progress.enabled'));
string(1) "1"
php > var_dump(ini_set('session.upload_progress.enabled', '0'));
bool(false)
php > var_dump(ini_get('session.upload_progress.enabled'));
string(1) "1"
@fabpot
Copy link
Member
fabpot commented Oct 2, 2021

@kafene Would you like to work on a PR deprecating these settings in 5.4 so that we can remove them in 6.0?

@kafene
Copy link
Author
kafene commented Oct 2, 2021

@fabpot I would be happy to.

kafene pushed a commit to kafene/symfony that referenced this issue Oct 8, 2021
…ssion options

Related to [issue symfony#43284](symfony#43284). Deprecate support for configuring session.upload_progress.* - these options are `PHP_INI_PERDIR` options and can not be configured at runtime via `ini_set`. Deprecate support for configuring `url_rewriter.tags` - as of PHP 7.1.0, this option is no longer used for session configuration and that setting is now `session.trans_sid_tags`. `url_rewriter.tags` is used only by `output_add_rewrite_var`.
kafene pushed a commit to kafene/symfony that referenced this issue Oct 8, 2021
…ssion options

Related to [issue symfony#43284](symfony#43284). Deprecate support for configuring session.upload_progress.* - these options are `PHP_INI_PERDIR` options and can not be configured at runtime via `ini_set`. Deprecate support for configuring `url_rewriter.tags` - as of PHP 7.1.0, this option is no longer used for session configuration and that setting is now `session.trans_sid_tags`. `url_rewriter.tags` is used only by `output_add_rewrite_var`.
@fabpot fabpot closed this as completed Oct 10, 2021
fabpot added a commit that referenced this issue Oct 13, 2021
…ewriter.tags session options (Matthew Covey)

This PR was squashed before being merged into the 5.4 branch.

Discussion
----------

[HttpFoundation] Deprecate upload_progress.* and url_rewriter.tags session options

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | yes
| Tickets       | Fix #43284
| License       | MIT

Related to [issue #43284](#43284).

Deprecate support for configuring session.upload_progress.* - these options are `PHP_INI_PERDIR` options and can not be configured at runtime via `ini_set`.

Deprecate support for configuring `url_rewriter.tags` - as of PHP 7.1.0, this option is no longer used for session configuration and that setting is now `session.trans_sid_tags`. `url_rewriter.tags` is used only by `output_add_rewrite_var`.

---

I wasn't sure about writing a test for this. I couldn't find any other places that deprecation notices were being tested. However the existing HttpFoundation test uses `url_rewriter.tags` and I can see the new deprecation notice when running `php ./phpunit src/Symfony/Component/HttpFoundation/` -- it may be appropriate to update that to use a new option.

Commits
-------

da02720 [HttpFoundation] Deprecate upload_progress.* and url_rewriter.tags session options
symfony-splitter pushed a commit to symfony/http-foundation that referenced this issue 6AA3 Oct 13, 2021
…ewriter.tags session options (Matthew Covey)

This PR was squashed before being merged into the 5.4 branch.

Discussion
----------

[HttpFoundation] Deprecate upload_progress.* and url_rewriter.tags session options

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | yes
| Tickets       | Fix #43284
| License       | MIT

Related to [issue #43284](symfony/symfony#43284).

Deprecate support for configuring session.upload_progress.* - these options are `PHP_INI_PERDIR` options and can not be configured at runtime via `ini_set`.

Deprecate support for configuring `url_rewriter.tags` - as of PHP 7.1.0, this option is no longer used for session configuration and that setting is now `session.trans_sid_tags`. `url_rewriter.tags` is used only by `output_add_rewrite_var`.

---

I wasn't sure about writing a test for this. I couldn't find any other places that deprecation notices were being tested. However the existing HttpFoundation test uses `url_rewriter.tags` and I can see the new deprecation notice when running `php ./phpunit src/Symfony/Component/HttpFoundation/` -- it may be appropriate to update that to use a new option.

Commits
-------

da027202b7 [HttpFoundation] Deprecate upload_progress.* and url_rewriter.tags session options
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
0