View Issue Details

IDProjectCategoryView StatusLast Update
0007907mantisbtinstallationpublic2007-12-21 23:16
Reportergiallu Assigned Tovboctor  
PrioritynormalSeveritytrivialReproducibilityN/A
Status closedResolutionfixed 
Product Version1.1.0a3 
Target Version1.1.0Fixed in Version1.1.0a4 
Summary0007907: Allow using system adodb
Description

Linux packages should use (when available) system wide libraries; this is primarily done to avoid leaving security holes in packages embedding their own version of a 3rd party library/add-on.

To allow this, line 19 in core/database_api.php has to be modified from:

require_once( $t_core_dir . 'adodb/adodb.inc.php' );

to

require_once( 'adodb/adodb.inc.php' );

Please note this will not affect those using the official releases, since "." is always in the php include_path

TagsNo tags attached.

Relationships

parent of 0008020 closedgiallu Port 7907: Allow using system adodb 

Activities

vboctor

vboctor

2007-04-19 21:33

manager   ~0014373

The issue with that is that we will be using a version of ADODB that we haven't tested Mantis with. However, I agree on the concept.

The other issue is that there is some minor changes in the ADODB version that we ship. Following are the changes that I can remember:

  1. I believe that paulr (grangeway) has introduced a new field type which is XS. I don't remember the exact reason, but I would like to get rid of this if we can. This will also simplify our upgrade to newer version of ADODB. Any changes that we end up keeping should be documented in a Wiki page so that we can easily re-apply them if necessary.

  2. In some cases we added a fix for notices or minor bugs. I don't know if we have any of these now.

So, I would suggest comparing the ADODB version that we ship now, with the original distribution files corresponding to this version. Then we can document the changes and try to get back in line with the standard distribution files.

giallu, would you be able to do this analysis? In the mean time, I will ask grangeway to provide us with some input on this.

vboctor

vboctor

2007-04-19 21:36

manager   ~0014374

Reminder sent to: grangeway

What are your thoughts on this issue?

giallu

giallu

2007-04-20 03:39

reporter   ~0014377

I assumed the version would be the exactly the same as upstream. I am adding this to my todo list and report here any news.

In reply to your points:

I am not sure how many users are there, but Debian is already using their own adodb and I am going to do the same in the next Fedora release, so there will be some "volunteers" for testing...

On the other side, you will stop worrying to ugrade the library in case of security fixes (and the final rpm is smaller)

1&2. If we need some modifications or fix bugs, it's fine to add patches but they should also be submitted upstream for inclusion; this way everybody will benefit (if the patch is sane and got accepted), or discover we are not using adodb properly ;) )

vboctor

vboctor

2007-04-20 03:55

manager   ~0014378

I would like to keep ADODB in the standard Mantis distribution but allowing Linux distributions to just delete these folders (assuming ADODB is somewhere in the include path) and Mantis continues to work. I would like to keep the process simple for users that only use Mantis or Mantis is the only tool they use which uses ADODB.

giallu

giallu

2007-04-21 18:22

reporter   ~0014382

I had a look at adodb: it seems we are using version 4.80 and, apart from removing their "tests" directory, the only significant diff seems to be the following:

diff -u adodb/adodb-datadict.inc.php /var/www//html/mantisbt/core/adodb/adodb-datadict.inc.php
--- adodb/adodb-datadict.inc.php 2006-03-10 09:58:55.000000000 +0100
+++ /var/www//html/mantisbt/core/adodb/adodb-datadict.inc.php 2006-04-22 13:35:37.000000000 +0200
@@ -549,12 +549,14 @@

        $ftype = $this->_GetSize($ftype, $ty, $fsize, $fprec);
  • if ($ty == 'X' || $ty == 'X2' || $ty == 'B') $fnotnull = false; // some blob types do not accept nulls
  • // plr - allow this for now as we use this until we identify what DB's dont allow this
  • //if ($ty == 'X' || $ty == 'X2' || $ty == 'B') $fnotnull = false; // some blob types do not accept nulls

        if ($fprimary) $pkey[] = $fname;
    
        // some databases do not allow blobs to have defaults
  • if ($ty == 'X') $fdefault = false;
  • // plr - allow this for now as we use this until we identify what DB's dont allow this
  • //if ($ty == 'X') $fdefault = false;

        //--------------------
        // CONSTRUCT FIELD SQL

I had a look at the original commit log and at the tracker, but I was not able to determine why those was needed in the first place.

Are we storing null values into BLOBs ?
Are we creating BLOBSs with default values ?

grangeway

grangeway

2007-04-23 17:21

reporter   ~0014386

I'd need to look at what the current adodb behaviour is, however from memory it was the following:

