TYPO3 Core

Hi Jörg,

It seems that your patch is reversed, it should have been:

- } elseif(preg_match('/^"/', $v)) {
+ } elseif(preg_match('/^".*[^"]$/', $v)) {

Regards
Jacob

Yes Jacob, you are right.
I will post a new patch - together with more insights into this problem.

After feedback from Susanne Moog on typo3.teams.core I did some further investigation on this problem, as it appeared to be Windows specific. But the reason lies deeper and can affect Linux systems too.

Here are all details:

The path+file parts in the parameters argument of t3lib_div::unQuoteFilenames($parameters) are treated with a function wrapFileName() before they are concatenated into the parameters argument (see class.t3lib_stdgraphic.php, function combineExec).
wrapFileName() in turn uses the PHP function excapeshellarg to add surrounding quotes and to escape inner quotes in these strings.
Now, excapeshellarg works differently on Windows and on Linux: On Linux it puts single quotes around its argument, on Windows it uses double quotes (see http://www.php.net/manual/en/function.escapeshellarg.php, user comment by Egorinsk).

So we end up with something like this:
LINUX
+profile '*' -compose over +matte 'typo3temp/temp/969233c0cf24db7f144048d059b1032a_menuNT.gif' ...
WINDOWS
+profile '*' -compose over +matte "typo3temp/temp/969233c0cf24db7f144048d059b1032a_menuNT.gif" ...

Now, our beloved function unQuoteFilenames() only checks for double quotes when trying to do its explode-like job. So on Linux the whole algorithm never really activates - and the bug in it never shows up.

You can easily test this by doing static calls to unQuoteFilenames() in your IDE:

t3lib_div::unQuoteFilenames('aaa bbb "ccc ddd"')
: array =
0: string = "aaa"
1: string = "bbb"
2: string = "\"ccc ddd\""
(CORRECT!)

t3lib_div::unQuoteFilenames("aaa bbb 'ccc ddd'")
: array =
0: string = "aaa"
1: string = "bbb"
2: string = "'ccc"
3: string = "ddd'"
(WRONG BECAUSE OF SINGLE QUOTES - argument 3 is split up!!!)

t3lib_div::unQuoteFilenames('aaa bbb "ccc" "ddd" "eee"')
: array =
0: string = "aaa"
1: string = "bbb"
2: string = "\"ccc\" \"ddd\""
3: string = "\"eee\""
(WRONG BECAUSE OF THE BUG - arguments 3 and 4 are joint!!!)

t3lib_div::unQuoteFilenames("aaa bbb 'ccc' 'ddd' 'eee'")
: array =
0: string = "aaa"
1: string = "bbb"
2: string = "'ccc'"
3: string = "'ddd'"
4: string = "'eee'"
(CORRECT, but only because with single quotes the function's algorithm is dead.)

SOLUTION:
I am attaching a new patch that contains my original bug fix and additionally extends unQuoteFilenames() to recognize single AND double quote quoting. So the function will actually start to do what it was designed for on Windows and Linux systems.

Cheers,
Jörg.

The flaw in function unQuoteFilenames() is already present in the 4.2 branch. So I add a patch for that branch too.

Another issue: What to do with

+profile '*'

Is it necessary to preserve the quotes in some conditions?

Hi Georg,
the parameter part
+profile '*'
is correct.
It is used to strip any profile information from thumbnails to reduce their size. See http://www.imagemagick.org/Usage/thumbnails/#profiles for details. This parameter part can be changed or eliminated in localconf.php through $TYPO3_CONF_VARS['GFX']['im_stripProfileCommand']. The quotes around the asterisk are neeeded and an integral part of it.

BTW:
All and any quotes are left in place by t3lib_div::unQuoteFilenames()! The function's only task is to explode the input string along blanks while keeping quoted strings together (even if they contain blanks). But quotes are not stripped (as you can see in the above examples of the function's output).

I created a testcase in tests/t3lib/t3lib_div_testcase.php
(see patch testcase_4-4-alpha2_0013750.patch)

It tests some generic strings like those listed in the notes, and some real world examples.

Even with the patch many tests failed, so I made a rewrite of the method (see patch 4-4-alpha2_0013750.patch). All tests now succeed, both with single and double quotes.

@Jörg
I know about +profile '*'. The question was, if there is a call to t3lib_div::unQuoteFilenames from the graphics API that uses the $noQuote parameter. If there is, does this method break the +profile parameter? I just took a quick look at the methods, so I don't think so, but I'm not certain. If there is, my patch is rendered useless.

Georg,
sorry, but your solution is very flawed. Here are the worst (but far from all) defects:

1.
Multiple blanks in a quoted string are not recognized:
t3lib_div::unQuoteFilenames("'aa bb cc'")
: array =
0: string = "'aa"
1: string = "bb"
2: string = "cc'"

2.
Single quotes can close double quoted strings and vice versa:
t3lib_div::unQuoteFilenames("'aa bb\" cc'")
: array =
0: string = "'aa bb\""
1: string = "cc'"

3.
Your primary regex pattern accounts for only a fragment of the allowed characters in Linux path and file names. Considering that TYPO3 can run on a multitude of operating systems the situation becomes even more complex and your strategy is much to minimalistic. Some examples:
t3lib_div::unQuoteFilenames("'aa bb)cc'")
: array =
0: string = "'aa"
1: string = "bb)cc'"
t3lib_div::unQuoteFilenames("'aa bb!cc'")
: array =
0: string = "'aa"
1: string = "bb!cc'"
t3lib_div::unQuoteFilenames("'aa bb;cc'")
: array =
0: string = "'aa"
1: string = "bb;cc'"

Considering your testcases:
You write that many tests failed with the patched version. The original function version (with or without my patch) will in deed fail on all of your testcases which have a closing quote that is NOT at the end of the quoted string. The function expects the opening and closing quotes to enclose the WHOLE string. But many of your cases have an appended [0] after the quoted part of one of the parameters. Does this really happen? I am not much aware of the IM and GM parameter structure, but this looks like a contradiction to me. Maybe I am overlooking something her but I would argue that you cannot quote a parameter and then put a part of it outside of the quotes. What is this parameter structure...
"path/filename.gif"[0]
...meant for that you use so often in your testcases?

If these appended [0] parts in your testcases should indeed be irrelevant, than I strongly suggest to stay with the original function and my patch. All the remaining testcases should work well with it.

Cheers, Jörg.

Hi Jörg, please follow the discussion in Core List!

Hi Christopher. Ok, will do.
Just FYI: There is a new volley of patches available in the core list that will eliminate the fact that my solution shares #2 of the above defect list.
Cheers, Jörg.

REMINDER#3

For the sake of BugDay/201005, here is the current status (as discussed in core list):

A) Latest version of patches added here for r7477, trunk, 4.3.3 and 4.2.12.

B) I checked through the complete documentation of the ImageMagick command-line tool. The parameters used in Georg's testcase are definitely wrong. An integer in square brackets following a file name is allowed for source files and selects a single frame in a multi frame image format, but must be enclosed INTO the quotes and may never be appended AFTER them! (See section "Selecting Frames" of http://www.imagemagick.org/www/command-line-processing.html)
So all file parameters in the testcase that have appended brackets must be changed from...
"C:/Users/...fe_maskNT.gif"[0]
to
"C:/Users/...fe_maskNT.gif⁰"
With the latter form all testcases should work with the latest patches.

