Now that #652566: Support multiple download files per release in release history XML is committed and deployed on updates.drupal.org, the release history XML feed generates PHP warnings on the legacy crappy PHP4 XML parser in update.module in D6 core. We knew this was going to happen, but figured it was a lesser evil to the complication of a whole separate tree of release history XML files, especially given that official releases of D6 core don't ship with E_ALL enabled.

Luckily, it's a trivial patch that's already been well tested by both myself and mikey_p. Instead of it being lost and forgetten over at #555362-1: Support multiple download links in the available updates report (.tar.gz + .zip packages like on drupal.org) (which is probably never going to be backported to D6, anyway), I wanted to open a new issue for the patch:

http://drupal.org/files/issues/555362-1.update_parse_files.D6.patch

Over a year later, it still applies cleanly to update.fetch.inc. ;)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dww’s picture

Just to be clear: D7 core doesn't need this fix, since the legacy PHP4 parser is already gone. There's no "Fix in HEAD, then backport" here because this bug only effects D6 core...

merlinofchaos’s picture

Status: Needs review » Reviewed & tested by the community

Looks fine to me, inasmuch as I remember that crappy XML parser. If dww and mikey_p have both tested it, then +1.

NROTC_Webmaster’s picture

Worked for me

westwesterson’s picture

tested! Works!

greg.harvey’s picture

+1 - applied cleanly and works in Pressflow too, FYI...

justin.cherniak’s picture

So if this works for everyone, what is the possibility of getting it in the next Drupal release?

dww’s picture

Hopefully there's a strong possibility of getting this committed before the next release. To help, the folks who test this should document their environment when they reply to say "it works", at least the php version. We need to have this manually confirmed on php 4 vs. 5, possibly multiple versions of each (e.g. 4.3, 4.4, 5.2 and 5.3).

Thanks,
-Derek

justin.cherniak’s picture

Works for me using php 5.3

joshuajabbour’s picture

works on php 5.2.13

greg.harvey’s picture

@dww - sorry, update based on #7: PHP 5.3 + latest Drupal and Pressflow cores - it worked fine.

dww’s picture

Status: Reviewed & tested by the community » Needs review

Has anyone tested this with PHP 4.4 (and perhaps 4.3) yet? Once you have, please set back to RTBC. Thanks!

alberto56’s picture

subscribing, thanks.

Shadlington’s picture

Sub'd

caschbre’s picture

I applied the patch and it appears to have resolved the issue for me as well.

vectoroc’s picture

subscribing

dww’s picture

@caschbre: What version of PHP are you using?

@all: We need testers for this patch with PHP4.

Thanks,
-Derek

caschbre’s picture

I'm using PHP 5.2.14

klonos’s picture

@dww: I know we need people in php4 to test this and see if anything breaks Derek, but the fact remains that by waiting for this to happen all of us in php5 need to address *actual* issues we are faced with while we wait for people to test for any *possible* issues. Besides, servers still in php4 possibly won't even have updated installations of D6 to begin with -not 6.x-dev-. So we might wait for ever here and have (yet) another patch to remember to maintain in our setups during each update.

Please push this and let people still in php4 file separate issues if they (ever) have problems.

Thanx for considering this.

dww’s picture

I can't push this, only Gabor. I'm just trying to get people to focus on steps that will aid that happening.

klonos’s picture

Yeah, the "Please push this..." paragraph was not for you alone/specifically. It's just that you are the one that created this patch and even though you cannot actually push (as in committing it yourself) you can "push" others to push it. Is Gábor the only one that can give this a proper review? Can't we "ping" anyone else?

Thanx for your work btw ;)

dww’s picture

Right, and I'm trying to say, if we set this to RTBC (which is when Gabor is likely to look at it), if there's no comment that clearly confirms it was tested in PHP4, he's just going to set it back to 'needs review', not 'fixed'...

klonos’s picture

Point taken.

caschbre’s picture

So what needs to be done to test in PHP4? I typically use the acquia DAMP stack so I don't mess with PHP, apache, etc.

Would I need to create a virtual machine then manually load php4, mysql, and apache?

greg.harvey’s picture

An old release of something like XAMPP would do it...
http://sourceforge.net/projects/xampp/files/

If someone has the chance to install one of those LAMPP or WAMPP installers from 2003, that'll be PHP4.

klonos’s picture

I can do it, if only someone please specify in detail the tests that need to be done.

dww’s picture

Pretty standard testing:
- Turn on E_STRICT
- Fetch available updates and see PHP notices being generated
- Apply the patch
- Re-fetch available updates and see no PHP notices

Thanks!
-Derek

deekayen’s picture

I know it's not PHP4, but the notices have gone away after applying this on Ubuntu 10.04. The available updates page also looks sane.

PHP 5.3.2-1ubuntu4.7 with Suhosin-Patch (cli) (built: Jan 12 2011 18:36:55)

VM’s picture

Just worked with this patch on PHP 5.3.3 in Ubuntu 10.10 and all seems to be working perfectly fine. I don't have PHP4 to test against.

I'll try to dig out an old laptop that I can throw PHP 4 on and see if this issue can't be moved forward.

anantagati’s picture

Works good for on PHP 5.2.10-2ubuntu6.4 and Pressflow 6.20.97.

greg.harvey’s picture

There is no need for anyone else to report it works in PHP 5! It does! ;-)

See: http://drupal.org/node/1006938#comment-4188754

VM’s picture

I've been working on this for a portion of the day. I've made my way through versions of XAMPP up to 1.4.12 (2005) and can't get drupal installed to test the patch on PHP4 as of yet. I keep running into "notice: Use of undefined constant PHP_INT_MAX - assumed 'PHP_INT_MAX' in C:\xampp\xampp\htdocs\drupaltest\includes\database.inc on line 212." which stops me from getting to the configuration screen after the profile runs and installs the core modules.

I'll keep plugging away but thought I'd update on progress. If anyone has a suggestions on how to get around PHP_INT_MAX issue so as that I don't get too far away from minimal Drupal 6 requirements, I'd appreciate it.

tstoeckler’s picture

Is there an easy way to install PHP4 on Ubuntu Maverick, i.e. a package repository where old packages are maintained or something?

deekayen’s picture

Why not just grab an old ISO of Debian or Ubuntu for a VM instead of trying to backport a 5.x install?

greg.harvey’s picture

@deekayen - that's not a bad idea either, but #31 is odd. It sounds as though something else has already broken PHP4 compatibility for Drupal 6, in which case this whole test is null and void.

I'm now sufficiently curious to bother creating a VM of an old Linux release.

mlee11111’s picture

Subscribing.

DeFr’s picture

Ok, so, yet another round of my terminal being flooded by this notice actually managed to trick me into installing a brand new VM running Ubuntu 6.06 to try to get the needed confirmation on PHP4.

First of all, it looks like support for PHP < 4.4 is already broken: according to http://php.net/manual/en/language.types.integer.php , PHP_INT_MAX has available "only" since PHP 4.4.0. This might be a problem in itself, given that INSTALL.txt still list PHP 4.3.5 as the minimum version, and its still what's defined by system.module . It was introduced in #499254: BIGINT handling, and probably broked pretty much everything for PHP < 4.4, since it now compares the value to the string 'PHP_INT_MAX', which will be true for all positive integers...

I've thus tested only in PHP 4.4, more precisely 4.4.2-1build1 (corresponding to http://packages.ubuntu.com/dapper/libapache2-mod-php4 ). What I did to setup the whole thing, in details :

  1. Download Dapper Drake iso from ubuntu.com
  2. Setup a new vm, booting on the iso, and choosing Setup a LAMP server
  3. Follow the basic debconf setup, reboot and get to a standard prompt
  4. Enable the universe source, because Dapper already provided php5 by default, and php4 is only available as a "community supported" option
  5. apt-get install libapache2-mod-php4 php4-mysql php4-gd , this gets rid of the default PHP5 installation, and brings PHP4 back.
  6. Tweak /etc/php5/apache2/php.ini to boost the memory limit from 8Mb (those were the times !) to 64Mb and change the error_reporting to E_ALL | E_STRICT
  7. Create a new database, download today's Drupal 6.x-dev, and a few releases of CCK (6.x-2.5, 6.x-2.8 and 6.x-2.9)
  8. Installed Drupal, installed the Content submodule of CCK 6.x-2.5, went to admin/reports/updates , forced the update check

At this point, I got exactly the same flood of notices and the big red warning that CCK 2.5 contained a security issue and should be updated as soon as possible, which was the expected output.

I then applied the patch, re-forced the update check, and this time, the notices were all gone, which sounded promising. CCK 2.5 was still marked as containing a security issue, which was good too.

I then upgraded CCK to 2.8, and the message correctly changed to a yellowish "That's not the recommanded version". After upgrading to CCK 2.9, all was green again. I haven't dealt with the update code much, but I guess this all means that both before and after the patch, D6 can still parse the informations update on PHP 4.4.2.

I would thus say that this patch is now tested in PHP 4.4.2 too, but I haven't done significant work in the VM, so if there's something else you'd like me to do to be super extra sure that it's still working, just ask.

dww’s picture

Status: Needs review » Reviewed & tested by the community

@DeFr: Perfect, thanks! Wish we had gotten this in before 6.22, but I guess we can assume a 6.23 will happen at some point. ;)

dww’s picture

p.s. Sounds like we either need to reopen #499254: BIGINT handling or create a new bug about updating the min PHP requirements for D6.

DeFr’s picture

