mantisbt-plugins / inline-history

Show issue history entries interleaved with bug notes
4 stars 8 forks source link

Infinite loop when there is only one history entry #2

Closed bruinsg closed 10 years ago

bruinsg commented 11 years ago

Originally reported here http://www.mantisbt.org/bugs/view.php?id=16066

When the plugin displays the history in ascending order and the last bugnote is more recent than all history items the next_entry function, it will go into an endless loop crashing the server.

PHP Fatal error: Allowed memory size of 134217728 bytes exhausted (tried to allocate 71 bytes) in /var/www/mantisbt/core/lang_api.php on line 212

To reproduce:

  1. Issue with some issue history.
  2. Add a note
  3. Reload page and the page will crash
dregad commented 11 years ago

Thanks for the pull request.

I have not had time to test this, but since array_shift/pop already return the element we need and NULL if array is empty, wouldn't it be more efficient to get rid of this whole 'count' business and do something like this instead ?

while( NULL !== ( $t_entry = array_shift( $this->history ) ) ) {
    if( $t_entry['date'] < $t_note_time ) {
        $t_entries[] = $t_entry;
    }
}
bruinsg commented 11 years ago

Thanks for looking at this:


    while( NULL !== ( $t_entry = array_shift( $this->history ) ) ) {
        if( $t_entry['date'] < $t_note_time ) {
            $t_entries[] = $t_entry;
        } else {
            array_unshift($this->history, $t_entry);
            break;
        }
    }

Would this be better?


while( $this->history[0] !== NULL && $this->history[0]['date'] < $t_note_time ) {
    $t_entries[] = array_shift( $this->history );
}
bruinsg commented 11 years ago

Hi,

I have simplified the sorting and also fixed the descending order as that was also broken.

Please have a look and see if there is anything else you would like me to change.

dregad commented 10 years ago

I agree that the approach of using a function is cleaner and easier for maintenance.

Thanks for your contribution.