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

[zend-file-transfer]: unable to load adapters #11

Closed
glensc opened this issue Jul 9, 2019 · 9 comments · Fixed by #12
Closed

[zend-file-transfer]: unable to load adapters #11

glensc opened this issue Jul 9, 2019 · 9 comments · Fixed by #12

Comments

@glensc
Copy link
Contributor

glensc commented Jul 9, 2019

given code:

<?php
require __DIR__ . '/vendor/autoload.php';
$file = new Zend_File_Transfer();

with zf1s/zend-file-transfer installed, i get error:

PHP Fatal error:  Uncaught Error: Class 'Zend_Loader' not found in vendor/zf1s/zend-file-transfer/library/Zend/File/Transfer.php:67
Stack trace:
#0 vendor/zf1s/zend-file-transfer/library/Zend/File/Transfer.php(54): Zend_File_Transfer->setAdapter('Http', false, Array)
#1 test.php(5): Zend_File_Transfer->__construct()
#2 {main}
  thrown in vendor/zf1s/zend-file-transfer/library/Zend/File/Transfer.php on line 67

loading "zf1s/zend-loader", the error changes to:

PHP Warning:  include_once(Http.php): failed to open stream: No such file or directory in vendor/zf1s/zend-loader/library/Zend/Loader.php on line 134
PHP Stack trace:
PHP   1. {main}() test.php:0
PHP   2. Zend_File_Transfer->__construct() test.php:5
PHP   3. Zend_File_Transfer->setAdapter() vendor/zf1s/zend-file-transfer/library/Zend/File/Transfer.php:54
PHP   4. Zend_Loader::loadClass() vendor/zf1s/zend-file-transfer/library/Zend/File/Transfer.php:72
PHP   5. Zend_Loader::loadFile() vendor/zf1s/zend-loader/library/Zend/Loader.php:82

I've created gist to play around with:
https://gist.github.com/glensc/ce6a301ddc60a0d1dba8878ae32709c3

with git tags v1 and v2 respectively for the tests.

@glensc
Copy link
Contributor Author

glensc commented Jul 9, 2019

some thoughts: as constructor calls setAdapter unconditionally:

which calls Zend_Loader unconditionally:

zend-loader should be added as package dependency.

@glensc
Copy link
Contributor Author

glensc commented Jul 9, 2019

the current code ends up calling Zend_Loader::loadClass(Http) because $adapter = 'Zend_File_Transfer_Adapter_' . ucfirst($adapter); is never assigned.

    public function setAdapter($adapter, $direction = false, $options = array())
    {
        if (Zend_Loader::isReadable('Zend/File/Transfer/Adapter/' . ucfirst($adapter). '.php')) {
            $adapter = 'Zend_File_Transfer_Adapter_' . ucfirst($adapter);
        }

        if (!class_exists($adapter)) {
            Zend_Loader::loadClass($adapter);
        }

@falkenhawk
Copy link
Member

falkenhawk commented Jul 9, 2019

looks like I missed that one... it should probably be: (similar to the changes in https://github.com/zf1s/zf1/blob/1.13.0/packages/zend-translate/library/Zend/Translate.php#L130)

        if (class_exists('Zend_File_Transfer_Adapter_' . ucfirst($adapter)) {
            $adapter = 'Zend_File_Transfer_Adapter_' . ucfirst($adapter);
        } elseif (!class_exists($adapter)) {
            Zend_Loader::loadClass($adapter);
        }

... and zend-loader dependency should be probably added to the composer.json anyway

@glensc
Copy link
Contributor Author

glensc commented Jul 9, 2019

was not sure what was the purpose of the Zend_Loader::isReadable, so I came up with #12

@glensc
Copy link
Contributor Author

glensc commented Jul 9, 2019

ok, updated #12; however the code now runs without "zf1s/zend-loader" (see v3 tag in gist)

@glensc
Copy link
Contributor Author

glensc commented Jul 9, 2019

oh, btw, don't link to branches!

@glensc
Copy link
Contributor Author

glensc commented Jul 10, 2019

ironically, I don't even need to use that component. it was loaded but never used in the codebase.

so no pressure from my side to get this released :D

@falkenhawk
Copy link
Member

falkenhawk commented Jul 10, 2019

I am going to wait a bit with the next release, in case of other issues occur in the meantime, to avoid bumping versions for all components too often. If anyone needs to use this please switch to @dev version for now.

@glensc
Copy link
Contributor Author

glensc commented Jul 10, 2019

and as you said you missed this one: #11 (comment)

should probably inspect codebase for more of the adapter loading and check their compatibility.

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

Successfully merging a pull request may close this issue.

2 participants