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

Calls to uri_stat() triggers a notice #12

Open
sjadema opened this issue Feb 26, 2019 · 15 comments
Open

Calls to uri_stat() triggers a notice #12

sjadema opened this issue Feb 26, 2019 · 15 comments

Comments

@sjadema
Copy link

sjadema commented Feb 26, 2019

When checking for directories with is_dir() and the directory exists, the following notice is thrown:

PHP Notice: Undefined offset: 0 in <SOME_PATH>/vendor/twistor/flysystem-stream-wrapper/src/Flysystem/Plugin/Stat.php on line 157

The following code should trigger the error:

<?php

use League\Flysystem\Filesystem;
use League\Flysystem\Memory\MemoryAdapter;
use Twistor\FlysystemStreamWrapper;

require_once 'vendor/autoload.php';

$filesystem = new Filesystem(new MemoryAdapter());
FlysystemStreamWrapper::register('test', $filesystem);

$directory = 'test:///test';
var_dump(is_dir($directory)); // Works as expected, no notice

mkdir($directory);
var_dump(is_dir($directory)); // Works as expected, but a notice is thrown
@sjadema
Copy link
Author

sjadema commented Feb 26, 2019

This happens because $metadata['visibility'] = false. It tries to use that false as an index for $this->permissions and the false is casted to 0. The indexes in $this->permissions['dir'] are private & public hence the 0 isn't found.

@sjadema
Copy link
Author

sjadema commented Feb 27, 2019

After doing some research, the error originates in het MemoryAdapter. When creating a new directory, the visibility isn't stored properly. I still think you should validate the metadata properties though.

@kor3k
Copy link

kor3k commented Aug 26, 2019

replace
https://github.com/twistor/flysystem-stream-wrapper/blob/master/src/Flysystem/Plugin/Stat.php#L157
with

$ret['mode'] += empty($this->permissions[$metadata['type']][$metadata['visibility']]) ? $metadata['visibility'] : $this->permissions[$metadata['type']][$metadata['visibility']];

@ronaldvdlp
Copy link

ronaldvdlp commented Aug 29, 2019

A recent change in league/flysystem (1.0.55) is causing more problems with this:
thephpleague/flysystem@b0f7bee
Error "Undefined index: 0666" is triggered in the mergeMeta function when calling $metadata = $this->filesystem->getMetadata($path); from Stat:getWithMetdata when working with the Local Adapter.

@kor3k
Copy link

kor3k commented Aug 29, 2019

the $metadata['visibility'] is supposed to be either a "named permission" string like public, or an octal pemission mode.

this should fix it for the moment:
https://github.com/twistor/flysystem-stream-wrapper/pull/15/files

but it looks it will be changed again:
thephpleague/flysystem#1051

@oofz
Copy link

oofz commented Mar 15, 2020

the $metadata['visibility'] is supposed to be either a "named permission" string like public, or an octal pemission mode.

this should fix it for the moment:
https://github.com/twistor/flysystem-stream-wrapper/pull/15/files

but it looks it will be changed again:
thephpleague/flysystem#1051

tested and works 👍

@hexus
Copy link

hexus commented Jul 28, 2020

This is affecting usage on PHP 7.4.

This repo hasn't been updated in quite a while, is it no longer being maintained?

@hexus
Copy link

hexus commented Oct 19, 2020

@twistor, any chance we could get #15, #18 and #19 reviewed or merged? It appears that #15 and #19 address this particular error in different ways, and #18 is just about PHP 7.4 support in Travis.

@kor3k
Copy link

kor3k commented Oct 19, 2020

@hexus mine #15 was just a quick fix, but #19 looks more sophisticated, so i'd prefer this one.
well, @twistor ain't responding for over a year 🤷‍♂️ so perhaps i'll make a fork and merge #18 and #19 in there.

@kor3k
Copy link

kor3k commented Oct 19, 2020

ok so here it is, tagged to v1.0.10:
https://github.com/kor3k/flysystem-stream-wrapper

overriding packagist repo source in composer:

    "repositories": [
        {
            "type": "vcs",
            "url": "[email protected]:kor3k/flysystem-stream-wrapper.git"
        }
    ]

@JoshuaBehrens
Copy link

JoshuaBehrens commented Nov 5, 2022

Hi @kor3k I just stumbled upon your fork and really like that you made sure to have a version of this with the available bug fixes merged. I want to refer to your version in my composer requirements. As only root composer files can override repositories I basically can't reference it. Would you mind submitting your version to packagist with your prefix so one can easily pull in your version as dependency?

I am a bit sad that @twistor neglects this. Solutions are available and there seems to be activity by @twistor on GitHub. If this is not updated anymore please mark it as archived so we can be sure that someone else needs to step in.

@kor3k
Copy link

kor3k commented Nov 8, 2022

@JoshuaBehrens hello and thank you.

yes i'll take a look on adding it to packagist.

but, this is for flysystem v1, which is obsolete now. (imho that's why twistor is neglecting it).
there is actively maintained version of stream wrapper for flysystem v2|v3. i recommend you to upgrade it in your projects if possible, like

"league/flysystem": "^3.0",
"m2mtech/flysystem-stream-wrapper": "^1.3",

@JoshuaBehrens
Copy link

JoshuaBehrens commented Nov 8, 2022

you're welcome @kor3k and thank you very much

I know I use the other wrapper already. In our library we use stream wrappers as abstraction layer. But at one point the library is used within shopware 6 and up to this point they depend on v1:

https://github.com/shopware/platform/blob/6.4.16.0/composer.json#L83-L85

        "league/flysystem": "~1.1.4",
        "league/flysystem-aws-s3-v3": "~1.0.29",
        "league/flysystem-memory": "~1.0.2",

@kor3k
Copy link

kor3k commented Nov 19, 2022

it is here: https://packagist.org/packages/kor3k/flysystem-stream-wrapper
and tagged to v1.0.11

@JoshuaBehrens

@JoshuaBehrens
Copy link

@kor3k thank you very much :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants