View Issue Details

IDProjectCategoryView StatusLast Update
0026765mantisbtbugtrackerpublic2020-03-15 15:23
Reporteruser37337 Assigned Todregad  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version2.23.0 
Target Version2.24.0Fixed in Version2.24.0 
Summary0026765: Inheritance of sub project not read correctly from database
Description

Hi,
I've found an bug that produces wrong results in reading the inheritance from the database. In some cases the inheritance was set to false even the database entry was set to true,
I've found the problem in project_hierarchy_cache() in project_hierarchy_api.php line 192:

if( $t_row['inherit_parent'] && !isset( $g_cache_project_inheritance[(int)$t_row['id']][(int)$t_row['parent_id']] ) ) {

The problem here was the isset function which returned true even when the project id of the parent project was not in the array because the project id was small (in my case 1) and the index 1 was already filled with another id.
Simply replacing of isset() with in_array() fixes the issue for me.

Maybe there are more such problems in this function because there are more isset uses for integers but I'm not a PHP guru so someone else should look for it.

TagsNo tags attached.

Activities

dregad

dregad

2020-03-09 05:23

developer   ~0063736

I'm not sure I fully understand the problem.

Could you please explain what you do, what you expect to get and the results actually get, and provide detailed, step-by-step instructions to reproduce the issue.

user37337

user37337

2020-03-09 05:48

reporter   ~0063737

I'm not sure how to reproduce the problem with a new installation. The problem seems to occur when there are more than one parent project for some sub-projects.
In my case there are parent projects with the IDs: 1, 5 and 13. In the sub-project with ID 2 which inherits from these 3 parents, I got this problem. The Database returned the data ordered by name so the project IDs were ordered as follows: 13, 5, 1.

The while loop added a new row to $g_cache_project_inheritance with key value 2.
Then the values 0 (ALL_PROJECTS), 13 and 5 are added correctly to the sub-array.
But when the value 1 should be added to the sub-array the isset() function returned true because the sub-array already contained an element with the index 1 (containing value 13) but no element with the value 1.
So in_array() should be the correct function in this case.

dregad

dregad

2020-03-09 08:20

developer   ~0063738

Thanks for the feedback, now that I understand that you're using multiple inheritance (i.e. a single subproject with several parents), I am able to reproduce the problem.

The root cause is that when are not consistent when accessing and updating the cache array: sometimes it is treated as a list (i.e. $cache[$t_project_id][] = $t_parent_id, which creates sequential numeric keys), and others as a map (isset( $cache[$t_project_id][$t_parent_id'] )).

The consequence is that we're effectively comparing apples and oranges (i.e. project id 1 vs an unrelated array index key 1, corresponding to another project).

Using in_array() would indeed fix the problem, but considering that we are already using the cache array as a map for the ALL_PROJECTS case, I think we should be consistent and do the same for other projects. This has an added benefit of being more efficient, as dereferncing the array $cache[$key] is faster than searching it `in_array($key, $cache) and we don't even need to bother with an isset() call.

dregad

dregad

2020-03-09 08:57

developer   ~0063739

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

user37337

user37337

2020-03-09 10:00

reporter   ~0063740

Thank you,
my simple fix produces an error when using the SOAP API. Your fix works fine!

Related Changesets

MantisBT: master e8738fc0

2020-03-09 08:26:51

dregad

Details Diff
Build project Inheritance cache as a map

project_hierarchy_cache() was inconsistent when processing the
$g_cache_project_inheritance array:

- used as a list when inserting values, which creates sequential numeric
keys (i.e. `$cache[$t_project_id][] = $t_parent_id`)
- treated as a map when checking for value's existence
(i.e. `isset( $cache[$t_project_id][$t_parent_id'] )`

The consequence is that we're effectively comparing apples and oranges
(e.g. project id 1 vs an unrelated array index key 1).

Consistently treating the array as a map (i.e. using the parent id as
key) allows for simpler and more efficient code, as we can just assign
the value without needing to call isset() or in_array() first.

Fixes 0026765
Affected Issues
0026765
mod - core/project_hierarchy_api.php Diff File

Issue History

Date Modified Username Field Change
2020-03-09 04:47 user37337 New Issue
2020-03-09 05:23 dregad Status new => feedback
2020-03-09 05:23 dregad Note Added: 0063736
2020-03-09 05:48 user37337 Note Added: 0063737
2020-03-09 05:48 user37337 Status feedback => new
2020-03-09 08:20 dregad Status new => confirmed
2020-03-09 08:20 dregad Note Added: 0063738
2020-03-09 08:57 dregad Assigned To => dregad
2020-03-09 08:57 dregad Status confirmed => assigned
2020-03-09 08:57 dregad Note Added: 0063739
2020-03-09 08:58 dregad Target Version => 2.24.0
2020-03-09 10:00 user37337 Note Added: 0063740
2020-03-15 13:02 dregad Changeset attached => MantisBT master e8738fc0
2020-03-15 13:02 dregad Status assigned => resolved
2020-03-15 13:02 dregad Resolution open => fixed
2020-03-15 13:02 dregad Fixed in Version => 2.24.0
2020-03-15 15:23 vboctor Status resolved => closed