+1 on testing with GM on WIN7 for 4.2, 4.3 and 4.4

+1 testing on Windows Server 2008 with GM 1.3.12 and TYPO3 4.4.2

+1 on testing with IM 6.5.6 and TYPO3 4.3.5 (13750_trunk.diff)

In response to a diverging discussion in the core group I want to make some points clear here, so hopefully we can get this ironed out very soon:

1. My patch alters the behavior under Linux in only one detail:
With the original function, params that are quoted and do NOT contain spaces were WRONGLY joint with the next param:
t3lib_div::unQuoteFilenames('aaa bbb "ccc" "ddd" "eee"')
: array =
0: string = "aaa"
1: string = "bbb"
2: string = "\"ccc\" \"ddd\"" <--WRONG!
3: string = "\"eee\""
This has been corrected. But this improvement can be easily taken out if you insist on not altering the Linux behavior at all.

2. BESIDE THAT THE BEHAVIOR ON LINUX IS NOT CHANGED BY THE PATCH.

3. The only additional functionality is that with the patch the function works an single quotes too. The original works on double quotes only, which makes it fail on Windows. (Single quotes on the other hand do not occur under Linux.)

4. The whole discussion about parameters of the form
"path/filename.gif"[0]
does only affect the testcases presented by Georg. The behavior of the function towards this kind of parameter is NOT changed.
Actually this kind of parameter is WRONGLY (!) treated by the function WITH and WITHOUT the patch. No change here!

