Modify

Opened 6 years ago

Closed 4 years ago

Last modified 20 months ago

#3920 closed defect (fixed)

[Patch] Error in code causing diff crash on tickets with changes in description only (web_ui.py)

Reported by: ergo@… Owned by: coderanger
Priority: normal Component: MasterTicketsPlugin
Severity: major Keywords:
Cc: martin@…, dale.miller@…, JasonWinnebeck, macke@…, mmitar@… Trac Release: 0.11

Description

source:masterticketsplugin/0.11/mastertickets/web_ui.py@4178:63-64#L62
NOW:

63	            for change in data.get('changes', []):
# next line get key error, if data.get use default:
64	                for field, field_data in change['fields'].iteritems():

CORRECTED:

63	            for change in data.get('changes', {'fields':[]}):

With this correction, diff of tickets with description changed only passes again.
rgds, Paul

Attachments (1)

fix_ticket_diff.patch (672 bytes) - added by dagvl 6 years ago.
Patch for the fix discussed in the ticket

Download all attachments as: .zip

Change History (23)

comment:1 Changed 6 years ago by anonymous

  • Cc martin@… added; anonymous removed

comment:2 Changed 6 years ago by martin@…

this correction doesn't help for me. Could someone clarify this patch? Does only line 63 have to be changed, or does line 64 also have to be removed? How about the rest of the block (which accesses field) - shouldn't it need modifications as well?

comment:3 Changed 6 years ago by ergo@…

Replace only line 63 with this
for change in data.get('changes', {'fields':[]}):
nothing else

comment:4 Changed 6 years ago by martin@…

The above fix doesn't help for me: I still get the KeyError:

  File "build/bdist.linux-i686/egg/mastertickets/web_ui.py", line 65, in post_process_request
    for field, field_data in change['fields'].iteritems():
KeyError: 'fields'

after debugging a bit, I found that the call to data.get('changes', []) was indeed returning an array with data and not returning the default value []. The problem is that the array changes contains several dictionaries with keys such as diff, but without a key fields. This is why the following line 64 chokes when trying to access changesfields?.

my fix follows:

            for change in data.get('changes', []):
                if not change.has_key('fields'):
                    continue
                for field, field_data in change['fields'].iteritems():

comment:5 Changed 6 years ago by anonymous

Thanks! I can confirm the latest patch fixes the problem for me, too.

comment:6 Changed 6 years ago by anonymous

This solution is not complete - if

data.get('changes', [])

return default value (= [] = list), next line will crash:

change.has_key('fields')

because list([]) has no attribute has_key
Correct solution is to combine both patches:

    for change in data.get('changes', {}):
        if not change.has_key('fields'):
            continue

comment:7 Changed 6 years ago by martin@…

Yes, that make sense - data.get('changes', {}) is what is should be.

Changed 6 years ago by dagvl

Patch for the fix discussed in the ticket

comment:8 Changed 6 years ago by dagvl

I've attached a diff which implements the fix discussed in this ticket. It should be applied with -p0 from the masterticketsplugin/0.11-directory.

comment:9 Changed 6 years ago by anonymous

  • Cc dale.miller@… added

comment:10 Changed 6 years ago by JasonWinnebeck

  • Cc JasonWinnebeck added

I'm using the latest version of the plugin and I think this is happening to me too. I haven't tried dagvl's patch yet. I assume because this ticket is not closed that it has not been merged into the upstream yet?

How to Reproduce

While doing a GET operation on /ticket/598, Trac issued an internal error.

Click on "diff" next to the description on any ticket that I've tried so far in my Trac.

Request parameters:

{'action': u'diff', 'id': u'598', 'version': u'2'}

User Agent was: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.5) Gecko/2008120122 Firefox/3.0.5 (.NET CLR 3.5.30729)

System Information

Trac 0.11.2
Python 2.5.2 (r252:60911, Jul 31 2008, 17:44:49)
[GCC 4.2.3 (Ubuntu 4.2.3-2ubuntu7)]
setuptools 0.6c8
SQLite 3.4.2
pysqlite 2.5.0
Genshi 0.5.1
mod_python 3.3.1
Pygments 0.11.1
Subversion 1.5.1 (r32289)
jQuery: 1.2.6

Python Traceback

Traceback (most recent call last):
  File "/usr/lib/python2.5/site-packages/Trac-0.11.2-py2.5.egg/trac/web/main.py", line 432, in _dispatch_request
    dispatcher.dispatch(req)
  File "/usr/lib/python2.5/site-packages/Trac-0.11.2-py2.5.egg/trac/web/main.py", line 216, in dispatch
    self._post_process_request(req, *resp)
  File "/usr/lib/python2.5/site-packages/Trac-0.11.2-py2.5.egg/trac/web/main.py", line 308, in _post_process_request
    resp = f.post_process_request(req, *resp)
  File "build/bdist.linux-i686/egg/mastertickets/web_ui.py", line 64, in post_process_request
    for field, field_data in change['fields'].iteritems():
KeyError: 'fields'

comment:11 Changed 6 years ago by JasonWinnebeck

The patch attached to this ticket works for me, by patching the latest source (as of r4179).

comment:12 Changed 6 years ago by dale.miller@…

When I installed the MasterTicketsPlugin 4 weeks ago I applied the patches listed for #3920 and #4167. After reviewing the open tickets I decided both of these looked like they should be included so I applied the patches prior to building the plugin.

#3920 - Error in code causing diff crash on tickets with changes in description only (web_ui.py). The patch.

#4167 - Concurrent edits of tickets can overwrite changes to blocking / blocked_by fields. The patch.

comment:13 Changed 6 years ago by macke@…

  • Cc macke@… added

I got bitten by this too. What's holding the patch from being folded into svn?

comment:14 Changed 6 years ago by JasonWinnebeck

I presume the only reason is because the maintainer (coderanger) has not been responsive to commit. Open-source is great because people can make patches, but the unfortunate part is if there's no one to integrate the patch, the only choice is to fork.

comment:15 Changed 6 years ago by macke@…

Yeah. Moving it to github or bitbucket (yay! :) would make it tons easier to fork and fix in the meantime, with an easy opton to merge changes back when/if the original author gets some time to spend.

CVCS:es aren't really good for handling open source projects that fall out of active maintenance.

comment:16 Changed 6 years ago by dagvl

I pushed my local git repos for this plugin into http://github.com/dagvl/masterticketsplugin/tree/master

It contains the two patches mentioned here. Additionally it has a modification which allows tickets with blockers in the state "resolved" to be closed. This pertains to our custom workflow at my workplace, but shouldn't interfere with anything.

The version numbering I've used is to suffix the original version number it with a -viz1, -viz2 etc as I've done modifications.

comment:17 Changed 5 years ago by rjollos

  • Summary changed from Error in code causing diff crash on tickets with changes in description only (web_ui.py) to [Patch] Error in code causing diff crash on tickets with changes in description only (web_ui.py)

comment:18 Changed 5 years ago by anonymous

  • Cc mmitar@… added

comment:20 Changed 4 years ago by boonekamp

  • Resolution set to fixed
  • Status changed from new to closed

comment:21 Changed 20 months ago by rjollos

(In [12910]) Fix #5503, #6766 and #3920 with the patch from #3920: http://trac-hacks.org/attachment/ticket/3920/fix_ticket_diff.patch

[Patch] Error in code causing diff crash on tickets with changes in description only (web_ui.py)

comment:22 Changed 20 months ago by rjollos

#10961 closed as a duplicate.

Add Comment

Modify Ticket

Action
as closed The owner will remain coderanger.
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.