Opened 8 years ago

Closed 7 years ago

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

Reported by: Owned by: ergo@… Noah Kantrowitz normal MasterTicketsPlugin major martin@…, dale.miller@…, Jason Winnebeck, Marcus Lindblom, Mitar 0.11

### Description

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

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

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

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

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

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

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

### Changed 8 years ago by Dag Viggo Lokøen

Patch for the fix discussed in the ticket

### comment:8 Changed 8 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:10 Changed 8 years ago by Jason Winnebeck

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

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

### comment:14 Changed 8 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 8 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 8 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 7 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:19 Changed 7 years ago by Frau Boonekamp

Fixed in MasterTickets 3.0 (for Trac 0.12):

### comment:20 Changed 7 years ago by Frau Boonekamp

Resolution: → fixed new → closed

### comment:21 Changed 4 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 4 years ago by Ryan J Ollos

#10961 closed as a duplicate.

### Modify Ticket

Action
as closed The owner will remain Noah Kantrowitz.
The resolution will be deleted. Next status will be 'reopened'.