8000 Symfony LockHandler throws wrong error · Issue #22882 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Symfony LockHandler throws wrong error #22882

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.

Sign up for GitHub

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

Jump to bottom
Closed
Xendiadyon opened this issue May 24, 2017 · 3 comments
Closed

Symfony LockHandler throws wrong error #22882

Xendiadyon opened this issue May 24, 2017 · 3 comments
Labels
Bug Filesystem Status: Needs Review Status: Waiting feedback Unconfirmed

Comments

@Xendiadyon
Copy link
Xendiadyon commented May 24, 2017 •
edited
Loading
Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? ?
Symfony version 2.6.6 - 2.6.8
(Edit: I meant 3.2.6-3.2.8)

In https://github.com/symfony/filesystem/blob/master/LockHandler.php the error_handler is deactivated from lines 70-82. Thus, if an error is thrown in line 85-86, the error-message is wrong.

Found in contao/core-bundle upon symlink generation: contao/core-bundle#845
The system tries to open a specific file in /tmp but has no read/write access due to malconfiguration (?). A Critical Exception is thrown, but the error-message does not fit to the error.
@Xendiadyon Xendiadyon mentioned this issue May 24, 2017
Closed
@linaori
Copy link
Contributor
linaori commented May 24, 2017

Note that you're reporting this bug for symfony 2.6.* versions, which are not supported anymore (for a long time). Can you verify if this is the same with 2.7?

The fil 8000 e you linked is in the master, but in 2.6, the behavior is different: https://github.com/symfony/filesystem/blob/2.6/LockHandler.php#L69
https://github.com/symfony/filesystem/blob/2.7/LockHandler.php#L72

@Xendiadyon
Copy link
Author
Xendiadyon commented May 24, 2017 •
edited
Loading

Sorry, I totally confused the versions.
I was working with version 3.2.8 and downgraded to version 3.2.6. Both showed this problem.

image
I have some files in the /tmp directory which are not readable, the file permissions are somehow scrambled (this probably will be fixed as soon as the hoster cleans the /tmp directory).

So I would expect a "fopen() Permission denied" error.
Indeed, when I uncomment the "silencing" of the error-reporting on line 72, I get a few of these error notifications since I cannot open/read/chmod this file (although it exists).
grafik

But, the silencing of the error reporting triggers a mistake:
When in this section 72-82 the error-reporting is silenced, the errors are not handled.
When there is a file permission problem, the handler $this->handle would not be set.
This is checked on line 84.
Here, it throws an error (which is good) but the message is crap (a "critical notice"):

[2017-05-23 18:38:57] request.INFO: Matched route "contao_install". {"route":"contao_install","route_parameters":{"_scope":"backend","_token_check":true,"_controller":"Contao\\InstallationBundle\\Controller\\InstallationController::installAction","_route":"contao_install"},"request_uri":"https://***.de/contao/install","method":"GET"} []
[2017-05-23 18:38:57] app.CRITICAL: An exception occurred. {"exception":"[object] (Symfony\\Component\\Filesystem\\Exception\\IOException(code: 0): Using the Contao\\RequestToken class has been deprecated and will no longer work in Contao 5.0. Use the Symfony CSRF service via the container instead. at /kunden/***/www/vendor/symfony/symfony/src/Symfony/Component/Filesystem/LockHandler.php:86)"} []

This notice "Using the Contao\RequestToken class has been deprecated and will no longer work in Contao 5.0." has nothing to do with the error which is triggered.
This is since the $error = error_get_last(); on line 85 fetches the last notice (which is not important at all) and not the true error (which was "silenced" due to shutting down the error handling in the few lines before)

@xabbuh xabbuh mentioned this issue May 25, 2017
Merged
@xabbuh
Copy link
Member
xabbuh commented May 25, 2017

@Xendiadyon Can you check if #22910 improves the error message for you?

nicolas-grekas added a commit that referenced this issue May 28, 2017
@nicolas-grekas
bug #22910 [Filesystem] improve error handling in lock() (xabbuh)
3ac8c5b 
This PR was merged into the 2.7 branch.

Discussion
----------

[Filesystem] improve error handling in lock()

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

Commits
-------

25381a2 [Filesystem] improve error handling in lock()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Filesystem Status: Needs Review Status: Waiting feedback Unconfirmed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@nicolas-grekas @linaori @xabbuh @Xendiadyon @carsonbot
0