Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mod_mirrorbrain victim of incompatibility between apr-util 1.2 and #4

Closed
poeml opened this issue Jun 5, 2015 · 0 comments
Closed

Comments

@poeml
Copy link
Owner

poeml commented Jun 5, 2015

                                                                                                                  [          ]

Issue migrated (2015-06-05) from old issue tracker http://mirrorbrain.org/issues/issue7

Title    mod_mirrorbrain victim of incompatibility between apr-util 1.2 and
              1.3
 Priority   urgent               Status        resolved
Superseder                     Nosy List       dfarning, poeml
Assigned To poeml               Keywords

msg17 (view) Author: poeml Date: 2009-10-06.22:28:51

the next issues....
I am getting the following error report when trying to download

Ah. Interesting one.


[Tue Oct 06 11:53:46 2009] [warn] [client 96.42.62.145] [mod_mirrorbrain] MirrorBrainEngine On,
mirror_base '/var/www/downloads/', referer: http://mirrorbrain-
testing.sugarlabs.org/sources/honey/InfoSlicer/
[Tue Oct 06 11:53:46 2009] [warn] [client 96.42.62.145] [mod_mirrorbrain] URI:
'/sources/honey/InfoSlicer/InfoSlicer-4.tar.bz2', referer: http://mirrorbrain-
testing.sugarlabs.org/sources/honey/InfoSlicer/
[Tue Oct 06 11:53:46 2009] [warn] [client 96.42.62.145] [mod_mirrorbrain] filename:
'/var/www/downloads/sources/honey/InfoSlicer/InfoSlicer-4.tar.bz2', referer: http://mirrorbrain-
testing.sugarlabs.org/sources/honey/InfoSlicer/
[Tue Oct 06 11:53:46 2009] [error] [client 96.42.62.145] [mod_mirrorbrain] could not resolve continent,
referer: http://mirrorbrain-testing.sugarlabs.org/sources/honey/InfoSlicer/
[Tue Oct 06 11:53:46 2009] [warn] [client 96.42.62.145] [mod_mirrorbrain] Country 'US', Continent '--',
referer: http://mirrorbrain-testing.sugarlabs.org/sources/honey/InfoSlicer/
[Tue Oct 06 11:53:46 2009] [warn] [client 96.42.62.145] [mod_mirrorbrain] AS '--', Prefix '--', referer:
http://mirrorbrain-testing.sugarlabs.org/sources/honey/InfoSlicer/
[Tue Oct 06 11:53:46 2009] [warn] [client 96.42.62.145] [mod_mirrorbrain] Successfully acquired database
connection., referer: http://mirrorbrain-testing.sugarlabs.org/sources/honey/InfoSlicer/
[Tue Oct 06 11:53:46 2009] [warn] [client 96.42.62.145] [mod_mirrorbrain] SQL lookup for (canonicalized)
'sources/honey/InfoSlicer/InfoSlicer-4.tar.bz2', referer: http://mirrorbrain-
testing.sugarlabs.org/sources/honey/InfoSlicer/
[Tue Oct 06 11:53:46 2009] [warn] [client 96.42.62.145] [mod_mirrorbrain] Found 2 mirrors, referer:
http://mirrorbrain-testing.sugarlabs.org/sources/honey/InfoSlicer/

Apache successcully connects and talks to the database, and it does find the file on two
mirrors. That's great, and everything should work but...

[Tue Oct 06 11:53:46 2009] [error] client 96.42.62.145Unknown error 18446744073709551615:
[mod_mirrorbrain] Error looking up sources/honey/InfoSlicer/InfoSlicer-4.tar.bz2 in database, referer:
http://mirrorbrain-testing.sugarlabs.org/sources/honey/InfoSlicer/

...something else seems wrong, and I'm quite puzzled by this one.

In fact, after reading mod_mirrorbrain.c back and forth I'm quite sure
that this means that reading result lines from the database query fails.

The module correctly gets (and logs) the number of rows in the result,
but iterating over the result set fails (apr_dbd_get_row() call).

I suspect that the Apache Portable Runtime version of Ubuntu 9.04, and
the shipped PostgreSQL database adapter are a little bit too old (or
missing a bug fix) so it results in different behaviour.

Yes, this is probably the reason. On my Ubuntu 9.04 system I see a
libaprutil1 package with version 1.2.12, and in the ChangeLog I see:

  • update to 1.3.0
    [...]
    • ) All DBD drivers now count rows from 1, which affects PostgreSQL and MySQL
      drivers in particular. Using row number zero is an error.
      [Bojan Smojver]

That would explain the error.

I can probably come up with a workaround. Sorry about the inconvenience...

msg19 (view) Author: poeml Date: 2009-10-06.23:19:52

The following patch seemingly makes mod_mirrorbrain work with the old libaprutil:

--- mod_mirrorbrain.c (revision 7798)
+++ mod_mirrorbrain.c (working copy)
@@ -1127,7 +1127,7 @@
const char val = NULL;
short col = 0; /
incremented for the column we are reading out */

  •    rv = apr_dbd_get_row(dbd->driver, r->pool, res, &row, i);
    
  •    rv = apr_dbd_get_row(dbd->driver, r->pool, res, &row, i-1);
       if (rv != 0) {
           ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r,
                     "[mod_mirrorbrain] Error looking up %s in database", filename);
    