Please have a look at the patch. It is small and not difficult to understand.

Version 4 of the patch (pending in core list) makes the image functions of TYPO3 4.5 work on WIN7 - without it there is no way to generate images with masks, nice text and the like at all!

+1 only with patch 13750_trunk.diff it worked on Windows Server 2003 SP2 IIS-6.0 ImageMagick-6.3.7 TYPO3 4.4.4
and THANKS Joerg Wagner for your work!

Hi Jigal, thanks for daring to own this problem!

It would be great if you could change the title of this report to ...
1. include the Windows specific nature of the problem
2. remove the IM and GM references - they are not adding to the information

Proposal:
Several GIFBUILDER features broken on Windows
or even
Several GIFBUILDER features broken on Windows for TYPO3 since 4.2

With TYPO3 4.4.5 this bug gets worse.
Not only has it not been solved, but even the provided patch(es) will not work anymore because new Windows related flaws have been introduced.

This time:
Quoting of the path and filename of the ImageMagick/GraphicsMagick executable has been introduced with 4.4.5. Here is an example:
4.4.4:
C:\Programme\GraphicsMagick-1.3.7-Q8/gm.exe convert -geometry 50x50! "uploads/pics/myimage.gif⁰" "typo3temp/pics/f58c0b5193.gif"
4.4.5:
"C:/Programme/GraphicsMagick-1.3.7-Q8/gm.exe" convert +profile '*' -geometry 50x50! "uploads/pics/myimage.gif"[0] "typo3temp/pics/f58c0b5193.gif"

Both of these commands will work, if executed directly on the Windows command line. But the 4.4.5 version will fail when executed through the PHP exec() command on Windows.
The reason for that is the quoting of the IM/GM executable path and filename in the complete command line. If this is done, the whole string has to be quoted a second time to make it work with exec() on Windows. Details can be found in the posting of "luko post cz", 30-Jan-2008 02:15, on http://php.net/manual/en/ref.exec.php
The working version for 4.4.5 must look like this:
""C:/Programme/GraphicsMagick-1.3.7-Q8/gm.exe" convert +profile '*' -geometry 50x50! "uploads/pics/myimage.gif"[0] "typo3temp/pics/f58c0b5193.gif""

I will upload a patch to correct the problem (13750_Problem2_v1_4.4.5.diff). As with my earlier patch, this does not affect the behavior on Linux in any way. Both patches can and must be combined to get TYPO3 4.4.5 working on Windows.

And finally I cannot refrain from voicing some real criticism here:
The Windows functionality of TYPO3 is gradually falling apart. For the better part of a year all current TYPO3 versions of the branches 4.2, 4.3, 4.4 and 4.5 do NOT work on Windows.
While I am aware that Win is definitely not a widespread target platform for live systems, it surely is a standard working platform for many developers.
The continued malfunction over such a long time and so many versions sheds a bad light on TYPO3. We risk that new developers turn away from the system because they just cannot get it up and running on their local machines.

And I am quite sure that we will see this kind of problem again and again as long as the core team is not provided with a centralized testing environment that allows for easy unit testing on all major operating systems. As long as such a testing platform does not exist, the development process and its outcome can best be described as Russian roulette.

Sorry for the harsh words. I love TYPO3, but this is definitely going the wrong way.

Even the documentation quote to do so, it fails on my win

"C:/xampp/GraphicsMagick-1.1.7-Q8/gm.exe" identify -version
works
""C:/xampp/GraphicsMagick-1.1.7-Q8/gm.exe" identify -version"
doesn't work

"Der Befehl """C:" ist entweder falsch geschrieben oder
konnte nicht gefunden werden."

So double quoting all commands is no solution.

Steffen, your double quoted command works perfectly on my local Win XP system. Beside that the patch works perfectly on a Win 2003 Server SE life system under rather heavy load.
Attention: This double quoting does NOT work on the command line. It only works from within a PHP exec() command!
To quickly try out your command use this code in a PHP file:

<?php
$cmd = <<<EOT
""C:/xampp/GraphicsMagick-1.1.7-Q8/gm.exe" identify -version" 
EOT;

