Status code should be 500 on error

[sophie-h]

Steps to reproduce

1. Make config file write protected
2. Visit Nextcloud main page

Expected behaviour

The server should return status code 500 to alert standard monitoring systems. (Or ignore, that the config is not writeable)

Actual behaviour

The server returns status code 200

Server configuration

Operating system:
Debian

Web server:
Apache 2

Database:
Postgresql

PHP version:
5.6

Nextcloud version: (see Nextcloud admin page)
12

[Morikko]

I can take this issue.

[Morikko]

The function OC_Util::checkServer that arrives after checkConfig also tracks if config is writable.

• OC_Util::checkServer tracks config/ only
• checkConfig tracks config/config.php

Is it necessary to have those two checks ?

Should I do a unit test for checking the header status ? If yes, in which test file should I add it ?

[sophie-h]

I think it is completely sufficient to have an error when changing the configuration fails. I don't know if a proper handling in this case already exists.

Having php files non-writable is a sane security decision for me. However, if writable config files are recommended, this information would fit into the "Admin" –> "Security & setup warnings" section.

[tflidd]

There is even an option for the config file in case the config file is read only:
https://github.com/nextcloud/server/blob/master/config/config.sample.php#L648-L658

Having php files non-writable is a sane security decision for me. However, if writable config files are recommended, this information would fit into the "Admin" –> "Security & setup warnings" section.

I would prefer this option as well. Tell user to either set the option "read-only" in the config file or make it writable. Most people are quite responsive to messages in Security & setup warnings. A 500 status code error will trigger many users to end up with a broken setup and they will complain that the update procedure broke their system.

[Morikko]

Notes:

• A first check is done in lib/base.php with function checkConfig(), this one is handle completely.
• A second one in lib/private/legacy/util.php with function checkServer. This one only check the folder. Thus, it can throw an error if config folder is not writable but config file is writable. Moreover, the http status returned was already 503.

Questions:

• Should I change the http status returned with the second check to 500 ?
• Should we open a new issue to correct the behavior of the second check to focus on the file instead of the folder ?

[MorrisJobke]

Should I change the http status returned with the second check to 500 ?

This one is fixed in #9995 - inspired by @Morikko

And @Morikko's PR was in #7205 - rebased as #9996