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

Database named as 'New' causes PHP fatal error #14698

Closed
MauricioFauth opened this issue Oct 31, 2018 · 9 comments
Closed

Database named as 'New' causes PHP fatal error #14698

MauricioFauth opened this issue Oct 31, 2018 · 9 comments
Assignees
Labels
Bug A problem or regression with an existing feature has-pr An issue that has a pull request pending that may fix this issue. The pull request may be incomplete help wanted
Projects
Milestone

Comments

@MauricioFauth
Copy link
Member

Describe the bug

Database named as 'New' causes PHP fatal error.
This is dependent on the language that has been set. See screenshots.

To Reproduce

Steps to reproduce the behavior:

  1. Create a database called 'New'
  2. Click on the 'New' database in the navigation menu
  3. See error

Expected behavior

The database should open like any other.

Screenshots

db1
db2

Server configuration

  • Operating system: Linux Mint 19
  • Web server: PHP 7.2.11 Server
  • Database version: MariaDB 10.3.10
  • PHP version: PHP 7.2.11
  • phpMyAdmin version: QA_4_8 and master

Client configuration

  • Browser: Google Chrome
  • Operating system: Linux Mint 19

Additional context

PHP Fatal error:  Uncaught Error: Call to undefined method PhpMyAdmin\Navigation\Nodes\Node::getHiddenItems() in libraries/classes/Navigation/NavigationTree.php:581
Stack trace:
#0 libraries/classes/Navigation/NavigationTree.php(355): PhpMyAdmin\Navigation\NavigationTree->_addDbContainers(Object(PhpMyAdmin\Navigation\Nodes\Node), '', 0)
#1 libraries/classes/Navigation/NavigationTree.php(309): PhpMyAdmin\Navigation\NavigationTree->_buildPathPart(Array, '', 0, '', 0)
#2 libraries/classes/Navigation/NavigationTree.php(891): PhpMyAdmin\Navigation\NavigationTree->_buildPath()
#3 libraries/classes/Navigation/Navigation.php(67): PhpMyAdmin\Navigation\NavigationTree->renderPath()
#4 navigation.php(79): PhpMyAdmin\Navigation\Navigation->getDisplay()
#5 {main}
  thrown in libraries/classes/Navigation/NavigationTree.php on line 581
@MauricioFauth MauricioFauth added Bug A problem or regression with an existing feature help wanted labels Oct 31, 2018
@computerManiac
Copy link

Hey, can I try solving this ?

@MauricioFauth
Copy link
Member Author

MauricioFauth commented Nov 12, 2018

@computerManiac Of course you can. Don't forget to use QA_4_9 branch as the base branch.

@nmilo
Copy link
Contributor

nmilo commented Jan 20, 2019

Some info on this issue. First parameter passed to _addDbContainers function should be of class NodeDatabase instead of Node. I will try to trace this issue further.

@williamdes williamdes self-assigned this Apr 29, 2019
@williamdes williamdes added this to to be fixed soon in issues Apr 29, 2019
@williamdes williamdes removed their assignment Jun 17, 2019
@williamdes
Copy link
Member

$cfg['ShowCreateDb'] = false;

and the error disappears

@nina-py
Copy link
Contributor

nina-py commented Sep 14, 2020

This is a curious bug - I have just figured out where exactly the error originates from. As the initial report says, the name of the database has to match the string "New" in English or whatever string this is translated to in other locales for the fatal error to appear. It is case-sensitive as well so probably doesn't really happen in the wild that often given that database names are usually lower case.

An array of NodeDatabase objects is successfully created and added as children to the node tree, so the correct NodeDatabase object for the "New" database does exist in the tree. We just never get it back. What happens instead is the following:

  • NavigationTree->buildPath() iterates through the NodeDatabase objects and calls NavigationTree->buildPathPart() for each of them, passing path[0] ("New") for the database in question.

  • NavigationTree->buildPathPart() runs $db = $this->tree->getChild($path[0]); (line 380), which iterates through the children and gets the first node with the name "New" back, which happens to be the top one that leads you to the page where you can create a new database, NOT the database with the same name.

  • This returned child node is of class Node, not NodeDatabase, so on line 388 $this->addDbContainers() falls over with a fatal error.

I'm not quite sure what to do with a tree that has two nodes with the same name (and the same realName property as well), perhaps adding a check in Node->getChild() for the type of object that is expected back is warranted?

Hope this helps!

@williamdes
Copy link
Member

Hi @nina-py

Thank you so much for your precious debug !, could you try to fix this issue using our QA_5_0 branch ?

@nina-py
Copy link
Contributor

nina-py commented Sep 14, 2020

Hi @williamdes, I'm not sure what would be a good fix for it though as I'm just getting familiar with the codebase. A possible workaround would be to block users from creating databases named with the magic "New" keyword?

I forgot to add that a very similar issue can occur one level down - if you try to create a table named "New", the nav tree breaks with a fatal error in NavigationTree::addTableContainers(). I think some sort of decision of what is best - restrict user input or fix the node tree is needed for both of these issues. Restricting user input so that the "New" keyword cannot be used probably has the least potential of introducing new bugs into the code.

@williamdes
Copy link
Member

A possible workaround would be to block users from creating databases named with the magic "New" keyword?

Well, we can not do that because they can create databases using their command line :)

I think we need to find the real cause of such a bug rather than limiting our users because of problems they do not care about ;)

@nina-py
Copy link
Contributor

nina-py commented Sep 14, 2020

Well, we can not do that because they can create databases using their command line :)

Good point! There is no choice but to fix the node tree then :).

@williamdes williamdes self-assigned this Sep 15, 2020
@williamdes williamdes added the has-pr An issue that has a pull request pending that may fix this issue. The pull request may be incomplete label Sep 15, 2020
@williamdes williamdes added this to the 5.0.3 milestone Sep 15, 2020
williamdes added a commit that referenced this issue Sep 15, 2020
…l error

Pull-request: #16354
Fixes: #14698

Signed-off-by: William Desportes <williamdes@wdes.fr>
williamdes added a commit that referenced this issue Sep 15, 2020
Signed-off-by: William Desportes <williamdes@wdes.fr>
issues automation moved this from to be fixed soon to Closed Sep 15, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A problem or regression with an existing feature has-pr An issue that has a pull request pending that may fix this issue. The pull request may be incomplete help wanted
Projects
issues
  
Closed
Development

No branches or pull requests

5 participants