View Issue Details

IDProjectCategoryView StatusLast Update
0018050mantisbtinstallationpublic2015-05-27 12:49
ReporterfoXen Assigned Todregad  
PrioritynormalSeveritytweakReproducibilityalways
Status closedResolutionno change required 
Product Version1.3.0-beta.1 
Summary0018050: Warnings on display_errors and display_startup_errors while there is all right
Description

Both checks in check_php_inc.php do "!ini_get_bool" instead of not negating that. So, you get a warning, while everything is OK (and vice versa?).

check_print_test_warn_row(
'display_errors php.ini directive is disabled',
!ini_get_bool( 'display_errors' ),
array( false => 'For security reasons this directive should be disabled on all production and Internet facing servers.' )
);

check_print_test_warn_row(
'display_startup_errors php.ini directive is disabled',
!ini_get_bool( 'display_startup_errors' ),
array( false => 'For security reasons this directive should be disabled on all production and Internet facing servers.' )
);

Steps To Reproduce

Simply extract new download anbd perform check.php

TagsNo tags attached.

Activities

foXen

foXen

2015-01-22 04:08

reporter   ~0042242

I'm sorry! So in my environment display_startup_errors IS set to "On"... and your warning is right. BUT: As it is display as:
<code>display_startup_errors php.ini directive is disabled
For security reasons this directive should be disabled on all production and Internet facing servers.</code>
This is (like all other warnings) not really clear, why this is a warning. First it is statet "x in php.ini IS disabled" - but this is only your internal test...

Would like to change the "Auswirkung" to "Unschönheit".

dregad

dregad

2015-01-22 07:16

developer   ~0042246

As you pointed out, this test works exactly as designed.

In terms of wording: the checks's first line should always be read as "Check that ...", and the following lines (in italics) indicate the outcome of the test, i.e. why it failed or triggered a warning.

I may be biased, but I think it's pretty clear. If you think otherwise, you're welcome to suggest a better wording.

Your last line is confusing (sorry my German is not so good) - you want to change the "Impact" to ???

atrol

atrol

2015-01-22 09:10

developer   ~0042248

Your last line is confusing (sorry my German is not so good) - you want to change the "Impact" to ???

He means: Set "Severity" to "Tweak"

foXen

foXen

2015-01-22 10:09

reporter   ~0042250

Yes it's only wording and therefor just a Tweak. :)

What about prefixing the first lines with something like "Check that " or "Checking: "? That way it's easier/simpler to understand for new mantis-admins and those that don't update regularly.

PS: Maybe also change this subject. :/

dregad

dregad

2015-01-22 11:14

developer   ~0042251

I don't really like the idea of prefixing the lines, it makes the output very repetitive without much benefit (not really clearer) my opinion ...

Check that xxxxx
Check that yyyyy
Check that zzzzz
...

foXen

foXen

2015-01-22 12:04

reporter   ~0042252

Your right, my last suggestion wasn't thought well. But what about replacing the word "is" with "should be"?
Well, looking on other messages, others should/could also be reworded... But there are better "check that"-messages like "Directory doc does not need to exist within the MantisBT root". Also without any further explanation every one understands this - hopefully.
Furthermore I did't checked the code, on how you implemented all this... But would really like to tweak this, so I'll take a more deeper lool on weekend.

dregad

dregad

2015-01-23 03:34

developer   ~0042254

OK. If you do come up with something better, I suggest you submit your proposed changes as a pull request on Github [1]

[1] https://github.com/mantisbt/mantisbt

dregad

dregad

2015-05-16 05:02

developer   ~0050764

Since you did not provide any feedback, I'm resolving this as no change required. Feel free to reopen this if and when you can propose specific changes