View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0020478 | mantisbt | code cleanup | public | 2016-01-05 08:10 | 2016-06-12 00:42 |
Reporter | badfiles | Assigned To | dregad | ||
Priority | normal | Severity | tweak | Reproducibility | always |
Status | closed | Resolution | fixed | ||
Product Version | 1.3.0-beta.1 | ||||
Target Version | 1.3.0-rc.2 | Fixed in Version | 1.3.0-rc.2 | ||
Summary | 0020478: 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 ) { | ||||
Tags | No tags attached. | ||||
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 ) { |
|
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 ? |
|
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. |
|
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...
I get: /home/dregad/dev/mantisbt/bug_update.php:260:string '10' (length=2) $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). |
|
I absolutely agree with you, but I was changing the code and in my case $t_updated_bug->view_state happened to be string. |
|
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... |
|
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. |
|
MantisBT: master 101b9316 2016-01-05 23:41 Details Diff |
Do not use strict type comparison unless necessary Fixes 0020478 |
Affected Issues 0020478 |
|
mod - bug_update.php | Diff File | ||
MantisBT: master bddd139e 2016-01-06 00:12 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 |
Affected Issues 0020478 |
|
mod - bug_update.php | Diff File |