On Sat, Jan 9, 2010 at 7:15 AM, Victor Subervi <span dir="ltr"><<a href="mailto:victorsubervi@gmail.com">victorsubervi@gmail.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im"><div class="gmail_quote">On Sat, Jan 9, 2010 at 9:35 AM, Steve Holden <span dir="ltr"><<a href="mailto:steve@holdenweb.com" target="_blank">steve@holdenweb.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left:1px solid rgb(204, 204, 204);margin:0pt 0pt 0pt 0.8ex;padding-left:1ex">
But we are now in the realm of theory as far as you are concerned, since<br>
you have already stated several times that you aren't interested in<br>
correcting your design until after you have got the current mess into<br>
production. So good luck with that.<br></blockquote><div><br>
</div></div></div>And if you were in my shoes, I'm sure you'd do the same thing. </blockquote><div><br></div><div>... Really, no, he wouldn't :) We're not all just hobbyists here who only do work for random open source projects. A lot of us are professionals who actually do have clients, actually do have deadlines, actually do have an understanding for production requirements. Getting something into production as soon as possible is certainly an important goal in commercial work. But it is not the only goal. Proper database design is very important because if you don't do it, you'll actually end up usually wasting *more* time and effort then if you just bite the bullet and fix it now.</div>
<div><br></div><div>Proper database design -- in particular in your case, not having multiple tables with various names that even need "%sThis" or "%sThat", and using parameterized queries to access those tables, is really important. It will save you time, it will save you effort, and it'll save you money-- because /not/ doing it is among other things, a major security risk. Getting code out fast and now is a noble goal in commercial projects, getting code out which is by design prone to attacks by hackers is negligence. </div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">Well, it *is* working now :)) And I am interested in cleaning this up. I should probably start with the matter of databases, since that's something I won't be able to easily change once clients actually start entering data. Please share with me any further concepts or questions to get me thinking how to redesign the databases.<br>
</blockquote><div><br></div><div>Your first goal needs to be in the layout of your tables, yes. Instead of having multiple "Packages" tables that vary, have a single Packages table, with an additional column that determines what kind of package it is (this information used to be in the name of the table). This may seem like everything's jumbled together, but it works. Put an index on that column if you need to: don't worry about if that one table might have thousands or tens of thousands of records. Databases are -designed- to handle that, and handle it well and faster. </div>
<div><br></div><div>Do the same for any other table which has various names-- CategoriesPackages seems to be another one. Segment the data by adding columns and indexes when needed, and not by breaking it out into different tables: if two tables have approximately the same columns, they belong in one table, with a column to simply differentiate between the two.</div>
<div><br></div><div>These steps are taking your database towards the ultimate goal of normalization-- a laudable goal, but I won't go into that in detail. It takes a book. </div><div><br></div><div>The next thing to do is to re-do your SQL queries. You should never do string interpolation, e.g:</div>
<div><br></div><div> SQL="SELECT x FROM y WHERE z = %s" % (arg,)</div><div> cur.execute(SQL)</div><div><br></div><div>If you have done the first step, your tables always have a set names so you never need to interpolate to do the queries-- and then at this point, under no circumstances ever, ever-- consider this a law that you will get fined for violation-- use string interpolation to generate your queries.</div>
<div><br></div><div>Instead, do:</div><div><br></div><div> cur.execute("SELECT x FROM y WHERE z = %s", (arg,))</div><div><br></div><div>That looks really similar, but is lightyears away. Its very unfortunate that some database drivers use 'format' as the paramstyle because it confuses issues, but the two effects are very different. In one, Python is just munging together and creating a new string. In the other, the database driver is doing a few things. Its analyzing the query, its storing it (often caching it, which speeds up further executions of that query), etc, etc, and then finally its seeing that you are passing arguments into it, and it is -safely- binding those arguments into the expression; this prevents SQL Injection attacks. You can use interpolation and prevent injection if you -meticulously- check -every- string that comes from the user, and -never- trust it (even if that string was written out to a hidden <input> and legitimate users have no way to alter, because illegitimate users will alter it anyways). Or you can use parameterized queries and just avoid it, while getting plenty of other benefits as well.</div>
<div><br></div><div><br></div><div>At work, we had a third-party package that we re-sold as part of our offering, and glancing over its source, I noticed something. It did, in essence to check login:</div><div><br></div>
<div>
cur.execute("SELECT user_id FROM usertable WHERE username = '%s' AND password = '%s' % (username, password))</div><div><br></div><div>I blinked, and emailed them to point out the problem. I suggested they log in as:</div>
<div><br></div><div> Username = dummyuser</div><div> Password = '; DROP usertable</div><div><br></div><div>You see, when using interpolation, the string that got sent to the database was:</div><div><br></div><div>
SELECT user_id FROM usertable WHERE username = 'dummyuser' AND password = ''; DROP usertable</div><div><br></div><div>And thus, from the login screen of this app-- I destroyed their environment.</div>
<div>
<br></div><div>Its sort of important that you not put code out into production which is susceptible to such things. Your clients will not at all appreciate it if and when some hacker discovers it and destroys their site, loosing them money. But even beyond that, there are many other benefits to just doing this right. You want to, believe me. Now, before you put anything into production. It might take a day or two longer, but its worth it.</div>
<div><br></div><div>Finally, go finish getting rid of the bare excepts, I saw one recently :) Just rip them all out, every one. All at once. Then fix any errors that come along with specific excepts or pre-tests if appropriate. ;)</div>
<div><br></div></div><div name="mailplane_signature">--S</div>