[fix] Do not create .htaccess with Nginx #223
Conversation
| if ($writtenBytes === false || $writtenBytes < 19) { | ||
| throw new Exception('unable to write to file ' . $file, 11); | ||
| } | ||
| if ((strncmp("Apache", $_SERVER['SERVER_SOFTWARE'],6) == 0) or defined('_SERVER_APACHE')) { |
There was a problem hiding this comment.
Is _SERVER_APACHE meant as a way to circumvent this check for admins? If so I'd say this should maybe better be a configuration variable… If not then is this really an official variable set by Apache?
There was a problem hiding this comment.
Thanks for you're welcome 👍
It's not official variable, this variable may call like this define('_SERVER_APACHE', 'false');
Yes, it seems interesting to put it in the configuration, but I do not know how to do it the best.
There was a problem hiding this comment.
The unit tests fail due to the missing check if the key SERVER_SOFTWARE exists. Also we need to add tests that mocks the existence of that variable emulating both an apache and non-apache environments to ensure the .htaccess files get created in Apache and not in others.
I can look into writing these tests later some time during this week.
There was a problem hiding this comment.
@elrido What do you say about the environmental variable in general? I'd say changing it to a config option is better…
@magikcypress If you want to see how to add a config option, in this commit a config option was added.
|
Sorry, this PR is dirty ! |
| @@ -3,9 +3,6 @@ | |||
| ; An explanation of each setting can be find online at https://github.com/PrivateBin/PrivateBin/wiki/Configuration. | |||
|
|
|||
| [main] | |||
| ; (optional) set a project name to be displayed on the website | |||
| ; name = "PrivateBin" | |||
|
|
|||
There was a problem hiding this comment.
Please merge the current master into your project. You've accidentally reverted some commits/code changes we did in the master branch recently.
|
No, that's not necessary. A quick fix could be to just run A proper fix would be to update your master branch on GitHub either by GitHub's GUI or by using the cli and doing basically the same as above, but in the master branch. |
|
My bad ! I could not do it ! Sorry, I tried to merge the master on patch-1 :(
My merge doesn't work, and my changes are not made !? You have a idea ? |
|
After |
|
I managed to catch up with my mistakes ?! |
|
Thanks for these changes (especially fixing the indentation), but it seems your changes still revert some commits in the master branch… Either follow this guide again or – if it is easier for you – create a new PR. PS: For easy and always correct indentation you can use http://editorconfig.org/ – we support it. |
|
I will create a new PR tomorrow. |
|
😄 |
|
BTW when creating a new PR you can add a string in order to (automatically) let issues close when a PR is merged. |
It's probably a good patch, but it is to discuss this issue #222