Some thoughts from my side. 1) Using a regexp to avoid sql injection is a nice idea. But I think most developers are not familiar with regexps and thus, the regexp mostly used will be ".*" opening the door for all kind of sql injection attacks. Why not use prepared (callable) statements ?.
Another problem I see is that the concept invites the user distributing sql fragments to different places making it hard to debug and/or alter the db design. I think it is better to have the sql stuff concentrated in few places in the code. 2) Passing the params in the environment. Again a nice idea, but hard to debug. Personally I feel better seeing the parameter handling in the code associated with the query object. The EnvFunction is some "magic", setting values at one place and using it at another place. EnvFunction.clearLocalValues is another danger if it is not called at the right time and my cause some nasty side effect. My 2 cents Quoting Andrea Aime <[email protected]>: > Hi, > some time ago I've sent this message to the mailing list, > and only Michael answered. > > I'm quite likely to get funding to implement it, so please, > if you have feedback, speak :-) > > ----------------------------------------------------------- > > Hi, > as you all probably know thanks to Michael work we recently got the > ability to peform on parameter substitution in SLD and filters > courtesy of the EnvFunction filter function. > > That is great and adds a great deal of flexibility to styling > without significant risks, as the params are either evaluated > in memory or properly parsed into a target type (actually, > just evaluated in memory now, but let me face that topic > in another mail). > > Now I'm looking into ways of doing something similar for the > VirtualTable support in JDBC data stores. > VirtualTable allows people to create a new feature type by > just providing a sql statement (plus a bit of metadata). > > Now, what I'm looking into doing is to add param replacement > in that sql statement, something like: > > select a.*, b.* > from a inner join b on a.id = b.fk > where a.size > ${param} > > or if the query is a call to a stored procedure: > > call myProcedure(10, 15, ${param}) > > The parameters would be explicitly declared in the VirtualTable, > and would be associated to a default value (so that the query is > always runnable) and to a validation regular expression. > The latter is crucial to avoid sql injection attacks, the regexp > should make sure no extra statements are embedded in the mix > (a good default regexp should probably disallow ; and '). > > So the VirtualTable would have a new array of Parameter, > where parameter has a name, a default value (String) and > a validation regular expression > > Now, an interesting question is, how do we pass down parameters? > > We are making queries, so a natural way would be to pass > them down as query parameters, something like: > > Query q = new Query(...); > q.setHints(new Hints(Hints.SQL_PARAMS, myStringStringMap) > > However there is another way that could be of interest: use > again EnvFunction. This would make param passing consistent > with SLD substitution: > > try { > EnvFunction.setLocalValues(myStringStringMap); > ... do query, render, whatever > } finally { > // if you want to clean up the thread local params map > EnvFunction.clearLocalValues(); > } > > and then internally the store would invoke: > > env = new EnvFunction().setParameters(singletonList("myParamName") > String value = env.evaluate(null, String.class); > > The advantages I see in this approach are: > - just one way to setup and retrieve "environmental" values in GeoTools > - does not require to fiddle with setting up Queries in MapLayer > just for the sake of passing down hints when rendering a parametrized > view layer > - the env function is somewhat more flexible in that it allows both > per thread and global enviroment values (see > EnvFunction.setGlobalValues(...) > > Opinions? > > ----------------------------------------------------------------------- > > Cheers > Andrea > > > -- > Andrea Aime > OpenGeo - http://opengeo.org > Expert service straight from the developers. > > ------------------------------------------------------------------------------ > ThinkGeek and WIRED's GeekDad team up for the Ultimate > GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the > lucky parental unit. See the prize list and enter to win: > http://p.sf.net/sfu/thinkgeek-promo > _______________________________________________ > Geotools-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/geotools-devel > ---------------------------------------------------------------- This message was sent using IMP, the Internet Messaging Program. ------------------------------------------------------------------------------ ThinkGeek and WIRED's GeekDad team up for the Ultimate GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the lucky parental unit. See the prize list and enter to win: http://p.sf.net/sfu/thinkgeek-promo _______________________________________________ Geotools-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/geotools-devel
