[BC BREAK] File::dirname is relative to TL_ROOT now

[backbone87]

(new File($path))->dirname is now relative to TL_ROOT, but was an absolute path in contao <= 3.5.9

#8295

https://github.com/contao/core/blob/master/system/modules/core/library/Contao/File.php#L918

[leofeyer]

The code does not convert the path, therefore I assume that you have passed in a relative path?

preg_match('%^(.*?)[\\\\/]*(([^/\\\\]*?)(\.([^\.\\\\/]+?)|))[\\\\/\.]*$%im', 'i/am/relative', $matches);
print_r($matches);
/*
Array
(
    [0] => i/am/relative
    [1] => i/am
    [2] => relative
    [3] => relative
    [4] => 
)
*/

preg_match('%^(.*?)[\\\\/]*(([^/\\\\]*?)(\.([^\.\\\\/]+?)|))[\\\\/\.]*$%im', '/i/am/absolute', $matches);
dump($matches);
/*
Array
(
    [0] => /i/am/absolute
    [1] => /i/am
    [2] => absolute
    [3] => absolute
    [4] => 
)
*/

[fritzmg]

@leofeyer just test something like

var_dump((new \File('files/test.txt'))->dirname);

In Contao 3.5.4 for example, the output is

string(30) "C:\xampp\htdocs\contao-3.5.4/files" 

In Contao 3.5.12, the output is

string(5) "files" 

As backbone87 reported, the dirname property of the File class used to be absolute instead of relative. I am assuming this happens now due to the changes introduced for #8289 and/or #8295.

Could be that it was only the case under Windows maybe, I didn't test on Linux yet.

[fritzmg]

This is because previously, the File class used the dirname property of the PHP function pathinfo: https://github.com/contao/core/blob/3.5.9/system/modules/core/library/Contao/File.php#L189

case 'dirname':
    if (!isset($this->arrPathinfo[$strKey]))
    {
        $this->arrPathinfo = pathinfo(TL_ROOT . '/' . $this->strFile);
    }
    return $this->arrPathinfo['dirname'];
    break;

which returns the absolute path. Now however, the dirname property is done via a preg_match operation on the given file path: https://github.com/contao/core/blob/3.5.12/system/modules/core/library/Contao/File.php#L914

Thus, since Contao 3.5.10 File::dirname only returns an absolute path, if the object was given an absolute path, while previously it would always return an absolute path.

[leofeyer]

Fixed in 5062b96.

[agoat]

Now this BREAK my extension!!
What is the purpose to have the file path absolute?? I can´t do anything with that. In my case I use the path and name to feed the picture class. The picture- and/or image-class does not work with an absolute image path.

As a workaroung I insert the same str_replace as in the bugfix a0df959 for 3.5.14.
But why we are adding TL_ROOT to remove it multiple times later.

BC-Break or not, the path should stay relative!! If you need an absolute path you can add TL_ROOT.
Otherwise we have lot unnecessary string adding and removing
And I have to adjust my extension that was working with the last 5 contao 3.5 versions...

[fritzmg]

Your extension wouldn't have worked with previous versions.

[agoat]

Sure, it worked.
My site stops working when I updated to 3.5.14.

In my case I create more alternative image sizes inside my template with \picture::create and the path+name. This is not working with absolute paths anymore.
So it´s not directly breaking my extension, I could do this anwhere in other template files but my extension brings some benefits to do this.

Anyway, I don´t see the need for absolute paths. Never ever needed an absolute path inside contao (all file handling methods of contao are handling this for me).
And like I wrote above I don´t think it´s a good solution performance wise to add TL_ROOT to the path and later remove it again (e.g. in DC_Folder, my extension, a.s.o.).

[fritzmg]

No, I meant your extension wouldn't work in any Contao Version other than Contao 3.5.10 and up.

[agoat]

Yes, that would be the case. I am developing this extension [contentblocks] since half a year and starting with contao version 3.5.6. It was under heavy development (and still not fully feature complete) and I think I used the image-class inside templates after contao 3.5.10 was released.

[fritzmg]

Then you can surely understand, why it was changed back.

Also I don't see why your extension would not work with absolute image paths?

[agoat]

I understand why we try to avoid BC-Breaks. But that was 4 month between switching back.
The image class throws an exception with absolute paths not finding the image. But I will test it tomorrow on a clean installation..

[fritzmg]

What exception exactly?

[agoat]

If I try to generate a image with the absolute path the following error is displayed:

Fatal error: Uncaught exception InvalidArgumentException with message Image "/kunden/arne-stappen.de/cms/contao3_dev3/files/renderart/images/portfolio/schulzentrumwest/Render_4K_NNOnSO_PT_22.jpg" could not be found thrown in system/modules/core/library/Contao/Image.php on line 112

#0 system/modules/core/library/Contao/Picture.php(66): Contao\Image->__construct(Object(Contao\File))
#1 system/modules/core/library/Contao/Picture.php(86): Contao\Picture->__construct(Object(Contao\File))
#2 templates/renderart/cb_projectgrid.html5(64): Contao\Picture::create('/kunden/arne-st...', Array)
#3 system/modules/core/library/Contao/BaseTemplate.php(88): include('/kunden/arne-st...')
#4 system/modules/core/library/Contao/Template.php(277): Contao\BaseTemplate->parse()
#5 system/modules/core/classes/FrontendTemplate.php(46): Contao\Template->parse()
#6 system/modules/contentblocks/elements/ContentBlockElement.php(117): Contao\FrontendTemplate->parse()
#7 system/modules/core/library/Contao/Controller.php(484): Contao\ContentBlockElement->generate()
#8 system/modules/core/dca/tl_content.php(1166): Contao\Controller::getContentElement(Object(Contao\ContentModel))
#9 system/modules/contentblocks/dca/tl_content.php(77): tl_content->addCteType(Array)
#10 system/modules/multilingualelements/dca/tl_content.php(65): tl_content_element->addCteType(Array)
#11 system/modules/core/drivers/DC_Table.php(4321): MultiLanguageContent->addLabeltoElement(Array)
#12 system/modules/core/drivers/DC_Table.php(378): Contao\DC_Table->parentView()
#13 system/modules/core/classes/Backend.php(650): Contao\DC_Table->showAll()
#14 system/modules/core/controllers/BackendMain.php(131): Contao\Backend->getBackendModule('article')
#15 contao/main.php(20): Contao\BackendMain->run()
#16 {main}

The image exists at that place.
This is because the file->exist method (https://github.com/contao/core/blob/master/system/modules/core/library/Contao/File.php#L437) called in the image class (https://github.com/contao/core/blob/master/system/modules/core/library/Contao/Image.php#L110) adds the TL_ROOT (again).

[fritzmg]

I would report that as a bug then instead.