Modify

Opened 5 years ago

Closed 4 years ago

Last modified 23 months ago

#5825 closed defect (fixed)

"TypeError: iterable argument required" when PrivateTickets and TracHours both enabled

Reported by: Charlie Armor Owned by: rjollos
Priority: high Component: PrivateTicketsPlugin
Severity: major Keywords: "TypeError: iterable argument required" PrivateTickets TracHours
Cc: wandi.ms@…, jagnaldosousa@…, hasienda Trac Release: 0.11

Description

When I enable both plugins:

TracHoursPlugin-0.3.1-py2.4
TracPrivateTickets-2.0.2-py2.4.egg

The following error will be returned if I try to save changes to a ticket or create a new one:

Oops…
Trac detected an internal error:

TypeError: iterable argument required

Python Traceback
Most recent call last:

File "/usr/lib/python2.4/site-packages/Trac-0.11.4-py2.4.egg/trac/web/main.py", line 435, in _dispatch_request
  dispatcher.dispatch(req)
File "/usr/lib/python2.4/site-packages/Trac-0.11.4-py2.4.egg/trac/web/main.py", line 205, in dispatch
  resp = chosen_handler.process_request(req)
File "/usr/lib/python2.4/site-packages/Trac-0.11.4-py2.4.egg/trac/ticket/web_ui.py", line 182, in process_request
  return self._process_ticket_request(req)
File "/usr/lib/python2.4/site-packages/Trac-0.11.4-py2.4.egg/trac/ticket/web_ui.py", line 504, in _process_ticket_request
  valid = self._validate_ticket(req, ticket) and not problems
File "/usr/lib/python2.4/site-packages/Trac-0.11.4-py2.4.egg/trac/ticket/web_ui.py", line 970, in _validate_ticket
  for field, message in manipulator.validate_ticket(req, ticket):
File "/usr/lib/python2.4/site-packages/TracHoursPlugin-0.3.1-py2.4.egg/trachours/hours.py", line 234, in validate_ticket
  can_add_hours = PermissionSystem(self.env).check_permission('TICKET_ADD_HOURS', user)
File "/usr/lib/python2.4/site-packages/Trac-0.11.4-py2.4.egg/trac/perm.py", line 425, in check_permission
  perm)
File "/usr/lib/python2.4/site-packages/TracPrivateTickets-2.0.2-py2.4.egg/privatetickets/policy.py", line 32, in check_permission
  if username == 'anonymous' or \

System Information:

User Agent: Mozilla/5.0 (X11; U; Linux i686; en-GB; rv:1.9.0.14) Gecko/2009090216 Ubuntu/9.04 (jaunty) Firefox/3.0.14
Trac: 	0.11.4
Python: 	2.4.4 (#1, Apr 15 2008, 23:37:53) [GCC 4.1.2 20061115 (prerelease) (Debian 4.1.1-21)]
setuptools: 	0.6c9
SQLite: 	3.3.8
pysqlite: 	2.3.2
Genshi: 	0.6dev-r1064
mod_python: 	3.2.10
Subversion: 	1.4.2 (r22196)
jQuery:	1.2.6

Both plugins work ok if the other is not enabled, but conflict with the error given above if I try to use them together.

The error message mentioned TracPrivateTickets so I have raised the bug here but I'm not really qualified to know which plugin is causing the problem.

Attachments (0)

Change History (13)

comment:1 Changed 4 years ago by uray

following patch seems fix this bug:

Index: policy.py
===================================================================
--- policy.py	(revision 7874)
+++ policy.py	(working copy)
@@ -31,7 +31,7 @@
     def check_permission(self, action, username, resource, perm):
         if username == 'anonymous' or \
            action in self.ignore_permissions or \
-           'TRAC_ADMIN' in perm:
+           (perm is not None and 'TRAC_ADMIN' in perm):
             # In these three cases, checking makes no sense
             return None

comment:2 Changed 4 years ago by jordi

Thanks for this patch. I was getting a headache due to email not going out. Works perfectly on Debian squeeze!

comment:3 Changed 4 years ago by rjollos

  • Cc wandi.ms@… added

#6947 closed as a duplicate. Adding reporter of #6947 to CC list.

comment:4 Changed 4 years ago by rjollos

  • Cc jagnaldosousa@… added

#5231 closed as a duplicate. Adding reporter of #5231 to CC list.

comment:5 Changed 4 years ago by rjollos

#5257 closed as a duplicate. Reporter of #5257 is the same as #5231.

comment:6 Changed 4 years ago by rjollos

  • Owner changed from coderanger to rjollos
  • Status changed from new to assigned

It looks like the API is designed such that the perm object can be None, so we might want to patch PrivateTicketsPlugin as described above. However, I tend to think that the correct solution is to not use check_permission in the TracHoursPlugin, but rather has_permission, which results in a call to check_permission while correctly passing the PermissionCache object. So, I will make that change to the TracHoursPlugin, as it fixes this issue.

I'd love to hear feedback from Mr. Coderanger on this issue, just to confirm that this is the correct fix ;)

comment:7 Changed 4 years ago by rjollos

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

(In [9569]) Verify permission with req.perm.has_permission since it correctly passes the PermissionCache object to check_permission. This fixes an error that occurred when using the PrivateTicketsPlugin because the check_permission method in that plugin doesn't check that the expected PermissionCache object might be a NoneType. Fixes #5825, #5826.

comment:8 Changed 4 years ago by rjollos

(In [9570]) Fix permission checks elsewhere in code, as in [9569]. Refs #5825, #5826.

comment:9 Changed 4 years ago by rjollos

  • Cc hasienda added

comment:10 Changed 4 years ago by hasienda

(In [9813]) AnnouncerPlugin: Switch permission check from PermissionSystem to PermissionCache, refs #5825 and #8458.

This has been requested for playing nicer with plugins that implement
IPermissionPolicy but do not deal with check_permission method,
if user permission cache is missing as additional argument there.
Currently this applies i.e. to PrivateTicketsPlugin, so the same
workaround has been implemented for TracHoursPlugin by Ryan Ollos before
(see #5825 for details).

comment:11 Changed 4 years ago by rjollos

Additional discussion regarding the correctness of the patch applied here occurs in #8458.

comment:12 Changed 4 years ago by rjollos

(In [9817]) Refactored permission check fixes made in [9569] and [9570]. Thanks you osimons for the pointers! Refs #8458, #5825.

comment:13 Changed 23 months ago by hasienda

(In [12311]) AnnouncerPlugin: Further improve ticket permission checks, refs #5825, #8458 and #8577.

This is a late follow-up to changeset [9813] after more in-deep discussion on
permission checking for whole Trac realms and specific Trac resources in #8458.
With my original patch proposal from 04-Feb-2011 in mind I call this an aged
and really matured changeset, and that's not so bad after all.
Furthermore code from [12304] gets improved here as well.

Special thanks to Odd Simon Simonsen, Ryan J. Ollos and Christian Boos for
their help on my way towards better understanding Trac permissions.

Add Comment

Modify Ticket

Action
as closed .
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.