Ticket #1396 (closed defect: fixed)

Opened 3 years ago

Last modified 2 years ago

[PATCH] turbogears.util.call_on_stack sometimes returns True where it should return False

Reported by: tlg Assigned to: anonymous
Priority: high Milestone: 1.5
Component: TurboGears Version: 1.0.4b3
Severity: critical Keywords: validate
Cc:

Description (Last modified by Chris Arndt)

If call_on_stack finds 2 identical func_names but with different args, it returns True while it should return False.

I can trigger the bug uing the @validate decorator on a method which is called from a method already having a @validate decorator. The @validate checks if validate is already on the stack with the func as an argument and fails there.

Attachments

util.diff (1.1 kB) - added by Chris Arndt on 12/19/07 03:37:23.

Change History

07/02/07 18:18:07 changed by faide

Ooops file seems garbled I'll try to have Cory attach it directly here :)

07/02/07 18:19:33 changed by faide

Discussion is here : http://groups.google.com/group/turbogears/browse_thread/thread/5e9980cb571e79d5

07/02/07 18:20:13 changed by faide

  • summary changed from call_on_stack in util.py returns True while it shoud return False in some cases to [PATCH] call_on_stack in util.py returns True while it shoud return False in some cases.

12/19/07 03:37:23 changed by Chris Arndt

  • attachment util.diff added.

12/19/07 03:56:11 changed by Chris Arndt

  • summary changed from [PATCH] call_on_stack in util.py returns True while it shoud return False in some cases to [PATCH] turbogears.util.call_on_stack sometimes returns True where it should return False.
  • description changed.
  • version changed from trunk to 1.0.4b3.
  • milestone set to 1.1.

Removed extraneous chars from patch.

Issue seems to be still open with TG 1.0.4b.

As Cory points out in the discussion linked above, there might be more bugs lurking in that function related to function calls with only positional params or no args at all. But, I think the intended behaviour is to check for keyword args only (since call_on_stack takes only func_name and kw as args).

12/19/07 03:57:08 changed by Chris Arndt

  • description changed.

We should write a test case to check if this is a real issue.

04/02/08 22:30:16 changed by chrisz

  • status changed from new to closed.
  • resolution set to fixed.

Fixed in r4329 with unit test for both call_on_stack() itself and nested @validate.

04/14/08 15:51:22 changed by chrisz

  • keywords set to validate.
  • priority changed from normal to high.
  • status changed from closed to reopened.
  • resolution deleted.
  • severity changed from normal to critical.

The formerly broken call_on_stack() caused that actually only the first validator was ever executed per request. After fixing call_on_stack() in r4329, several validators can now be applied in a row for one request, with the last one masking the former errors. This breaks backward compatibility and often does not make even sense.

I suggest getting the old behavior back by making the call_on_stack() check without checking for the function name now.

04/15/08 02:37:26 changed by chrisz

  • status changed from reopened to closed.
  • resolution set to fixed.

The old behavior has now been reestablished in r4397. The "recursion guard" has been removed completely, instead we just check whether validation already happened by simply checking for the existence of the corresponding request attribute.

So to sum up, the error in util.call_on_stack() has been fixed, but the behavior that nested validates are skipped has been reestablished, in order to be backward compatible and because it generally makes more sense, you don't want an error handler do another validation. You are still free to call validators directly in a nested call.