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

Unzip problems with v1.8.0+ #22

Open
danherd opened this issue May 10, 2021 · 12 comments · May be fixed by #23
Open

Unzip problems with v1.8.0+ #22

danherd opened this issue May 10, 2021 · 12 comments · May be fixed by #23
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@danherd
Copy link

danherd commented May 10, 2021

Hi,

This change has broken this tool for me.

I am using the php:7.1-fpm-alpine Docker image and trying to run the security-checker fails with the following error:

image

v1.7.0 works fine:

image

I presume the problem is that the version of unzip within BusyBox cannot handle the zip file:

image

Because it works fine on the unzip command in my host OS (Ubuntu 20.04

Cheers!

Dan

@paras-malhotra
Copy link
Member

paras-malhotra commented May 10, 2021

@danherd, based on the error screenshot, wonder if the real error is:

unzip: can't change directory to '/tmp/php_security_advisories': No such file or directory

Can you try using the --temp-dir option and check if it works?

@danherd
Copy link
Author

danherd commented May 10, 2021

Hi,

That didn't work.

The error from unzip is because it can't extract the zip file to the named directory. It is not a permission issue - it is simply that the version of unzip on this Docker image cannot handle the zip file:

image

Whereas on my Ubuntu 20.04 machine:

image

Here it is on the Docker machine using the same folder:

image

Cheers,

Dan

@paras-malhotra
Copy link
Member

Ahh, ok. I imagine an unzip -q works? I'll send a PR to that effect if you can confirm. Thanks!

@paras-malhotra paras-malhotra added the bug Something isn't working label May 10, 2021
@danherd
Copy link
Author

danherd commented May 10, 2021

Unfortunately not! I think this version of busybox unzip has some sort of bug with the '-d' flag. It doesn't work, no matter what I pass (unless I pass '.', then it extracts the contents of the zip into the current folder)

@danherd
Copy link
Author

danherd commented May 10, 2021

OK, I've managed to fix it my side by just installing the unzip package as part of the Docker image build. Behold:

image

Maybe you can add a check to see if the unzip binary is at a minimum version before trying to use it?

Thanks for your help and also thank you for continuing this useful package!

@paras-malhotra paras-malhotra added help wanted Extra attention is needed and removed bug Something isn't working labels May 10, 2021
@paras-malhotra
Copy link
Member

@danherd, thanks for all the information.

I think we can add an option such as --use-ext to force using the php zip extension. I'd accept a PR to do this if someone has the time. I'll leave this open so that folks willing to contribute can :)

@paras-malhotra paras-malhotra added the enhancement New feature or request label May 10, 2021
@jleonardolemos
Copy link
Contributor

This is not a problem with the package, the zip lib does not create the directory and this was fixed in the latest version, if you update your zip lib you will see this fixed.

@jleonardolemos
Copy link
Contributor

But if the --use-ext is still useful i can try implement this feature

@toaomatis
Copy link

toaomatis commented May 17, 2021

I am running into this same issue with the php:7.4-cli-alpine3.12 base image. It uses

$ php -v
PHP 7.4.14 (cli) (built: Jan  7 2021 17:42:00) ( NTS )
Copyright (c) The PHP Group
Zend Engine v3.4.0, Copyright (c) Zend Technologies
    with Xdebug v3.0.1, Copyright (c) 2002-2020, by Derick Rethans

and

$ unzip
BusyBox v1.31.1 () multi-call binary.

Usage: unzip [-lnojpq] FILE[.zip] [FILE]... [-x FILE...] [-d DIR]

Extract FILEs from ZIP archive

	-l	List contents (with -q for short form)
	-n	Never overwrite files (default: ask)
	-o	Overwrite
	-j	Do not restore paths
	-p	Print to stdout
	-q	Quiet
	-x FILE	Exclude FILEs
	-d DIR	Extract into DIR

and

$ composer -v
   ______
  / ____/___  ____ ___  ____  ____  ________  _____
 / /   / __ \/ __ `__ \/ __ \/ __ \/ ___/ _ \/ ___/
/ /___/ /_/ / / / / / / /_/ / /_/ (__  )  __/ /
\____/\____/_/ /_/ /_/ .___/\____/____/\___/_/
                    /_/
Composer version 2.0.12 2021-04-01 10:14:59

@danherd
Copy link
Author

danherd commented May 17, 2021

Hi @toaomatis

Just add this to an appropriate place in your Dockerfile:

RUN apk update && apk add unzip

That should fix it until the --use-ext feature is available.

@toaomatis
Copy link

Hi @toaomatis

Just add this to an appropriate place in your Dockerfile:

RUN apk update && apk add unzip

That should fix it until the --use-ext feature is available.

Thanks, this solved the problem (for now). Was not aware there was an additional / external unzip package for Alpine as well. Always used the build-in / BusyBox one.

@jleonardolemos
Copy link
Contributor

jleonardolemos commented May 22, 2021

I just did a PR adding the --use-ext option
@paras-malhotra I dont know if it is OK fell free to change everything hahaha

@paras-malhotra paras-malhotra linked a pull request May 25, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants