id,summary,reporter,owner,description,type,status,component,version,severity,resolution,keywords,cc,stage,has_patch,needs_docs,needs_tests,needs_better_patch,easy,ui_ux 8808,Form Wizard keeps state on subsequent requests and provides no way to cut short inside parse_params,Michael P. Jung,nobody,"Since this issue is subtle I'll describe my concrete usecase first and than describe the issue and possible solutions to this. '''Usecase:''' I have a form that contains a ChoiceField which depends on the currently logged in user and some request parameters. Since I need a special behaviour I subclassed the FormWizard and did overwrite the methods {{{parse_params}}}, {{{done}}}, {{{process_step}}}, {{{get_form}}} and {{{get_template}}}. Inside {{{parse_params}}} I set some attributes of self. For some reason I assumed that returning anything but None would cause the Wizard to abort. However the return value is ignored and the wizard code continues by calling {{{get_form}}} and {{{process_step}}}. Since {{{get_form}}} is the only way of putting the stuff which was set earlier in {{{parse_params}}} I have a hook that calls some helper methods on the form to set up the options of the ChoiceField depending on the stuff set by {{{parse_params}}} earlier. Since the only way of injecting custom data there is {{{self}}} one has no chance of telling wether the value comes from the current request or something earlier which might just have failed. Thus no AttributeError is raised as the attribute is set. The data just does not come from the current request. Even worse: In my case - since I was assuming that returning from {{{parse_params}}} with an {{{HttpResponse}}} would stop the wizard and return the HttpResponse immediately - caused invalid requests to be rendered with the state from a previous requests. A horrid leak of sensible data, as the wizard is part of a checkout process and the {{{ChoiceField}}} is used to pick an address. I'm just glad our QA noticed that glitch before putting this stuff live. It would not be hard to exploit and I don't like the idea of leaking sensible data. '''Solutions:''' 1. By overwriting the {{{__call__}}} method one can inject any attribute into self and return any HttpResponse object if something is wrong. I consider this rather ugly as the FormWizard explicitly states {{{parse_params}}} for getting stuff from the request object, but provides no way of breaking out of the wizard and returning a HttpResponse. I would like to see a way to return a HttpResponse object from within {{{parse_params}}} for shortcutting requests to a Wizard. (see attached patch) 2. The Wizard could be made immutable, but provide a custom 'state' attribute for transfering data between {{{parse_params}}}, {{{process_step}}}, etc. Anyhow this still would not enable the user of the FormWizard to shortcut a request with a HttpRequest. I would consider it a good idea not to be able to reuse a state from a previous request by accident. Another idea would be to discourage the users to store such state information as attribute, but rather enhance the form wizard to implement {{{__getitem___}}}, {{{__setitem__}}}, etc. so that it can be used to transfer state between the hook methods. Thus instead of doing ""{{{self.my_state = args[0]}}}"" one would do ""{{{self['my_state'] = args[0]}}}"". 3. In my case I solved this issue by wrapping the wizard inside a custom request method which creates a new instance of the Form wizard on every request. Maybe it would be a good idea to make it possible to have a class inside the url config rather than an instance. e.g. the url handler code could just check if the contained code is a class/type which needs to be instanciated first. This would make it possible to get a fresh instance of the object on every request. '''Bottom line:''' The FormWizard documentation needs to state that the Wizard object will be reused. Thus one must be aware that subsequent requests might get the same wizard instance. Thus any logic depending on a state set to {{{self}}} should be considered unsafe. I can imagine quite a lot of people using the FormWizard wrong without even knowing it. I posted this as a 1.0 issue as I think this one should be figured out or at least documented properly. It gave me a hard time and caused a critical software bug that was hard to spot. It surely was my mistake to assume that {{{parse_params}}} would be able to cut short the request, but the way the FormWizard uses {{{self}}} to transfer state from one method to another rounded it up to some rather unpleasing issue. I attached a simple patch which enhances {{{parse_params}}} so that it is capable of cutting short a request. The solutions (2.) is a bit more complex and needs some tweaking of the source and changing of the documentation so I did not provide a patch. Solution (3.) is just a quick fix that I don't consider very pretty, but use it for now.",New feature,closed,contrib.formtools,dev,Normal,fixed,"FormWizard, state, parse_params, shortcut, HttpResponse",mpjung@… rajesh.dhawan@… magma.chambers@…,Design decision needed,1,0,0,0,0,0