Modify

Opened 15 years ago

Closed 14 years ago

Last modified 11 years 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: Noah Kantrowitz
Priority: normal Component: MasterTicketsPlugin
Severity: major Keywords:
Cc: martin@…, dale.miller@…, Jason Winnebeck, Marcus Lindblom, Mitar 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 Dag Viggo Lokøen 15 years ago.
Patch for the fix discussed in the ticket

Download all attachments as: .zip

Change History (23)

comment:1 Changed 15 years ago by anonymous

Cc: martin@… added; anonymous removed

comment:2 Changed 15 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 15 years ago by ergo@…

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

comment:4 Changed 15 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 15 years ago by anonymous

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

comment:6 Changed 15 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 15 years ago by martin@…

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

Changed 15 years ago by Dag Viggo Lokøen

Attachment: fix_ticket_diff.patch added

Patch for the fix discussed in the ticket

comment:8 Changed 15 years ago by Dag Viggo Lokøen

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 15 years ago by anonymous

Cc: dale.miller@… added

comment:10 Changed 15 years ago by Jason Winnebeck

Cc: Jason Winnebeck 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 15 years ago by Jason Winnebeck

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

comment:12 Changed 15 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 15 years ago by Marcus Lindblom

Cc: Marcus Lindblom added

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

comment:14 Changed 15 years ago by Jason Winnebeck

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 15 years ago by Marcus Lindblom

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 15 years ago by Dag Viggo Lokøen

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 14 years ago by Ryan J Ollos

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

comment:18 Changed 14 years ago by anonymous

Cc: Mitar added

comment:20 Changed 14 years ago by Frau Boonekamp

Resolution: fixed
Status: newclosed

comment:21 Changed 11 years ago by Ryan J Ollos

(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 11 years ago by Ryan J Ollos

#10961 closed as a duplicate.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Noah Kantrowitz.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


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

 
Note: See TracTickets for help on using tickets.