Validation of forms with value attribute set to an int about webpy HOT 12 CLOSED

I don't see a problem with this behavior. I think it is okay to assume that the Dropdown options must be strings.

If we implicitly convert value to string before comparison, we might get into more trouble. Explicit is better than implicit!

from webpy.

The problem is that ints are already being implicitly converted when the are rendered but the validation logic doesn't acknowledge this. The library allows forms to be defined with int values (ex: form.Radio in the OP) so it should be able to handle their validation, even though they have been converted by the HTML round trip.

Either the validation logic needs to be changed to account for the implicit conversion when the ints described in the form object are returned from HTML or the form object needs to restrict the ability to define ints as values in recognition of the inability to mantain type when rendered.

I don't have a strong opinion about which is better but it's clear that the current behavior is buggy. It should not be possible to define a form that can't keep track of its own state during during validation b/c of datatypes.

from webpy.

So you want Radio to crash when it gets an int value? That seems reasonable to me.

from webpy.

I don't have a clear preference for a particular solution (i.e. where you choose to check for stringyness and what action is taken). I do however think that form init needs to stop writing checks that form validation can't cash.

The patch I posted would help validation deliver on init's promise. I think presumptive conversion in the validation layer would work for most people most of the time. It would also create maddeningly subtle errors for someone. On the other hand type checking on form creation and crashing will create (unnecessary) frustrating WTF moments for new users while alerting everyone to validation edge cases. Seen in this light the selection amongst solutions is clearly a philosophical. Are you making Gentoo or Ubuntu?

Given these options I suppose my preference is, as you said, to crash early and clearly.

As an aside I wouldn't implement this by looking for ints in just Radio. The fact is that regardless of what goes in a form field a string is coming back. Any number of combos could demo this problem: floats in textareas, ints in radio buttons, or as was the case for me numpy floats in dropdowns. I suspect anything where obj != obj.str() will do it.

from webpy.

I agree that the inconsistency is troublesome and it's better to crash early (with a good error message if possible) rather than help hide bugs.

It looks to me like the bug might not be in Radio as much as it's in net.websafe(), which coerces its argument to unicode ; perhaps it should instead raise a ValueError? This would also get rid of the special-looking check for None at the beginning of the routine. It would become something like:

def websafe(val=u''):

    if isinstance(val, str):
        val = val.decode('utf-8')
    if not isinstance(val, unicode):
        raise ValueError("Cowardlyly refusing to autoconvert to unicode")

    return htmlquote(val)

from webpy.

       if isinstance(val, str):
           val = val.decode('utf-8')
       if not isinstance(val, unicode):
           raise ValueError("Cowardlyly refusing to autoconvert to unicode")

Yeah, that's probably right. It's really only intended to deal with
the charset issue.

from webpy.

On second thought (after reviewing the context in aa0a356), I've changed my mind on this change. Isn't websafe used by the template system? Coercing None and integers to quoted strings is pretty widely used and doesn't seem obviously wrong to me. I think crashing on receiving an integer at the Form level is a much better solution.

from webpy.

Should it error immediately when passed to Radio() or should it wait until it tries to get render()'d ?

from webpy.

Immediately, I'd think.

from webpy.

Another problem related to the original issue (not the websafe discussion) is when filling a form from a database query.

If the table has integers or bigint columns, the storage object returned by any DB query has ints or longs (e.g. 3L), and using form.fill(query_result) will not match the Dropdown and Radio options if they are strings. Conversely if you define the options as integers, they match with form.fill(query_result) but they do not work with form.validates(), as the web.input() only contains strings (cgi.FieldStorage values).

Any ideas on how to solve this without parsing each value manually?

from webpy.

I was looking at this again, and I see two options:

1. do typechecking in the Radio() constructor to check that the args are all strings or tuples/lists of strings. This would fix the issue by making it an error to call radio with non-string arguments to be rendered

2. explicitly document that we're going to call websafe()/unicode() on args, and do so, so that the validation logic (which currently checks 'self.value == value' ) will work. This could be done either once in the constructor, or at test-time in the aforementioned render() line ('self.value == net.websafe(value)').

Preferences?

from webpy.

On Tue, Oct 25, 2011 at 10:07:08PM -0700, Paul Jimenez wrote:

I was looking at this again, and I see two options:

1. do typechecking in the Radio() constructor to check that the
   args are all strings or tuples/lists of strings. This would fix
   the issue by making it an error to call radio with non-string
   arguments to be rendered

2. explicitly document that we're going to call
   websafe()/unicode() on args, and do so, so that the validation
   logic (which currently checks 'self.value == value' ) will work.
   This could be done either once in the constructor, or at test-time
   in the aforementioned render() line ('self.value ==
   net.websafe(value)').

Preferences?

Hi Paul,

what I ended up doing was converting the non-string values of the
source to string in the web.form.fill method.

Changed this:
def fill(self, source=None, *_kw):
return self.validates(source, _validate=False, *_kw)

To this:
def fill(self, source=None, *_kw):
d = source
cs = dict(map(lambda k: type(d[k]) in [int, long, float] and (k, str(d[k])) or (k, d[k]), list(d)))
return self.validates(cs, _validate=False, *_kw)

Thanks,

Ciro Mondueri

from webpy.