[AppDB] - make variables more consistent, fix indenting

[Chris Morgan]

On Tuesday 20 June 2006 5:03 pm, Tony Lambregts wrote:
> Chris Morgan wrote:
> > On Tuesday 20 June 2006 4:12 pm, Tony Lambregts wrote:
> >>Chris Morgan wrote:
> >>>Make the code more consistent by using the appdb coding standards.
> >>>
> >>>Change $result = query_appdb(...) to $hResult = ... etc.
> >>>
> >>>Also fix some odd indenting due to spaces vs. tabs.
> >>>
> >>>There are still a ton of variables that are integers or strings that
> >>>should be prefixed with 'i' or 's' if anyone is interested in cleaning
> >>>some of that up.
> >>>
> >>>Chris
> >>
> >>I am not opposed to doing this but I think that the patch is way too big.
> >>At the momment we still have issues with the last "big patch" that are
> >> not cleared up [1]. On the whole unless we cannot avoid it I really do
> >> not think that we should be making such large changes in one go like
> >> this. Please break up the patch into smaller patches. That way it is
> >> easier to find and fix any issues that arise.
> >>
> >>[1] New versions end up as orphans with current setup.
> >>
> >>--
> >>
> >>Tony Lambregts
> >
> > Would per-directory patches be easier to read?  Like one for /include,
> > one for /admin, one for / ?
> >
> > This patch is a precursor to a patch that will close up a bunch of
> > security holes that could result in the db being destroyed, then on to
> > finding a more appropriate solution to the makeSafe() patch.
> >
> > Chris
>
> No That is not what I would prefer I would prefer a single patch for each
> file that is changed (ie one for addcomment.php and one for
> include/vendor.php. If the changes to one file need changes in other in
> order for the appdb to continue to function then they should be together in
> one patch.
>

There is no way I'm going to send a patch in per-file.  Its a huge pain in the 
ass and doesn't change how the monolithic patch is reviewed since you'll end 
up reviewing the same code anyway.  Breaking the patch up given that its a 
atomic noop change is also not necessary.

> Now you may think that is overboard but I have seen these "big patches"
> break the AppDB too many times.
>
> As far as "security holes that could result in the db being destroyed" we
> do have backup. I dont want you to think I do not care about security. That
> is simply not true. I am however very much against breaking the AppDB for
> regular use. The fact is that so far we have lost more data through bad
> patches then through security holes.
>

I agree.  I'll fix the existing issues before applying any more patches like 
this one.

> We have preaty good security in place already, We check that id's are
> numeric and escape date before running it. Right now if you ask me we would
> be better off making a audit of our querys than what you are advocating.
>

Our security is awful.  You should really re-read the email sent to the appdb 
list by Mortiz Naumann and see how bad it is.

Chris