Re: Bug #566650: Please unblock dtc 0.32.2-1
Hi.
Please, this requires *no reply* AT ALL, we don't need to waste more
time, the decision has been made already.
I'm not arguing or what so ever, but I think I still need to comment on
the comments, just for the record, to tell what happened, and to tell
what's behind the code being reviewed.
On 11/08/2010 11:48 PM, Neil McGovern wrote:
> On Mon, Nov 08, 2010 at 10:47:54PM +0800, Thomas Goirand wrote:
>> As it stands, it's unreasonable to even try working on the 0.30.x branch
>> for Squeeze, given the short amount of time remaining. I feel very sad
>> about it, but as there's no way to convince the RT that the 0.32.x
>> branch is in a very good shape for Squeeze (my users can tell it is...),
>> I have to ask for the removal of src:dtc from testing. Please proceed if
>> there's no way to change your mind about unblocking.
>
> I've had a look at the diff
> ( 425 files changed, 102770 insertions(+), 49242 deletions(-) ) !
There we go again... I think it has been discussed already. Never mind.
Yes, as this is the result of one year of development, that ended on the
10th of September when it was released. I spent all the summer
cleaning-up so that I could finally do an upload to SID on the 11th of
last September, just after I was made DD last June. I unfortunately
first worked on all other packages (and some that DTC was depending on),
which were pending since I didn't find sponsors. What a bad timing, and
what a bad move on my side...
> and there's things in just the first few files that make this unsuitable
> for this stage of the freeze, and some worrying changes in general. Just
> picking through the file at complete random:
>
> - return "Admin not found!";
> + return "Admin $adm_login not found line ".__LINE__." file ".__FILE__;
>
> Would this lead to some information disclosure?
Hard to tell as you don't tell where you found it, but generally no.
This is an open source project, anyone could dig and find out: this just
helps debugging when needed, but I don't see how this could disclose
anything important. What security issue would it be to have a source
code line number and file name? If someone differs, please explain your
view.
> dtc-0.30.20/admin/inc/dtc_config.php - huge set of changes, including a
> load of new features (Custom registration fields, a new radius
> implementation etc)
Yes, as (again) I worked on version 0.32.x for Squeeze, and left behind
0.30.x. Version 0.32.x is NOT just a fix for 0.30.x. If you are
considering the diff, then you will find lots of changes for sure. Never
the less, 0.30.x has *never* been aimed at Squeeze. This is totally
irrelevant here to try to see differences between the 2 versions, and
that's the wrong thing being done. I've stressed it many times, but
never mind.
> dtc-0.32.5/admin/dtc_db.php and
The engine for maintaining the db has been rewritten (see
dtc/admin/restor_db.php), and dtc_db.php had lots of minor changes
because of this. But the upgrade is still very smooth, and nobody
complained about it. In fact, it's a WAY better than many hacks in
0.30.x, because this version of the code also changes variables TYPES if
needed (producing some ALTER TABLE). But globally, the SQL structure is
the same, and upgrades are smooth, since day one of DTC, nobody every
complained about SQL structure upgrades.
> dtc-0.32.5/admin/dtc_import_all_dbs:
This one isn't at all used for the setup. It's for migrating one DTC
instance from a given server to another (and more specifically, this
file import what has been scp to the new server). It's called by
dtc_migrate. This helps moving a full server within minutes (I already
managed to do this kind of operation in less than 15 minutes thanks to
this script). I don't even think this file was there in version 0.30.x.
> looks like a load of changes to the database.
>
> A lot of:
> -<form action=\"".$_SERVER["PHP_SELF"]."\" method=\"post\">
> +<form action=\"?\" method=\"post\">
> Makes me wonder if this been through a search and replace tool.
Not at all. Please don't assume. :)
Yes, I've done a grep to find them all, but I have carefully replaced
them were it was armless (and in other places, like in the payment
gateway forwarders did some escape). This, plus many other *careful*
cleanups pushed me to ask for removal, as I don't want to see the older
versions in Squeeze. This also, has been used in production by many
users, nobody ever reported anything about it.
> As this is such a small selection from what is a huge diff, I'm afraid
> I've gone with the suggestion and added the DTC removal hint.
>
> Neil
Thanks.
Thomas
P.S: Again, let's not continue this thread please.
Reply to: