Roundup Tracker - Issues

Issue 2550712

classification
Title: exportcsvaction errors poorly when given invalid columns
Type: behavior Severity: major
Components: Web interface Versions: 1.4
process
Status: new Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: ber, schlatterbeck, willkg
Priority: Keywords:

Created on 2011-07-07 18:50 by willkg, last changed 2011-08-23 07:25 by ber.

Messages
msg4326 (view) Author: [hidden] (willkg) Date: 2011-07-07 18:50
I'm running Roundup 1.4.15 from the Debian testing packages on
http://pyblosxom.bluesock.org/ .

GoogleBot hits the following url on my Roundup instance several times a day:

/issue?@action=export_csv&@columns=kobtoscgsowfa&@group=actor,creator&@pagesize=50&@startwith=50

The problem with that is that kobtoscgsowfa is not a valid column.  I
have no clue where GoogleBot got this link, but it's totally wrong.

Anyhow, every time GoogleBot hits this url, Roundup kicks up a KeyError
because kobtoscgsowfa is not a valid propname in the database and I get
this email:


KeyError: 'kobtoscgsowfa'	Python 2.6.6
/usr/bin/python

A problem occurred while running a Python script. Here is the sequence
of function calls leading up to the error, with the most recent
(innermost) call first. The exception attributes are:

/usr/lib/pymodules/python2.6/roundup/backends/rdbms_common.py in
get(self=<hyperdb.Class "issue">, nodeid='18', propname='kobtoscgsowfa',
default=[], cache=1)
 1561 
 1562         # get the property (raises KeyErorr if invalid)
 1563         prop = self.properties[propname]
      prop = undefined, self = <hyperdb.Class "issue">, global
properties = undefined, propname = 'kobtoscgsowfa'
 1564 
 1565         # handle there being no value in the table for the property

/usr/lib/pymodules/python2.6/roundup/cgi/actions.py in
handle(self=<roundup.cgi.actions.ExportCSVAction instance>)
 1065                         'You do not have permission to view %(class)s'
 1066                     ) % {'class': request.classname})
 1067 
      row = [], global append = undefined, global str = undefined, klass
= <hyperdb.Class "issue">, global get = undefined, itemid = '18', name =
'kobtoscgsowfa'
 1068                 row.append(str(klass.get(itemid, name)))
 1069 

/usr/lib/pymodules/python2.6/roundup/cgi/actions.py in
execute(self=<roundup.cgi.actions.ExportCSVAction instance>)
   37         """Execute the action specified by this object."""
   38         self.permission()
   39         return self.handle()
      self = <roundup.cgi.actions.ExportCSVAction instance>, global
handle = undefined
   40 
   41     name = ''

/usr/lib/pymodules/python2.6/roundup/cgi/client.py in
handle_action(self=<roundup.cgi.client.Client instance>)
 1140                 return getattr(self, action_klass)()
 1141             else:
 1142                 return action_klass(self).execute()
      action_klass = <class roundup.cgi.actions.ExportCSVAction>, self =
<roundup.cgi.client.Client instance>, global execute = undefined
 1143 
 1144         except (ValueError, Reject), err:

/usr/lib/pymodules/python2.6/roundup/cgi/client.py in
inner_main(self=<roundup.cgi.client.Client instance>)
  450                 # possibly handle a form submit action (may change
self.classname
  451                 # and self.template, and may also append
error/ok_messages)
  452                 html = self.handle_action()
      html = undefined, self = <roundup.cgi.client.Client instance>,
global handle_action = undefined
  453 
  454                 if html:


There are two big problems here:

1. The handle method of ExportCSVAction in roundup/cgi/actions.py
doesn't check to see if the columns are valid before it starts doing
things based on them.  It doesn't catch the KeyError, so it fails on bad
input from the url which is uber-bad.

2. At the point where it's failing, it's already sent the HTTP response
headers with a HTTP 200.  So not only does it fail and send me an email,
but the response is an HTTP 200 so GoogleBot doesn't even know it's
doing something stupid.


I think the solution here is to verify that the columns are correct in
ExportCSVAction _before_ it starts sending HTTP headers and if they're
not correct, kick up an error that would get properly handled into an
HTTP 404 or 500 or whatever.

I checked Roundup 1.4.18 and I'm pretty sure the problem is there, too.

If someone can walk me through validating the columns, I can write a patch.

Meanwhile, I did something totally bogus and wrapped the row.append line
in a try/except:

    try:
        row.append(str(klass.get(itemid, name)))
    except KeyError:
        row.append("")
msg4328 (view) Author: [hidden] (ber) Date: 2011-07-08 13:02
Hi Will,
thanks for the report.
Indeed the next step, as I see it, is to make sure that the defect
is there in current svn and then ideally write a test that
fails to define the behabiour and then write the test.

Do you think you can write a test for this?

Best,
Bernhard
msg4392 (view) Author: [hidden] (willkg) Date: 2011-08-22 17:16
I spent a few hours trying to write a test last month and wasn't able to
get the test working, so I gave up.

The code hasn't changed between the version I was using and what was in
svn when I reported the bug, thus I have no reason to believe that it's
fixed.  Having said that, I didn't spend the time to set it all up with
the latest Roundup in svn to see if it's still a problem.

Between this issue and another issue I'm now having where Roundup is
emailing me error reports that are due to invalidated user-supplied
querystrings, I'm probably going to switch to a different system and am
thus unlikely to do any further work on this ticket.

Sorry about that.  Hope you figure it out.
msg4393 (view) Author: [hidden] (ber) Date: 2011-08-23 07:25
Hi Will,
thanks for trying to write a test!
Sorry to here that roundup is not working for you.

What would we need to do to make it work?
Fix this issue and the other one you are having?
Is there already a report for it?

Best Regards,
Bernhard
History
Date User Action Args
2011-08-23 07:25:30bersetmessages: + msg4393
2011-08-22 17:16:34willkgsetmessages: + msg4392
2011-07-15 14:26:28schlatterbecksetnosy: + schlatterbeck
2011-07-08 13:02:00bersetnosy: + ber
messages: + msg4328
2011-07-07 18:50:14willkgcreate