Menu
▾
▴
Re: [namedgraphs] SQL escaping bug found (and fixed)
|
From: Jennifer C. <jen...@at...> - 2010-09-13 16:36:06
|
Asheesh - thank you for your work on this. And thank you in advance to Creative Commons for the willingness to share it back with the NG4J community. This will be a wonderful contribution to the project! When the patch series is prepared, please create a new entry in the patch tracking system for NG4J at http://sourceforge.net/tracker/?group_id=118754, and upload them there. It sounds like you have already tested this for both Apache Derby and MySQL? In that case after inspecting the code we should do a sanity check for each of the remaining supported databases - HSQLDB, Oracle, and PostgreSQL. Jennifer -----Original Message----- From: Asheesh Laroia [mailto:ng...@as...] Sent: Monday, September 13, 2010 11:57 AM To: ng4...@li... Subject: [namedgraphs] SQL escaping bug found (and fixed) Dear NG4J folks, I'm emaiilng you with a problem we ran into, a root cause we found, a strategy we took to fix it, and information on how we're sharing the patch with you. The problem (and background) ---------------------------- I'm using NG4J along with jenabean <http://code.google.com/p/jenabean/>, and we noticed that we were sometimes getting java.sql.SQLSyntaxErrorExceptions thrown within the NG4J code. You can see that in our Hudson (automat test running, AKA continuous integration) service at http://code.creativecommons.org/hudson/job/discovered-next/31/testReport/j unit/org.creativecommons.learn/TestCuratorFeed/testURLTitleProvenance/ This is what it looks like. com.hp.hpl.jena.shared.JenaException: java.sql.SQLSyntaxErrorException: Syntax error: Encountered "s" at line 1, column 308. at de.fuberlin.wiwiss.ng4j.db.QuadDB.executeQuery(QuadDB.java:354) at de.fuberlin.wiwiss.ng4j.db.QuadDB.find(QuadDB.java:108) at de.fuberlin.wiwiss.ng4j.db.QuadDB.insert(QuadDB.java:65) at de.fuberlin.wiwiss.ng4j.db.NamedGraphDB.performAdd(NamedGraphDB.java:92) at com.hp.hpl.jena.graph.impl.GraphBase.add(GraphBase.java:199) at com.hp.hpl.jena.rdf.model.impl.ModelCom.add(ModelCom.java:923) at com.hp.hpl.jena.rdf.model.impl.ResourceImpl.addProperty(ResourceImpl.java: 238) at thewebsemantic.Bean2RDF.saveOrUpdate(Bean2RDF.java:188) at thewebsemantic.Bean2RDF.write(Bean2RDF.java:176) at thewebsemantic.Bean2RDF._write(Bean2RDF.java:137) at thewebsemantic.Bean2RDF.write(Bean2RDF.java:130) at thewebsemantic.Bean2RDF.saveDeep(Bean2RDF.java:116) ("thewebsemantic" is the namespace for jenabean. jenabean is a sort of ORM for RDF data.) We use a few different backends for NG4J -- sometimes MySQL, and sometimes Apache Derby. We noticed a few peculiar things about the exception: * It only happened for a particular triple we were trying to set. * It only happened when we were using Derby -- MySQL did fine. We realized that the problematic triple had an apostrophe in it. This fact plus the SQL syntax error made us suspicious of bad escaping.... The root cause -------------- The escape() methods in QuadDB were generating SQL statements that looked fine, like: INSERT ... 'Here\'s the value' ... The Apache Derby backend considered that invalid syntax, whereas MySQL was happy to accept it. Frankly, it looks like valid syntax to me -- I don't know what Derby's deal is. (-: Our strategy ------------ These kinds of problems crop up frequently when you use strings concatenation to generate SQL queries. So we wrote some patches that, where it was feasible, converted QuadDb away from using an escape() function. Instead, it uses PreparedStatements. PreparedStatements let you pass a query *template* to the database engine; if I understand things right, they create a parsed syntax tree. You get to use methods like setString() to fill in the blanks. (More info about PreparedStatements cane be found at http://download.oracle.com/javase/tutorial/jdbc/basics/prepared.html and other places.) By using methods like setString(), the NG4J code doesn't have to do any escaping. Which means it can't get it wrong. (-: Our patches ----------- We already wrote patches to address this, and tested them, and they fix our issue. We hope you'll merge them! I did the work, but the copyright is held by Creative Commons because they supported it. Nathan Yergler, the CTO of Creative Commons, will reply saying that he permits the work to be merged into NG4J and licensed under the same terms as the the rest of NG4J. I'm preparing a patch series shortly, and hopefully within the day, I'm going to send it to the list. That's my thoughts -- if you need the patches reworked for some reason, let me know. A good way to find me is to join the #discovered IRC channel on irc.freenode.net and ping my nick, which is paulproteus. Or to just reply! -- Asheesh. -------------------------------------------------------------------------- ---- Start uncovering the many advantages of virtual appliances and start using them to simplify application deployment and accelerate your shift to cloud computing http://p.sf.net/sfu/novell-sfdev2dev _______________________________________________ ng4j-namedgraphs mailing list ng4...@li... https://lists.sourceforge.net/lists/listinfo/ng4j-namedgraphs |
