Bug: All zip types are accepted

[aljawaid]

I just tried to manually install a random zip file and it gave a success alert that the plugin is installed successfully. BUt the plugin does not show up anywhere obviously as it was just a random .zip of files unrelated to kanboard plugins.

I was expecting an error alert to say the zip file is not recognised.

@alfredbuehler Can you please have a look at that part of the code again?

[creecros]

I'd suspect the same with install by URL. I guess you should check for Plugin.php existence.

[alfredbuehler]

I think I did the same “verification” as KB does when installing a plugin. And this surely could be improved.
But is checking the existence of Plugin.php really sufficient? IIRC, KB silently ignores the failed installation of a “garbage” plugin.

[creecros]

It's definitely a minimumal check, but better than no check.

[aljawaid]

@alfredbuehler @creecros Can somebody please check this? I don't think the file version is working... because I THINK it looks for Plugin.php and never finds it because it will be in a subfolder and I don't know how to check a dynamic subfolder

[aljawaid]

If I have broken the code due to converting to Kanboard syntax, just let me know so I can revert it to the original PR

[aljawaid]

There is little kanboard checks for the from file and no checks for the from url... I have done the from file but I dont know how to code the from url in

PluginManager/Controller/PluginManagerController.php

Line 54 in 9541f54

private function installByURL(string $archiveUrl)

[alfredbuehler]

OK, I'll have a look.

[alfredbuehler]

It should be fixed by 38db9fc

[aljawaid]

It should be fixed by 38db9fc

I dont understand the merge, will look at it properly from a computer tonight... I noticed you moved the redirects in each error... I did it that way because if the install fails for whatever reason, a user should return to the page it came from - manual plugins.... kanboard by habit, redirects to the installed page regardless of response which is wrong i believe. I will put them back in for the erro messages once I can get my head around the changes.

[alfredbuehler]

But this is much clearer now. On success, it redirects to the installed plugin page. On failure, it remains on the manual plugin page, with an error message on top. It doesn't make sense to leave the page after a failure.

[aljawaid]

But this is much clearer now. On success, it redirects to the installed plugin page. On failure, it remains on the manual plugin page, with an error message on top. It doesn't make sense to leave the page after a failure.

exactly, agreed. Thats what I had done previously, maybe you did it in a better way.... anyway I will look at it properly tonight

[aljawaid]

For URL it is still not working

I tried installing this https://github.com/Tagirijus/AddShortcuts/archive/refs/tags/v1.6.1.zip and it should NOT be installed, do the same as a file install and it gives an error response (as expected)

[aljawaid]

For URL it is still not working

I tried installing this https://github.com/Tagirijus/AddShortcuts/archive/refs/tags/v1.6.1.zip and it should NOT be installed, do the same as a file install and it gives an error response (as expected)

@alfredbuehler I also think your speed on the manual page issue is to do with your last commit, as I am also having intermittent speed lags I am noticing now and then for the anual plugins page. Will keep an eye on that

[aljawaid]

I also tried with this url https://github.com/Tagirijus/AddSpentTime/archive/refs/tags/v1.3.0.zip... it says sucessful, does not install... but it should say error and not install

[alfredbuehler]

but it should say error and not install

Why? It was you, who wrote the plugin test. This plugin passes all the tests. It can be opened, contains at least one file, contains a Plugin.php, can be expanded…
As I mentioned earlier, these tests are very rudimentary.

[aljawaid]

Yes but it doesn't work when I do the same zip through the file option. Which seems to follow the tests. Are you sure the URL option is following the same tests?

[creecros]

the url, produces a zip, that meets all the criteria. your file does not. surely you can see that.

or maybe not, i have no idea what file you are uploading.

[alfredbuehler]

Are you sure the URL option is following the same tests?

Please see the code. The URL method calls the same method as for file, after downloading the archive.

[alfredbuehler]

Omitting the directory was not a great idea, you should assert that dirname == pluginname.

https://github.com/aljawaid/PluginManager/blob/master/Controller/PluginManagerController.php#L94

[creecros]

i just tested and both installed, obviously the folder name is incorrect to work, but they both installed it.

[aljawaid]

Omitting the directory was not a great idea, you should assert that dirname == pluginname.

https://github.com/aljawaid/PluginManager/blob/master/Controller/PluginManagerController.php#L94

Maybe I did it wrong then. I need help. I kitted the directory because plugin.php is in the subdirectory

[aljawaid]

i just tested and both installed, obviously the folder name is incorrect to work, but they both installed it.

Exactly but only as a success message, because of the wrong folder structure it will never install or show as installed but still give a success message.

[alfredbuehler]

Another solution could be, performing these checks after installing.

• Dir name wrong: remove plugin & fail
• Plugin.php missing: remove plugin & fail

[creecros]

i just tested and both installed, obviously the folder name is incorrect to work, but they both installed it.

Exactly but only as a success message, because of the wrong folder structure it will never install or show as installed but still give a success message.

i get the message for neither

[aljawaid]

I honestly don't know what to do

I will release it as it is and see if anyone reports anything.

Creecros says he got no error messages so that's baffled me more.

[alfredbuehler]

I will release it as it is and see if anyone reports anything.

No wait. I'll amend another check.

[alfredbuehler]

1bb35de

This now enforces namespace == directory == plugin name within the zip archive.