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

[Session] Error when updating to 3.3.11 on session handler. #24941

Closed
gmponos opened this issue Nov 13, 2017 · 7 comments
Closed

[Session] Error when updating to 3.3.11 on session handler. #24941

gmponos opened this issue Nov 13, 2017 · 7 comments

Comments

@gmponos
Copy link
Contributor

gmponos commented Nov 13, 2017

Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no
Symfony version 3.3.11

Hello there. I have the following test class.

namespace Tests\SmokeTests;

use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
use Symfony\Component\DependencyInjection\Container;
use Symfony\Component\DependencyInjection\ContainerInterface;

class ServicesTest extends WebTestCase
{
    /**
     * @var Container|ContainerInterface
     */
    private static $container;

    /**
     * @var array
     */
    private static $ignoreServices = [
        'lexik_jwt_authentication.encoder.lcobucci',
        'monolog.activation_strategy.not_found',
        'monolog.handler.fingers_crossed.error_level_activation_strategy',
    ];

    /**
     * @inheritdoc
     */
    public static function setUpBeforeClass()
    {
        parent::setUpBeforeClass();
        self::bootKernel();
        self::$container = self::$kernel->getContainer();
    }

    /**
     * Smoke test that checks whether all the services
     * load properly or not
     */
    public function testSmokeTestServiceLoading()
    {
        $serviceIds = self::$container->getServiceIds();
        foreach ($serviceIds as $serviceId) {
            if (in_array($serviceId, self::$ignoreServices, true)) {
                continue;
            }

            $service = self::$container->get($serviceId);
            $this->assertNotNull($service);
        }
    }
}

The goal of this Test Class is to detect if in your Yaml if you have declared services in a wrong way. Something like this:

services:
    app.myservice:
        class: NotExisting\Namespace
    app.yourservice:
        class: Existing\Namespace

The above will throw a test error when it will try to fetch app.myservice.

After updating to 3.3.11 my tests are failing because of this:

1) Tests\SmokeTests\ServicesTest::testSmokeTestServiceLoading
RuntimeException: Failed to set the session handler because headers have already been sent by "/MY_PATH/vendor/phpunit/phpunit/src/Util/Printer.php" at line 134.

/MY_PATH/vendor/symfony/symfony/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php:394
/MY_PATH/vendor/symfony/symfony/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php:120
/MY_PATH/var/cache/test/appTestDebugProjectContainer.php:4294
/MY_PATH/vendor/symfony/symfony/src/Symfony/Component/DependencyInjection/Container.php:329
/MY_PATH/tests/SmokeTests/ServicesTest.php:58
@linaori
Copy link
Contributor

linaori commented Nov 13, 2017

Possibly related to #24934

@sroze
Copy link
Contributor

sroze commented Nov 13, 2017

I don't think it's related to #24934 but it's another issue. We've introduced this for PHP 7.2 compatibility, which obviously makes the application breaking now. I'm not sure why we've introduced it, to be honest.

@sroze
Copy link
Contributor

sroze commented Nov 13, 2017

@gmponos just to double-check... if you simply comment out the throw in vendor/symfony/symfony/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php:394, is your application working as expected or PHP complains about something?

@gmponos
Copy link
Contributor Author

gmponos commented Nov 13, 2017

is your application working

The smoke tests above are passing. I have to test further if the rest application works as expected since it's an API application.

Is the session also started when I have an API in symfony?

@fabpot
Copy link
Member

fabpot commented Nov 13, 2017

@gmponos Can you test the fix and confirm it solves your issue?

@gmponos
Copy link
Contributor Author

gmponos commented Nov 13, 2017

Commenting this code part manually at 3.1.11 version passes the above smoke tests. I can't checkout to 2.* branch because my application is using 3.1.*

Also my end-to-end (functional) tests are passing either having the code commented either not.

@gmponos
Copy link
Contributor Author

gmponos commented Nov 13, 2017

Basically we need to be clear here. The above test all it does is getting the service ids and checks whether it can initiate the services.

Maybe another service id initiated the session and that is why the above test is failing.

I already have some ignoreServices for extreme cases like this. This might not even be a bug I just mentioned it here for confirmation in order for me to be sure that nothing broke.

Also this is my session config:

    session:
        # handler_id set to null will use default session handler from php.ini
        handler_id:  ~
        save_path:   "%kernel.project_dir%/var/sessions/%kernel.environment%"

nicolas-grekas added a commit that referenced this issue Nov 13, 2017
…kas, sroze)

This PR was merged into the 2.7 branch.

Discussion
----------

[HttpFoundation] Fix session-related BC break

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #24941, #24934, #24947 and #24946
| License       | MIT
| Doc PR        | -

Conservative fix.

Commits
-------

38186aa [HttpFoundation] Add test
3eaa188 [HttpFoundation] Fix session-related BC break
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

7 participants