Skip to content
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

[HttpFoundation] Failed to set session handler, headers already sent by phpunit #24524

Closed
anlutro opened this issue Oct 11, 2017 · 11 comments
Closed

Comments

@anlutro
Copy link
Contributor

anlutro commented Oct 11, 2017

Q A
Bug report? yes
Feature request? no
BC Break report? yes
RFC? no
Symfony version 3.4.x

I have a project that depends on symfony/http-foundation. With 3.4.x-dev I'm getting some RuntimeErrors recently:

RuntimeException: Failed to set the session handler because headers have already been sent
by ".../vendor/phpunit/phpunit/src/Util/Printer.php" at line 110.

.../vendor/symfony/http-foundation/Session/Storage/NativeSessionStorage.php:397
.../vendor/symfony/http-foundation/Session/Storage/NativeSessionStorage.php:112
.../classes/Http/SessionProvider.php:128
.../vendor/autarky/container/classes/Factory/Factory.php:89
.../vendor/autarky/container/classes/Container.php:256
.../vendor/autarky/container/classes/Container.php:226
.../classes/Application.php:511
.../tests/unit/Session/SessionProviderTest.php:20

I tracked down the line to #23711 which was merged 2 days ago. If I remove the if headers_sent thing from setSaveHandler, my test runs fine again.

PHPUnit 5.7.22, symfony/http-foundation is at 0fe9b3d

@nicolas-grekas
Copy link
Member

You're going to have this error anyway when running on PHP 7.2, so you should fix your tests.
Running them in a separate process should do the trick.

@sroze
Copy link
Contributor

sroze commented Oct 11, 2017

@nicolas-grekas I think the issue is a bit deeper. What's new in PHP 7.2 is that PHP won't allow you anymore to change the session's ini settings or save handlers after the session has been started.

This should be fixed in #24516 (i.e. you will get an exception only if you try to change things while creating the NativeSessionStorage object, not every time you create an instance of it 🤞) - but it's blocked by a remaining failing test.

nicolas-grekas added a commit that referenced this issue Nov 5, 2017
…e with PHP 7.2 (sroze)

This PR was merged into the 2.7 branch.

Discussion
----------

[HttpFoundation] Fix forward-compat of NativeSessionStorage with PHP 7.2

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #24524
| License       | MIT
| Doc PR        | ø

PHP 7.2 disallow setting session options when the session was already started. This PR will not set any option if the session is already started and throw an exception if trying to do so with custom options.

Commits
-------

00a1357 [HttpFoundation] Fix forward-compat of NativeSessionStorage with PHP 7.2
@garak
Copy link
Contributor

garak commented Nov 6, 2017

This should be merged in 3.4 branch too. I'm on a project with php 7.2/Symfony 3.4-beta3 and there's no way to get this working 😞

@garak
Copy link
Contributor

garak commented Nov 6, 2017

Also, I tried a dirty hack, by copying NativeSessionStorage class from #24531, and I'm still getting the same error ("Failed to set...")

@umpirsky
Copy link
Contributor

umpirsky commented Nov 6, 2017

Upgraded to v3.4.0-BETA3, same error I reported in #24800, which is probably a duplicate of this issue.

@nicolas-grekas I think we should reopen.

@nicolas-grekas nicolas-grekas reopened this Nov 6, 2017
nicolas-grekas added a commit that referenced this issue Nov 6, 2017
This PR was squashed before being merged into the 4.0-dev branch (closes #24516).

Discussion
----------

[Process] Fix broken tests for PHP 7.2

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | no
| Fixed tickets | #24524, #24515
| License       | MIT
| Doc PR        | ø

Following #24515, trying to fix Process tests with PHP 7.2

Commits
-------

b410a36 [Process] Fix broken tests for PHP 7.2
@tacman
Copy link
Contributor

tacman commented Nov 20, 2017

Any sort of hack we can use in the meantime?

@karser
Copy link
Contributor

karser commented Nov 20, 2017

I'm getting this error when APP_ENV=dev but the error goes away when APP_ENV=test.
Symfony v3.4.0-BETA4

@nicolas-grekas
Copy link
Member

would anyone be able to provide a short reproducer we could work on?

@anlutro
Copy link
Contributor Author

anlutro commented Nov 26, 2017

I did a composer update and the errors seem to have disappeared. I can do a git bisect to figure out what fixed it, if you want.

@anlutro
Copy link
Contributor Author

anlutro commented Nov 26, 2017

Looks like it was fixed in #24952. Closing.

@anlutro anlutro closed this as completed Nov 26, 2017
@anlutro
Copy link
Contributor Author

anlutro commented Dec 2, 2017

Maybe not.

<?php
class SessionTest extends PHPUnit_Framework_TestCase {
	public function test_session() {
		$h = new \Symfony\Component\HttpFoundation\Session\Storage\Handler\NativeFileSessionHandler();
	}
}
1) SessionTest::test_session
ini_set(): Headers already sent. You cannot change the session module's ini settings at this time

/usr/src/myapp/vendor/symfony/http-foundation/Session/Storage/Handler/NativeFileSessionHandler.php:52
/usr/src/myapp/tests/SessionTest.php:5

Should I re-open or create a new issue? It seems like almost the same issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants