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

Can no longer install with Doctrine DBAL 2.5 #54

Closed
leofeyer opened this issue May 30, 2017 · 30 comments
Closed

Can no longer install with Doctrine DBAL 2.5 #54

leofeyer opened this issue May 30, 2017 · 30 comments
Labels

Comments

@leofeyer
Copy link
Member

Doctrine DBAL 2.5 has added the server_version parameter. It it is not given, Doctrine tries to read it from the MySQL server, thus enabling a database connection, which leads to an Access denied for user ''@'localhost' exception.

Related issue: kriswallsmith/assetic#681

@leofeyer
Copy link
Member Author

@contao/developers @fritzmg /cc

@aschempp
Copy link
Member

Doctrine tries to read it from the MySQL server

Who's Doctrine? The bundle? The bundle extension?

@Toflar
Copy link
Member

Toflar commented May 30, 2017

When does that error happen? When you call the install tool? The screenshot does not include the whole stack trace :)

@leofeyer
Copy link
Member Author

leofeyer commented May 30, 2017

The problem is not limited to the install tool. It occurs everywhere.

@leofeyer
Copy link
Member Author

@aschempp
Copy link
Member

The quickest solution would be to add a conflict with 2.5?

@aschempp
Copy link
Member

The solution is fairly simple. We cannot assume a complete installation without a working DB connection. Adjust to the following:

// class Contao\Config
public static function isComplete()
{
	if (static::$blnHasLcf === null || !static::has('licenseAccepted'))
	{
		return false;
	}

	try
	{
		\Database::getInstance()->listTables();
	}
	catch (\Exception $e)
	{
		return false;
	}

	return true;
}

@fritzmg
Copy link
Contributor

fritzmg commented May 30, 2017

@aschempp but this would introduce two additional database queries on each request.

@aschempp
Copy link
Member

True, we should probably perform the DB check only when we're in the install tool.

Another option would be to store isComplete in the container, but that's rather strange imho.

@leofeyer
Copy link
Member Author

This does not fix the issue, because the problem originates somewhere else.

https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Connection.php#L429

As you can see in the stack trace, the detectDatabasePlatform() method is causing the issue, because it tries to connect to the database before the database credentials have been set.

@aschempp
Copy link
Member

aschempp commented May 30, 2017

It does fix the issue in my installation. detectDatabasePlatform is only called because someone tries to connect, in my case it's the cron job listener checking if table tl_cron exists.

@leofeyer
Copy link
Member Author

Nope, sorry. Your fix is not working.

@leofeyer
Copy link
Member Author

And just so you see that the issue is not at all related to the install tool:

@aschempp
Copy link
Member

Well to me the install tool must work without DB, everything else cannot work. Idk why the command tries to connect to the database?

@leofeyer
Copy link
Member Author

Because Doctrine DBAL tries to connect to the database to retrieve the version number.

https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Connection.php#L429

And this already happens in ConnectionFactory::createConnection(), which is executed upon $container->get('doctrine.dbal.default_connection');.

And all of this happens way before the installation controller is even constructed!

@aschempp
Copy link
Member

In my case no connection to the database was made if noone queried something…

@fritzmg
Copy link
Contributor

fritzmg commented May 30, 2017

This error will probably only occur, if you do not allow anonymous connections to your database server (see also contao/core-bundle#736).

@leofeyer
Copy link
Member Author

Yes, that's true. If anonymous connections are allowed, the issue does not occur. But that's rarely the case fortunately.

@leofeyer
Copy link
Member Author

leofeyer commented May 30, 2017

I have commented here to document the issue.

@leofeyer
Copy link
Member Author

BTW, this issue also affects the managed edition:

@leofeyer
Copy link
Member Author

leofeyer commented May 30, 2017

It seems that our BinaryStringType is the reason why the getDatabasePlatform() method is called in the ConnectionFactory class. We might be able to register it in an compiler path to work around it.

@mattjanssen
Copy link

Sounds like your GUI config bundle needs to be able to handle when the DB credentials are missing or are no longer valid? Perhaps you could register a kernel.exception event listener that disables the doctrine config section once a DB ConnectionException is thrown. It could do this by either editing config.yml or switching to the "install" environment which would load config_install.yml. Finally it redirects the user with a RedirectResponse to the GUI admin login page with a nice error.

This is assuming your GUI config bundle is in the business of editing config.yml, which seems like a possibility :)

@leofeyer
Copy link
Member Author

@mattjanssen Thanks a lot for your input. Much appreciated.

@leofeyer
Copy link
Member Author

leofeyer commented May 30, 2017

We are now dynamically adding server_version: 5.1 in the Contao managed edition, if there is no database connection: contao/manager-bundle@06f1f29

@aschempp
Copy link
Member

aschempp commented May 31, 2017

@leofeyer should we add an is_array check in contao/manager-bundle@fd465bc#diff-e3375107bffc209723fa127bb186d8c6R146 ?

@leofeyer
Copy link
Member Author

If $extensionConfig['dbal']['connections']['default'] is set, it should always be an array, shouldn't it?

@Toflar
Copy link
Member

Toflar commented May 31, 2017 via email

@leofeyer
Copy link
Member Author

leofeyer commented May 31, 2017

No true in this case. If $extensionConfig['dbal']['connections']['default'] is not an array, the user has misconfigured the application.

doctrine:
    dbal:
        connections:
            default: 'string'  <--  this is wrong!

@aschempp
Copy link
Member

Unit tests would tell you :P

Not really, because the data is coming from other bundles. I think the unifying of TreeBuilder is only done after our method. According to https://github.com/doctrine/DoctrineBundle/blob/master/DependencyInjection/Configuration.php#L69 it's actually possible to set the connection details directly in doctrine.dbal but I think that's just an unsupported case for us then, especially because we know that the config loaded by manager-bundle will not be like that.

@fritzmg
Copy link
Contributor

fritzmg commented Jun 20, 2017

Is this actually fixed in Contao 4.4.0? I am unable to open the Install Tool once I have removed the anonymous user accounts from my SQL database. Stack trace seems to be the same as posted by @leofeyer

// nvm, suddenly it worked after clearing the dev cache

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

No branches or pull requests

5 participants