delete ticket change fails if it contains a modification of a custom field no longer defined

[Christian Boos]

How to Reproduce

While doing a POST operation on /ticket/255, Trac issued an internal error (OperationalError: no such column: test_five)

Request parameters:

{'__FORM_TOKEN': u'f146ad994605e1310e6110f1',
 'action': u'delete-comment',
 'cdate': u'1340278350970000',
 'cnum': u'14',
 'id': u'255'}

User agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.4 (KHTML, like Gecko) Chrome/22.0.1229.56 Safari/537.4

System Information

Trac	1.1.1dev-r0
Babel	0.9.6dev-r0
Docutils	0.9.1
Genshi	0.6.1dev-r0 (without speedups)
GIT	1.7.7.msysgit.0
Mercurial	2.3.1+1-4b25077ae7ee
Pygments	1.4
pysqlite	2.6.3
Python	2.7.2 (default, Jun 12 2011, 15:08:59) [MSC v.1500 32 bit (Intel)]
pytz	2011h
setuptools	0.6
SQLite	3.7.6.2
Subversion	1.7.6 (r1370777)
jQuery	1.7.2

Enabled Plugins

graphviz	0.13.0.6
MilestoneOperation	Rev
MyPage	0.13.0.0dev
new-ticket-in-milestone	r6326
TracMercurial	1.0.0.1dev
TracWikiExtras	0.13.2dev-r10635
vulnerability-tickets	r6669

Python Traceback

Traceback (most recent call last):
  File "c:\Trac\repos\trunk\trac\web\main.py", line 497, in _dispatch_request
    dispatcher.dispatch(req)
  File "c:\Trac\repos\trunk\trac\web\main.py", line 214, in dispatch
    resp = chosen_handler.process_request(req)
  File "c:\Trac\repos\trunk\tracopt\ticket\deleter.py", line 146, in process_request
    ticket.delete_change(cdate=cdate)
  File "c:\Trac\repos\trunk\trac\ticket\model.py", line 531, in delete_change
    % field, (oldvalue, self.id))
  File "c:\Trac\repos\trunk\trac\db\util.py", line 121, in execute
    cursor.execute(query, params)
  File "c:\Trac\repos\trunk\trac\db\util.py", line 65, in execute
    return self.cursor.execute(sql_escape_percent(sql), args)
  File "c:\Trac\repos\trunk\trac\db\sqlite_backend.py", line 78, in execute
    result = PyFormatCursor.execute(self, *args)
  File "c:\Trac\repos\trunk\trac\db\sqlite_backend.py", line 56, in execute
    args or [])
  File "c:\Trac\repos\trunk\trac\db\sqlite_backend.py", line 48, in _rollback_on_error
    return function(self, *args, **kwargs)
OperationalError: no such column: test_five

[Remy Blank]

I wonder if this could be related to the recent changes related to #1942. A field named test_five should not end up at line 531 of model.py, but rather at line 526. Is our detection of custom fields somehow messed up?

[Christian Boos]

No, same thing with 1.0-stable:

File "c:/Trac/repos/1.0-stable/trac/ticket/model.py", line 492, in delete_change
  % field, (oldvalue, self.id))

And looking at the code (sorry had no time this morning besides posting the bug ;-) ) it's not surprising, the test is if field in self.custom_fields:, but that field is no longer part of the custom fields…

[Remy Blank]

Ah, right. So basically, we should revert the logic and check if field not in self.std_fields. And check all other uses of self.custom_fields for similar issues.

[Christian Boos]

See test, fix and a few code clean-ups in repos:cboos.git:ticket10847-deleted-custom-fields.

All the other uses of custom_fields seemed fine.

[Remy Blank]

Looks good. Is it intentional that the ticket10847-deleted-custom-fields branch name is set for all changesets in the revision log of your repository, even changesets that are on 1.0-stable? (Or in other words, am I misunderstanding git branches?)

[Christian Boos]

git branches are like hg bookmarks (well, it's even the other way round, historically), so it's just an external pointer to a given commit (in git terms, this pointer is a "ref"). Now the fact that, in Trac, we propagate that information to the ancestors is quite arbitrary. It is convenient when you follow a topic branch like here, but only up to a point, i.e. it gets really annoying when you go deep in the past, as then you have basically the list of all branches for each changeset (see … well I thought I created a ticket for that, can't find it). There must be something smarter to do.

[Christian Boos]

Btw, we need that in 0.12-stable as well.

[Christian Boos]

Applied in r11355 on 0.12-stable, will merge in the next changesets.