View Issue Details

IDProjectCategoryView StatusLast Update
0021124mantisbtadministrationpublic2016-07-09 19:28
Reporteratrol Assigned Todregad  
PrioritynormalSeveritymajorReproducibilityalways
Status closedResolutionfixed 
Product Version1.3.0-rc.2 
Target Version1.3.0Fixed in Version1.3.0 
Summary0021124: Creating/setting config options using adm_config_report.php is partially broken
Description

Trying to set options sometimes throws
APPLICATION ERROR # 103 Cannot set configuration option "option_name": No more tokens

It happens whenever a value should be set to a value which is considered to be empty in PHP [1].
e.g. try to set an integer value to 0

Caused by this piece of code in Tokenizer class
public function is_empty() {
return empty( $this->tokens );
}

[1] http://php.net/manual/en/function.empty.php

TagsNo tags attached.

Relationships

related to 0020787 closeddregad Setting of arrays (complex type) in Configuration Page doesn't work 

Activities

dregad

dregad

2016-06-16 13:02

developer   ~0053392

Are you only talking about trying to set a scalar value (i.e. not an array), e.g. leaving the 'Value' text area blank (= empty string), or setting value '0' (without the quotes) ?

Or did you notice the same problem using arrays too ? If so, I'd appreciate an example of string that triggers the exception.

atrol

atrol

2016-06-16 16:36

developer   ~0053395

try to set an integer value to 0

e.g.
Create option enable_email_notification with value 0 -> Error
Create option enable_email_notification with value 1 -> OK

dregad

dregad

2016-06-17 02:50

developer   ~0053399

I failed to consider the fact that '0' == 0 == emtpy().
PHP can be a bitch sometimes.

I'll also improve the handling of empty strings

atrol

atrol

2016-06-17 11:25

developer   ~0053403

Integer values like -1 do als not work
You get: Unexpected token "-"

dregad

dregad

2016-06-18 04:09

developer   ~0053404

Yes I noticed that while testing the issue with 0 values. The same behavior occurs with positive numbers (e.g. '+1').

I added a test case for this already, the fix should be quite simple.

dregad

dregad

2016-06-18 04:38

developer   ~0053405

PR for review & test: https://github.com/mantisbt/mantisbt/pull/796

Related Changesets

MantisBT: master-1.3.x afdd002a

2016-06-16 14:24

dregad


Details Diff
Config tests: add cases for '0' and '' (issue 0021124) Affected Issues
0021124
mod - tests/Mantis/ConfigParserTest.php Diff File

MantisBT: master-1.3.x 9636642a

2016-06-16 15:54

dregad


Details Diff
Tokenizer: no exception in constructor when given empty string

An empty string is a valid code snippet and as such should not trigger
an exception. Doing so forces the caller to perform additional error
handling.

Fixes 0021124
Affected Issues
0021124
mod - core/classes/Tokenizer.class.php Diff File

MantisBT: master-1.3.x b1173d80

2016-06-16 15:56

dregad


Details Diff
Improve handling of empty values in adm_config_set.php

When processing an empty value, the code now evaluates the config type
as if user had selected 'default'; this ensures the proper case of
'empty' is used depending on the config being set (i.e. 0, '').

We also avoid calling the config parser in this case, because a simple
typecast is sufficient to process user input.

Fixes 0021124
Affected Issues
0021124
mod - adm_config_set.php Diff File

MantisBT: master-1.3.x 9ed0b9f5

2016-07-03 01:50

dregad


Details Diff
Fix config parsing issues and improve unit tests

PR https://github.com/mantisbt/mantisbt/pull/796

Fixes 0021124, 0021136
Affected Issues
0021124, 0021136
mod - adm_config_report.php Diff File
mod - adm_config_set.php Diff File
mod - core/classes/ConfigParser.class.php Diff File
mod - core/classes/Tokenizer.class.php Diff File
mod - tests/Mantis/ConfigParserTest.php Diff File