$ret = exec($cmd, $out, $stat);

echo '<'.'pre>';
echo "\r\nRETURN:\r\n$ret";
echo "\r\nOUT:\r\n";
var_dump($out);
echo "\r\nSTAT:\r\n$stat";
?>

Hint: The string concat in my < pre > tag is only needed to circumvent limitations of the bug tracker editor.

If you want to test directly from the command line in a DOS box on Win you have to prepend "cmd /c" (as the PHP exec() command does).
This will work:

cmd /c ""C:/xampp/GraphicsMagick-1.1.7-Q8/gm.exe" identify -version" 

Sure it fails on commandline, but it fails in exec too, i debugged this to get the executed command. I'm with php 5.3.1, GM 1.1.7 and win vista.

Just tested with your suggestion which works on my system using the dummy command also in exec:

$cmdLine = TYPO3_OS === 'WIN' ? "cmd /c \"$cmdLine\"" : $cmdLine;

You quoted that exec command do this, which seems not to be the case on my environment.

Hi Steffen,

it took me quite some installing and testing to get to the bottom of this.

<b>Result:</b>
The additional outer quotes work and are necessary for PHP versions lower than 5.3 on Windows. Anything >=5.3 does not need the outer quotes and will not work if they are used.
I tested this with PHP 5.2.5 (outer quotes needed), 5.2.6 (outer quotes needed) and PHP 5.3.4 (outer quotes not needed and not working).

The PHP 5.3.0 change log (http://php.net/ChangeLog-5.php) contains a hint that the exec() behavior has been changed:

Version 5.3.0
30-June-2009
...
Fixed exec() on Windows to not eat the first and last double quotes. (Scott)

Prepending "cmd /c" to the exec() command string does not change the behavior and does not help the problem in any of the above cases.

<b>Bottom line:</b>
The double quoting is the way to go, but is only needed and working for PHP versions below 5.3 on Windows. So this should work.

$cmdLine = (TYPO3_OS === 'WIN' && version_compare(phpversion(), '5.3')<0) ? 
"\"$cmdLine\"" : $cmdLine;

Could you check this line on your system? I currently do not have a TYPO3 4.4.5 running on my PHP 5.3.4 machine.

Another 4 weeks with TYPO3 not working on Windows installations.
4.5 will be launched in 2 days - and it will also not run on Windows.

What exactly is not working for you on Windows?

I am currently running an unpatched trunk here and to me it seems as if it is working fine as long as you don't use

[GFX][im_stripProfileCommand]
and
[im_useStripProfileByDefault]

Image checks in the install tool are working as expected and even a complex GIFBUILDER testpage is generated without any flaws.

Safe mode is off, which is recommended anyway, since this is a deprecated PHP feature.

System Win7 Ultimate - GM - XAMPP

Still I think this should be fixed somehow before 4.5.0 final.

Actually it seems to be working even with the stripprofilecommand settings.

So I guess this has been fixed somehow even though I can't see a commit message in the core list. At least not with this bug id.

Got it: Has at least partly been fixed in #24377

Because of endless discussions it was split in three parts:
- fix unquotefilenames (committed, fixes problems in windows)
- put frame parameter inside quotes (should be according to IM manual, but problems with safe_mode and seems to work with frame outside the quotes too)
- fix escaped quotes in safe_mode (safe_mode can be ignored)

No problems reported after fixing unquotefilenames, so situation seems okay in 4.5; only situation in older versions can be a problem en not really fixable with safe_mode enabled.

Sorry guys, I missed the fix in #24377. My fault.
I am happy to hear that 4.5 will finally be working on Win again!
Thanks for your efforts.

You mean you haven't tried 4.5rc1 yet??? :-)

Nope, I actually haven't!
I suffered so much pain during last years updates that I did not dare to hurt myself again. :)

Sorry for being a pain in the ass, but...

The problem of quoting of the IM/GM executable path and filename in the complete command line still persists in TYPO3 v4.5.0.

As discussed above, this happens only if three conditions are met:
TYPO3 >= v4.4.5
PHP < v5.3
OS = Windows
Under these conditions the whole IM/GM command line must be encapsulated into another pair of quotes.

I created a new bug report for this: #24931
It would be helpful if a moderator could mark this report as parent of #24931.