Re: [SQLObject] Generic "find" & findOne method

[Carlos R. <car...@gm...>]

On Tue, 21 Dec 2004 23:33:13 +0300, Oleg Broytmann <ph...@ma...> wrote:
> On Tue, Dec 21, 2004 at 06:12:39PM -0200, Carlos Ribeiro wrote:
> > Let's assume that you have the result of a web form. You can simply
> > feed the form data to findOne as an argument. One possible example
> > (using a slightly different and improved syntax):
> >
> > person.findOne(**form.getData())
> 
>   Are you passing form data to SQL without validation?! Wow!! Isn't
> there a security risk?

Don't worry, validation is being done. That's the problem of
simplifications -- the example was intentionally simple just to
illustrate the use case. As I've told you, I'm trying to keep modules
as decoupled as possible. I'm also aggressively using TDD, and this
approach makes writing unit tests really easy. Data validity is
checked at *every* step. The form itself does some validation, using
client-side Javascript code. When the HTTP request is handled, the
form data is also passed to a validation object, using the same
intermediate representation for a slightly stricter validation (it
also avoids the problem that would occur if data was sent directly
without passing by the client-side validation; for example, if urllib
were used to bypass it). Also, code would only get that far after
passing several security checks (login, session management, etc.). In
the end, it's something along these lines (pseudo code, untested, not
optimized):

# builds a validator object using the data fetched from the form
validator = MyValidator(form.getData())
# warning and errors are lists of objects; if empty, everything is ok
if not validator.warnings and not validator.errors:
    person.findOne(**form.getData())

At this point, please bear in mind that I am just *locating* a record,
not filling it with data. So most of the security issues that you
point out are not really valid. Also, please note that the code on the
findOne() method iterates over the keys, and only reads values if the
key is a valid column name, which avoids the risk of a security
exploit.

>   Well, I am against such addition to the SQLObject. It is too
> specific. You can easily do it in your own project by creating a parent
> class like this:
> 
> class Finding(SQLObject):
>   def findOne(self, dict):
>      ...
> 
> and use it as the base for all your tables:
> 
> class Person(Finding):
>   ...

While I respect you opinion, I still think that the proposed function
is useful enough. However, I think that I may not have presented it
clearly enough. I'll try to write a less verbose (and cleaner)
explanation and present it again.

-- 
Carlos Ribeiro
Consultoria em Projetos
blog: http://rascunhosrotos.blogspot.com
blog: http://pythonnotes.blogspot.com
mail: car...@gm...
mail: car...@ya...