I think we can quite easily change mod_mirrorbrain to work with both APR versions,
either by detecting the APR version at build time, or by adding the patch to the Ubuntu
packages.

I don't know whether this fix works well, I can't really promise that at the
moment, without looking into the database adapter (and its changelog)
again.

msg22 (view) Author: poeml Date: 2009-10-08.07:52:36

I'm adding the proposed fix to the Ubuntu/Debian packages.

I have not learnt yet how to properly add the change as patch when packaging Debian packages in the openSUSE build
service, but a simple & stupid

  sed -i 's/&row, i);$$/\&row, i-1);/' mod_mirrorbrain.c

in the debian/rules file has the desired effect at the moment. Not nice, but for pragmatical reasons (also because
I'll be away for vacation for a while) I'm fixing it this way now.

msg28 (view) Author: poeml Date: 2009-10-08.10:59:49

The packages with the fix are scheduled to be built in the openSUSE build service. They might
turn up later today, but as the build service is extremely slow currently, I have put manually
built packages here:

http://www.poeml.de/~poeml/DEBS/

msg76 (view) Author: poeml Date: 2009-12-01.21:24:38

After sugarlabs is running with the fix for a month already, I think we can close
this as fixed.

And since it is easy to do compile time detection of the APR version, I'll commit
a fix that does just that, so we can drop the patch from the Debian and Ubuntu
packages.

msg77 (view) Author: poeml Date: 2009-12-01.21:32:27

Fixed in trunk:
http://svn.mirrorbrain.org/viewvc/mirrorbrain/trunk/mod_mirrorbrain/mod_mirrorbrai
n.c?r1=7887&r2=7886&pathrev=7887

Will be in the next release.

msg88 (view) Author: poeml Date: 2009-12-03.23:05:30

See also issue 29.

msg212 (view) Author: poeml Date: 2010-09-03.12:13:00

The check could be done at runtime, rather than at compile time.

That would protect the user in scenarios where a wrong package is installed (one
that is compiled against a different APR version that what the system has).

There is apr_version() which returns the runtime APR version. See apr_version.h.

Apache uses it:

httpd2 -V | grep "APR "

Server loaded: APR 1.4.2, APR-Util 1.3.9
Compiled using: APR 1.4.2, APR-Util 1.3.9

It would also allow us to log the versions to the error log at Apache's start.

msg215 (view) Author: poeml Date: 2010-09-05.19:01:23

mod_asn got a fix for this now (r86).
http://svn.mirrorbrain.org/viewvc/mod_asn?view=revision&revision=86

msg217 (view) Author: poeml Date: 2010-09-05.20:10:07

mod_mirrorbrain now has the same fix (r8107).
http://svn.mirrorbrain.org/viewvc/mirrorbrain?view=revision&revision=8107

msg218 (view) Author: poeml Date: 2010-09-05.23:36:10

There is one remaining compile-time if-branch, which is not about database access
though. It takes care of choosing %/d%lld as format string for (potentially) large
integers in the SQL query (the one that retrieves hashes). apr-1.2 knew only %d,
while apr-1.3 knows %lld for 64-bit integers.

if (APR_MAJOR_VERSION == 1 && APR_MINOR_VERSION == 2)

define DBD_LLD_FMT "d"

else

define DBD_LLD_FMT "lld"

endif

I'm leaving that as is, since the consequences are much less relevant (if it
results in breakage at all). Hence, I close this issue as resolved.

History
         Date         User  Action                      Args
2010-09-05 23:36:13 poeml set    status: chatting -> resolved
                                   messages: + msg218
2010-09-05 20:10:07 poeml set    messages: + msg217
2010-09-05 19:01:23 poeml set    messages: + msg215
                                   title: mod_mirrorbrain incompatible with
2010-09-03 12:36:16 poeml set    libaprutil 1.2 -> mod_mirrorbrain victim of
                                   incompatibility between apr-util 1.2 and 1.3
2010-09-03 12:13:01 poeml set    status: resolved -> chatting
                                   messages: + msg212
2009-12-03 23:05:36 poeml set    status: chatting -> resolved
2009-12-03 23:05:30 poeml set    status: resolved -> chatting
                                   messages: + msg88
2009-12-01 21:34:13 poeml set    status: chatting -> resolved
2009-12-01 21:32:28 poeml set    status: resolved -> chatting
                                   messages: + msg77
                                   status: testing -> resolved
2009-12-01 21:24:40 poeml set    nosy: + dfarning
                                   messages: + msg76
2009-10-08 10:59:50 poeml set    messages: + msg28
2009-10-08 07:52:36 poeml set    status: in-progress -> testing
                                   messages: + msg22
2009-10-06 23:19:52 poeml set    messages: + msg19
2009-10-06 22:28:51 poeml create

(end of migrated issue)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant