Page MenuHomePhabricator

Diff3 version checks are too strict, doesn't detect working diff3
Closed, ResolvedPublic

Description

Author: hagge.lists

Description:
MY SYSTEM
My system as per 2008-03-13:

uname -a

FreeBSD {censored} 6.2-RELEASE-p7 FreeBSD 6.2-RELEASE-p7 #0: Sun Sep 16 17:25:13 CEST 2007 {censored}:/usr/obj/usr/src/sys/INTERCORNER i386

php --version

PHP 5.2.5 with Suhosin-Patch 0.9.6.2 (cli) (built: Jan 18 2008 03:00:34)
Copyright (c) 1997-2007 The PHP Group
Zend Engine v2.2.0, Copyright (c) 1998-2007 Zend Technologies

where diff

/usr/bin/diff

/usr/bin/diff --version

diff - GNU diffutils version 2.7

where diff3

/usr/bin/diff3

/usr/bin/diff3 --version

diff3 - GNU diffutils version 2.7

THE PROBLEM
Now onto the problem. In fact its more like a fiew related problems.
During the installation of MediaWiki 1.12.0rc1 I got the following message "GNU diff3 not found.". I know diff is installed on my system (as shown above) and I don't like errormessages, even small ones, so I investigated it further.
At first I thought the root to this problem/error/bug was that I'm using Safemode, but it is not. So I made a workaround/fix for it.

First part of the problem:
In the file ./config/index.php on line 501 the script is searching for "diff3 (GNU diffutils)" in the output from "% diff3 --version" and according to my output above that does not match and that is part of why MediaWiki do not find the diff-util on my installation.

Second part of the problem:
I am running PHP in safemode and therefore the function shell_exec() is not allowed (back-quotes is an alias of this function) which generates some PHP-warnings instead of doing what it is should to do.

Third part of the problem:
MediaWiki are searching for the diff-util using static paths and not using the $PATH-variable during installation. This is no problem for me but it might be for someone else.
I do not understand the search for "diff3.exe" with hardcoded paths in unix-style format. But I'm no big fan of windoze and it was alot of years ago I programmed for it so I might be missing something.

MY MODIFICATIONS
My modifications and my function is included in this bugreport and here is a description of what and how I modify:

  1. ./includes/ShellExec.php -- This file is my function to fetch output from shell applications and is working with safemode enabled (if safemode is properly configured ofcource).
  2. ./config/index.php -- I added a require_once() for my function and modified locate_executable() to fetch both the old version-output-format and my different one.
  3. ./maintenance/parserTests.inc -- I added a require_once() for my function and modified quickDiff() to use my function instead of back-quotes to execute.

I haven't found any where else the diff-util is used. You probably have better knowloedge of the codebase than me. If you find this modification useful let me know. I will be glad if anyone can test my code and verify it is working or correct me if I am wrong about anything.

Happy coding!
Regards
Anders, Sweden


Version: 1.17.x
Severity: normal
OS: FreeBSD
Platform: PC

Details

Reference
bz13353

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 10:07 PM
bzimport set Reference to bz13353.
bzimport added a subscriber: Unknown Object (MLST).

hagge.lists wrote:

Safemode-compatible function for shellcommand execution

Attached:

Shell execution is crippled in safe mode because PHP corrupts properly-escaped parameters.

What's the status? Fix or wontfix?

A bit of both (two issues are identified here).

The main issue is exec() and friends in safe mode. As pointed out in comment #2, it's patently broken in these setups. If there are enhancements we can make to help safe mode users that don't create issues for the rest of us then they should be incorporated into wfShellExec() rather than unconditionally failing like we currently do. However, that's outside the scope of this bug (on a tangent, anything not using wfShellExec() should be) and is a WONTFIX as far as I'm concerned.

The second issue here is that when we're looking for diff3, we're expecting some rather specific output from --version (which is still true with the new installer merged to trunk). The original report obviously has a working diff3, we just don't allow it because its version output isn't matching. This should certainly be fixed and I'm repurposing the bug to concentrate on that.

Fixed the diff3 issue in r76302.