Prior to or up to mssql 7.0 (given there's been 2000 + 2005 since), varchar was limited to 1024 characters.
Since mssql 2000, it's up to 8000 chars

If I recall, the XS type was to be similar to the size of one of mysql's text types, without using 'text' as you can't do some queries against text. In fact, my commit to patch in XS last time effectively said it might not be required. Originally, X mapped to 'LONGTEXT' in mysql, where we wanted 'TEXT'. I can't recall whether adodb added X/XL to deal with what I was using XS/X for, or whether it was just a bug fix within adodb.


The fixes we added for minor notices got resolved in newer releases.

There is however a 1 line change that would be needed to support php-compiler.net, albeit again, something that can probably go into adodb.


The X/X2/B notnull stuff again was something that seemed silly.

Issue being, mysql/mssql/postgres/oracle, afaik all allow BLOB types to accept null's. Apparently some databases do not, so the adodb datadict stuff blocks it.

Given that we use null's in mantis (and it makes sense), and all the databases so far we've tried to use mantis with support it, i'm not sure exactly why those lines are in the adodb source.


In terms of mantis, my personal thought is that we should only support the bundled library. "On the other side, you will stop worrying to ugrade the library in case of security fixes " --> that doesn't make much sense. If one of our bundled libraries has a security issue, we should roll a new release anyway, even if it is just to update that library. By splitting mantis<>adodb, it's effectively an untested version --> The 2-3 times we have updated adodb during releases, I've tended to find that they've changed mappings ot types within db's in some way.

Paul

giallu

giallu

2007-04-24 02:31

reporter   ~0014388

In reply to (I think we miss a "reply" button...)

In terms of mantis, my personal thought is that we should only support
the bundled library. "On the other side, you will stop worrying to ugrade
the library in case of security fixes " --> that doesn't make much sense.

It does not makes sense in a Windows world, where you can not assume other packages to be present and with a dedicated maintainer.
In Linux, there is a clear split of responsibility between packages, so a security hole in adodb is addressed once by the adodb maintainer; on the other side, as a sysadmin you REALLY want to be sure the adodb update really closed the vulnerability of your system. If some component is not using the system wide lib, you have no guarantees this will happen

If one of our bundled libraries has a security issue, we should roll a new
release anyway, even if it is just to update that library.

That's exactly why bundling 3rd party libs is considered a bad practice: if I run in my system 10 services based on adodb, and each one has its own library bundled, when a security fix arrives the admin has to upgrade all of them.
In the other scenario, only one package is updated.

By splitting mantis<>adodb, it's effectively an untested version --> The 2-3
times we have updated adodb during releases, I've tended to find that
they've changed mappings ot types within db's in some way.

This is a sane concern. I am not sure what changing DB mappings means or imply, but any user in Fedora and Debian will be testing mantis + system adodb daily, so if anything breaks you will know...

schoenfeld

schoenfeld

2007-04-24 06:16

reporter   ~0014392

Hi,

i wanna give my 2 cents as the debian maintainer. Thanks to giallu for pointing me out on this.

Firstly: Yes, i do use libphp-adodb in my debian packages, which is the debian adodb package. I do not even ship mantis with your adodb. This is not only true for libphp-adodb, but also for libphp-phpmailer. And as far as i can tell this is not a problem so far. Personally i didn't see any difference in my various test installations and I'm aware of at least 40 installations (according to debian popcon [1]) of my packages. That does not necessarily mean, that they don't have problems, but if they do have problems, then they did not report them.

Okay, so why do i handle it different then mantis? Cause it is IMO the better way. Yeah, i understand that you frighten the possibility to have a broken mantis with a new adodb. But the chance to break things is not as worse, as the problems you introduce with an own installed version of a product you do not maintain. Take security as an example. Having more then one version of adodb on a system is a big fat potential problem. As an admin you do have to care for every version of adodb on a system. If more products like yours would act like this, it could be that admin would need to fix a lot of adodb installation instead of just fixing one. This would lead him to the conclusion that he better could do things himself, instead of trusting in a distribution. That is why it is not acceptable for me, to use the included adodb.

vboctor wrote:

  1. I believe that paulr (grangeway) has introduced a new field type
    which is XS.

Well, but if you need to change adodbs behaviour then you should forward such requests upstream. Cause it does not make much sense to have a complete own adodb copy just for some microscopic changes.

grangeway wrote:

If one of our bundled libraries has a security issue, we should roll a new
release anyway, even if it is just to update that library.

Yeah, but that does not make much sense, if the issue could be solved just by updating the library, cause it will significantly slow down the process of fixing security issues at target systems, which is not good at all. Also several people have to work with such a problem even though that less people could have solved an issue, if we would use system-wide libraries.

In any case it is not a good relation between use and effort. Neither for you as an upstream developer, nor for us as maintainers of specific distributions.

So IMO we need to find a way in the middle.

Best Regards

Patrick
[1] http://qa.debian.org/popcon.php?package=mantis

vboctor

vboctor

2007-05-18 03:30

manager   ~0014554

grangeway, any chance to update Mantis to latest ADODB and retire our custom changes or report them mainstream? Would also be nice if we create a Wiki page that is associate with this issue which outlines the mods, why they are needed and links to bugs in ADODB website that track our bug / feature requests to them.

vboctor

vboctor

2007-05-18 13:20

manager   ~0014560

Regarding the point that removing $t_core will not affect the current user. That is probably true if core/ under mantis/. However, if the user elects to move core outside the web root then that won't work.

Ideas are welcome to accommodate both scenarios.

vboctor

vboctor

2007-05-22 02:35

manager   ~0014582

I've upgraded ADODB from 4.80 to 4.95a in the 1.1.0x branch. I've reverted all our changes and changed our schema to use the standard XL rather than XS which was introduced by mods to ADODB. I've tested on MySQL and all seems to work. I would appreciate testing on other DBMSes.

Paul, let me know if you think we are broken and we need to re-introduce some of these changes. But I would really like this time we communicate very well why we are doing the mods and report bugs / feature requests against ADODB.

giallu

giallu

2007-05-22 03:00

reporter   ~0014584

In reply to comment 0007907:0014560:

victor, I don't think that is the case. The only file with an "include" for adodb is database_api.php (which is in core/), so the proposed modification already covers the case core/ resides outside the webroot, it's just that database_api.php will use a relative instead of assolute path for locating adodb.

The only case this would break is when a user wants to move adodb outside of core (the relative path would not find it), but in that case he should change the include path anyway.

vboctor

vboctor

2007-05-22 03:15

manager   ~0014587

giallu, good point. I've committed the fix to not include $t_core_dir.

giallu

giallu

2007-05-26 18:30

reporter   ~0014643

sorry. wrong target version...