@dww, was diving into the issue queue, and in fact, PHP_INT_MAX being undefined already has its own issue, #586468: Code uses a constant that is not defined in all version of PHP after version 4.3.5

dww’s picture

@DeFr: perfect, thanks for cross-linking that here. So, all that remains here is a "commush" from Gabor for the patch at http://drupal.org/files/issues/555362-1.update_parse_files.D6.patch ;)

dpearcefl’s picture

I got bit by this bug when I did a clean D6 install over the weekend so I'd like to see it get into the dev codebase also, which it is not.

I don't see that the testbot has processed the patch yet. Perhaps the reason is explained in the issue handbook under the "Needs Review" status:

A patch has been created and needs review and testing. The Testing Bot uses this status to trigger an automated review of the patch, and reports the results. Patch Reviewers also use this status to find patches that need to be reviewed and evaluated.

It looks like we may have skipped a step? The patch also needs to be attached to this ticket at the time the ticket goes to "needs review".

westwesterson’s picture

Uploading the same patch from above, in hopes it triggers test bot

klonos’s picture

dpearcefl’s picture

Sometimes the testbot ignores the patch if the filename is non-standard.

[description]-[issue-number]-[comment-number].patch
http://drupal.org/node/707484

You will need to resubmit the patch and change the status to needs review.

dww’s picture

There's no automated test suite for D6 core. The test bot ignores all issues that are for the "6.x-dev" version. All patches for D6 core require manual testing. Thanks for your attempts, but really, there's nothing else we can do here except wait for Gabor to commit my patch http://drupal.org/files/issues/555362-1.update_parse_files.D6.patch which has now been confirmed tested on both PHP 5 and PHP 4.

mikeytown2’s picture

Patch #42 works!

paulgemini’s picture

#42 Works for me too!

stemount’s picture

(Patch #42)++

DeFr’s picture

For the record, the patch posted in #42 is explicitely a repost of the first patch in this issue, which means that comment #7 and #30 still applies ; in short, the patch is known to work in PHP5, confirmation that it work in PHP4 was needed to get it commited though, which was given in #36 for PHP 4.4.2 . Obvisously, more reports that it work in PHP4 would be welcome, but all that's really needed here is to just wait for Gabor to comment and commit the patch, hopefully in his next round of reviews ;-)

VM’s picture

Status: Reviewed & tested by the community » Needs review

someone may want to ping gabor about this issue since it is specific to D6 ?

dww’s picture

Status: Needs review » Reviewed & tested by the community

@klonos: Thanks for the list of duplicates...

@VM: Not sure why you downgraded from RTBC. Leaving this in that state is (in theory) the best way to get Gabor's attention. Plus, my original patch has been reviewed and tested (heavily) by the community, so RTBC is still the right status here. Anyway, sure, I'll also ping him out of band about this.

VM’s picture

The switching of the status was unintentional. Must have been a slip of the mouse. My apologies.

Peter Bowey’s picture

#42 Works for (and is required) for Pressflow 6.22 too!

The fact that it NOT yet in Pressflow 6.22 surprises me - "a lot"!

"Hello' David Strauss" https://launchpad.net/~davidstrauss

Why is this 'FIX' not yet incorporated in D6 either?
The problem has been around for quite some time!

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

Now committed to Drupal 6 core, so it will be part of the next Drupal 6 release and consequently (I assume) Pressflow as well. Thanks!

klonos’s picture

Finally!!! Thanx Gábor.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

theohawse’s picture

don't see this in current pressflow distro, for those looking for this now, the following works for me with drush make:

core = 6.x
api = 2

projects[pressflow][type] = "core"
projects[pressflow][download][type] = "get"
projects[pressflow][download][url] = "https://github.com/pressflow/6/tarball/master"
projects[pressflow][patch][] = "http://drupal.org/files/issues/ereg-suppress_warnings-883038-drushmake-clean-D6.patch"
projects[pressflow][patch][] = "http://drupal.org/files/issues/555362-1.update_parse_files.D6_0.patch"

This will throw an error hopefully when pressflow gets a new release.

webservant316’s picture

I was directed to this issue from drupal.org/node/1193184.

just got this error when running update.php
===
warning: array_merge() [function.array-merge]: Argument #2 is not an array in update.php on line 173.
warning: Invalid argument supplied for foreach() in update.php on line 337.
===

I am running Drupal 6.26 and was upgrading 7 modules: flag, print, smtp, spambot, token, ubercart, and user_import.

This seems like an old issue, why would I get this error?

webservant316’s picture

Status: Closed (fixed) » Active
DeFr’s picture

Status: Active » Closed (fixed)

@webservant316: #1193184: Drupal Core's update module reports PHP Warnings was erroneously marked as a duplicate of this bug, instead of being marked as a duplicate of #274283: Warnings on running update.php . In short, one of the update performed didn't return an array as it was supposed too.

webservant316’s picture

oops sorry and thanks!