Skip to content
This repository has been archived by the owner on Jan 2, 2020. It is now read-only.

prevent fatal error when posix_getpwuid is disabled #103

Closed
wants to merge 4 commits into from

Conversation

fritzmg
Copy link
Collaborator

@fritzmg fritzmg commented Sep 16, 2016

Problem

The following code generates a Fatal error when the posix_getpwuid function is disabled, which some shared hosters do as a "security" measure:

// Add the posix_getpwuid function
if (!function_exists('posix_getpwuid')) {
    function posix_getpwuid($int) {
        return array('name'=>$int);
    }
}

Thus you cannot use the Contao Check in order to check the system requirements, install Contao or validate an existing Contao installation on such hostings.

See #86, #93, #100, #101.

Reproduction

Simply add

disable_functions = posix_getwpuid

to your PHP.ini and access the Contao Check (any PHP version).

Explanation

  1. function_exists returns false if a function is either not declared or disabled. It will return true otherwise.
  2. functions that are disabled are still declared.
  3. you cannot redeclare functions.

Proposed fix

This PR adds additional checks for a disabled posix_getpwuid function, thus not generating a fatal error anymore when you open the Contao Check. Instead it will display messages which explain that this function is in the list of disabled functions and thus file permissions cannot be checked.

screen shot 2016-09-16 at 15 24 08

screen shot 2016-09-16 at 15 24 40

Note: Since the check for the composer client also checks the file permissions, I removed that <li> from the index.phtml whenever the posix_getpwuid function is disabled (since you cannot say whether or not you can use the composer client then).

Translations

As mentioned in #102, I am not sure what the best practice for creating the appropriate *.pot translation files is. Thus any translation adaptions are yet missing from this PR.

@leofeyer
Copy link
Member

First of all, I still think that we do not need to handle this case, because we cannot really do anything about the initial problem. You are just replacing one error message with another.

Of course your error message is nicer than the current one. 😄

Note: Since the check for the composer client also checks the file permissions, I removed that <li> from the index.phtml whenever the posix_getpwuid function is disabled (since you cannot say whether or not you can use the composer client then).

Then we should say "the composer client requirements cannot be checked" instead of silently removing the check, shouldn't we?

@fritzmg
Copy link
Collaborator Author

fritzmg commented Sep 16, 2016

First of all, I still think that we do not need to handle this case, because we cannot really do anything about the initial problem.

Yes of course. But I mean, the Contao Check in general is all about showing problems with the current server environment that the user then has to fix (if he wants to use Contao, or any of the additional features). In this case it's a problem that prevents the checking of other problems, but still 😄

Then we should say "the composer client requirements cannot be checked" instead of silently removing the check, shouldn't we?

True, I'll update the PR accordingly with the same principle.

@fritzmg
Copy link
Collaborator Author

fritzmg commented Sep 16, 2016

I have updated the PR accordingly.

screen shot 2016-09-16 at 16 50 48

screen shot 2016-09-16 at 17 03 07

@barrystaes
Copy link

This looks great! Doesnt Contao require more functions that according to some popular best security practices could be in disable_functions? E.g. those i listed in this table #101 (comment)

@leofeyer
Copy link
Member

Merged in fbcf1e7.

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

Successfully merging this pull request may close these issues.

None yet

3 participants