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

Overwrite permissions of zip files for FAT jars #598

Merged
merged 7 commits into from
May 5, 2020
Merged

Overwrite permissions of zip files for FAT jars #598

merged 7 commits into from
May 5, 2020

Conversation

dfreilich
Copy link
Member

Some older methods of DOS compression don't contain proper permissions (e.g. FAT formatted jars, vs unix), and deny buildpacks access to their files. This PR identifies FAT files based on the first byte of their header.CreaterVersion, as done in the zip source (https://golang.org/src/archive/zip/struct.go).

Closes #464

Signed-off-by: David Freilich dfreilich@vmware.com

…issions (e.g. FAT jars)

Signed-off-by: David Freilich <dfreilich@vmware.com>
@dfreilich dfreilich requested a review from a team as a code owner April 30, 2020 14:11
Signed-off-by: David Freilich <dfreilich@vmware.com>
@codecov
Copy link

codecov bot commented Apr 30, 2020

Codecov Report

Merging #598 into master will increase coverage by 0.30%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #598      +/-   ##
==========================================
+ Coverage   70.14%   70.44%   +0.30%     
==========================================
  Files          64       64              
  Lines        3684     3695      +11     
==========================================
+ Hits         2584     2603      +19     
+ Misses        770      762       -8     
  Partials      330      330              
Flag Coverage Δ
#os_linux 70.44% <100.00%> (+0.30%) ⬆️
#os_macos 66.30% <100.00%> (+0.31%) ⬆️
#os_windows 100.00% <ø> (ø)
#unit 70.44% <100.00%> (+0.30%) ⬆️
Impacted Files Coverage Δ
internal/archive/archive.go 45.98% <100.00%> (+7.92%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 82549aa...e13709f. Read the comment docs.

Copy link
Contributor

@zmackie zmackie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple comments.
The function name is unconventional. I would say, prefer is functions generally for booleans.
Function call location is fine...that loop is so gnarly I'm not sure how to untangle it.

internal/archive/archive.go Outdated Show resolved Hide resolved
internal/archive/archive.go Outdated Show resolved Hide resolved
Signed-off-by: David Freilich <dfreilich@vmware.com>
dfreilich added 2 commits May 1, 2020 11:59
* Remove Focus on test

Signed-off-by: David Freilich <dfreilich@vmware.com>
Signed-off-by: David Freilich <dfreilich@vmware.com>
@jromero jromero requested a review from zmackie May 1, 2020 11:19
dfreilich added 2 commits May 1, 2020 18:01
* Allows for possibility of file not being stored as FAT, and therefore not overwriting permissions

Signed-off-by: David Freilich <dfreilich@vmware.com>
Signed-off-by: David Freilich <dfreilich@vmware.com>
@jromero jromero dismissed zmackie’s stale review May 5, 2020 00:50

Addressed

@jromero jromero merged commit 70d22be into buildpacks:master May 5, 2020
@dfreilich dfreilich deleted the origin/464-set-fat-jar-permissions branch May 5, 2020 10:58
@jromero jromero added this to the 0.11.0 milestone May 5, 2020
@jromero jromero added the type/enhancement Issue that requests a new feature or improvement. label May 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement Issue that requests a new feature or improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Permission denied within FAT formatted jars
4 participants