View Issue Details

IDProjectCategoryView StatusLast Update
0020478mantisbtcode cleanuppublic2016-06-12 00:42
ReporterbadfilesAssigned Todregad 
PrioritynormalSeveritytweakReproducibilityalways
Status closedResolutionfixed 
Product Version1.3.0-beta.1 
Target Version1.3.0-rc.2Fixed in Version1.3.0-rc.2 
Summary0020478: bug_update.php: do not use strict type checking unless necessary
Description

Original issue summary: Type mismatch in bug_update.php (260 git)

260 if( (int)$t_existing_bug->view_state !== $t_updated_bug->view_state ) {

TagsNo tags attached.

Relationships

related to 0019271 closedvboctor Reporter can't re-open or close issues even if they have access 
related to 0020075 closeddregad Error 1105 while changing bug status from bug_change_status_page.php 

Activities

atrol

atrol

2016-01-05 08:40

developer   ~0052249

Found this in description of 0019271

$t_existing_bug->view_state is of type string, needs to be casted to (int).

if( $t_existing_bug->view_state !== $t_updated_bug->view_state ) {

dregad

dregad

2016-01-05 09:48

developer   ~0052252

I'm not really sure why we're using strict type checking here.

That being said, I'm not able to reproduce the error, can you please provide steps to reproduce ?

badfiles

badfiles

2016-01-05 10:35

reporter   ~0052254

It should be obvious -- if you have no right to change view state and the view state is not changing, you'll get access denied error on the next line, but you should not.

dregad

dregad

2016-01-05 12:11

developer   ~0052257

Sorry if I'm being dense here, but what is obvious to you may not be for others.

As I said, I can't reproduce...

  • logged in as user with "updater" role
  • workflow threshold changed so that "updater" can't change view state
  • edit a bug, make some change

I get:
260 var_dump( $t_existing_bug->view_state,$t_updated_bug->view_state);

/home/dregad/dev/mantisbt/bug_update.php:260:string '10' (length=2)
/home/dregad/dev/mantisbt/bug_update.php:260:int 10

$t_existing_bug->view_state is cast to int, 10 === 10 so the test is not call.

The type mismatch can only occur if $t_updated_bug->view_state is not int, and I don't see how that can happen since it gets initialized by gpc_get_int(), which is defaulted to $t_existing_bug->view_state (since the user can't edit it, there is no POST variable for it).

badfiles

badfiles

2016-01-05 15:55

reporter   ~0052260

I absolutely agree with you, but I was changing the code and in my case $t_updated_bug->view_state happened to be string.
I just would not wish someone else had lost an hour again.

dregad

dregad

2016-01-06 04:43

developer   ~0052261

but I was changing the code

It would have been nice of you to mention this upfront, it would have saved me the effort to analyze and try to reproduce the issue, twice...

dregad

dregad

2016-01-06 04:50

developer   ~0052262

I changed category to code cleanup and reduced severity, as this is not an issue with MantisBT core (at least not one I can reproduce), and @badfiles admitted he was modifying the code.

Using loose comparison avoids having this kind of issues and to rely on typecasting to fix them.

Related Changesets

MantisBT: master 101b9316

2016-01-06 04:41:14

dregad

Details Diff
Do not use strict type comparison unless necessary

Fixes 0020478
mod - bug_update.php Diff File

MantisBT: master bddd139e

2016-01-06 05:12:08

dregad

Details Diff
Strict type check is needed for version compare

As pointed out by @atrol in https://github.com/mantisbt/mantisbt/commit/101b93166dfee4f31a1b31a6d666f56a5681ace0#commitcomment-15284448.

Issue 0020478
mod - bug_update.php Diff File

Issue History

Date Modified Username Field Change
2016-01-05 08:10 badfiles New Issue
2016-01-05 08:39 atrol Relationship added related to 0019271
2016-01-05 08:40 atrol Note Added: 0052249
2016-01-05 09:48 dregad Status new => feedback
2016-01-05 09:48 dregad Note Added: 0052252
2016-01-05 10:35 badfiles Note Added: 0052254
2016-01-05 10:35 badfiles Status feedback => new
2016-01-05 12:11 dregad Note Added: 0052257
2016-01-05 12:11 dregad Status new => feedback
2016-01-05 15:55 badfiles Note Added: 0052260
2016-01-05 15:55 badfiles Status feedback => new
2016-01-06 04:43 dregad Note Added: 0052261
2016-01-06 04:43 dregad Changeset attached => MantisBT master 101b9316
2016-01-06 04:43 dregad Assigned To => dregad
2016-01-06 04:43 dregad Status new => resolved
2016-01-06 04:43 dregad Resolution open => fixed
2016-01-06 04:43 dregad Fixed in Version => 1.3.0-rc.2
2016-01-06 04:50 dregad Severity major => tweak
2016-01-06 04:50 dregad Category other => code cleanup
2016-01-06 04:50 dregad Product Version 1.3.0-rc.1 => 1.3.0-beta.1
2016-01-06 04:50 dregad Target Version => 1.3.0-rc.2
2016-01-06 04:50 dregad Summary Type mismatch in bug_update.php (260 git) => bug_update.php: do not use strict type checking unless necessary
2016-01-06 04:50 dregad Description Updated View Revisions
2016-01-06 04:50 dregad Note Added: 0052262
2016-01-06 05:12 dregad Changeset attached => MantisBT master bddd139e
2016-02-26 08:08 dregad Relationship added related to 0020075
2016-06-12 00:42 vboctor Status resolved => closed