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

[Xamarin.Android.Build.Tasks] Fix issues with Library Resource Extraction #69

Merged
merged 1 commit into from
Jun 10, 2016

Conversation

dellis1972
Copy link
Contributor

@dellis1972 dellis1972 commented Jun 9, 2016

Since the switch to System.IO.Compression we have a few
issues when dealing with resource extraction.

  1. It was not ignoring __MACOSX or .DS_Store during extraction.
  2. It was not correctly detecting duplicate zip entries.

This commit updates the code to correctly filter __MACOSX and
.DS_Store entries. It also changes the way we check if an entry
exists in the zip already. And finally it updates
ResolveLibraryProjectImports to use the Files.ExtractAll method.
This is because the standard ZipFile.ExtractToDirectory has no
way of filtering the output (so it includes __MACOSX and .DS_Store).


if (string.Equals(entry.FullName, "__MACOSX", StringComparison.OrdinalIgnoreCase) ||
string.Equals(entry.FullName, ".DS_Store", StringComparison.OrdinalIgnoreCase))
if (entry.FullName.Contains ("__MACOSX") ||
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand how the previous check was "bad", and I'm not fond of the new check as it's locale-sensitive.

The commit message mentions that files named _MACOSX weren't ignored, but this new check still won't support that, as __ != _ -- the .Contains() check is looking for two underscores, not one, just as it originally did, so _MACOSX will still be processed.

Or is the problem that entry.FullName contains directory information, so it could be foo/bar/_MACOSX? If so, then the _MACOSX check should presumably follow the .DS_Store check and use .EndsWith() with StringComparison.OrdinalIgnoreCase.

Copy link
Member

Choose a reason for hiding this comment

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

If entry.FullName does contain path information, is the directory separator char at all normalized? The problem with the updated logic is that it'll process my-file.ds_store, which doesn't seem at all correct. Perhaps this should instead check for entry.FullName.EndsWith("/.DS_Store", StringComparison.OrdinalIgnoreCase)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The _MACOSX stuff appears like

'foo/bar/_MACOSX/.classes.jar'

hence the use of Contains :(

Copy link
Member

Choose a reason for hiding this comment

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

On the one hand, I'm wrong in saying that .Contains() is locale-sensitive. It isn't:

This method performs an ordinal (case-sensitive and culture-insensitive) comparison

Alas, that means it's case sensitive. Hopefully that won't break anything.

That said, it'll accept with some_file_name_that_contains_MACOSX.class, so if you happen to have some resource that you want that contains _MACOSX, you're screwed.

I would thus suggest two checks for _MACOSX, for a total of three:

if (entry.FullName.Contains ("/__MACOSX/") ||
        entry.FullName.EndsWith ("/__MACOSX", StringComparison.OrdinalIgnoreCase)) ||
        entry.FullName.EndsWith ("/.DS_Store", StringComparison.OrdinalIgnoreCase)))
    continue;

Additionally, the commit message should be updated to use two _s to be consistent with the code.

…tion

Since the switch to System.IO.Compression we have a few
issues when dealing with resource extraction.

1) It was not ignoring __MACOSX or .DS_Store during extraction.
2) It was not correctly detecting duplicate zip entries.

This commit updates the code to correctly filter __MACOSX and
.DS_Store entries. It also changes the way we check if an entry
exists in the zip already. And finally it updates
ResolveLibraryProjectImports to use the Files.ExtractAll method.
This is because the standard ZipFile.ExtractToDirectory has no
way of filtering the output (so it includes __MACOSX and .DS_Store).
@jonpryor jonpryor merged commit 43a8aa5 into dotnet:master Jun 10, 2016
radical pushed a commit that referenced this pull request May 8, 2018
…invalid name. (#69)

Addresses https://bugzilla.xamarin.com/show_bug.cgi?id=40172

setOn123Listener(On123Listener) will generate event named 123, but that
is not a valid C# name and will result in compilation error. So check
that name, warn it, and do not generate event.
grendello pushed a commit that referenced this pull request Aug 12, 2020
Fixes: dotnet/android-libzipsharp#64

Context: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1139578
Context: https://liquid.microsoft.com/Web/Object/Read/ms.security/Requirements/Microsoft.Security.SystemsADM.10039#guide

Changes: dotnet/android-libzipsharp@1.0.10...1.0.20

  * dotnet/android-libzipsharp@1752f95: Statically Linux libzip.so (#70)
  * dotnet/android-libzipsharp@2b1762f: Fix Dll SearchPath to include Assembly Directory. (#69)
  * dotnet/android-libzipsharp@28b4639: Merge pull request #68 from dellis1972/fixwindowssearch
  * dotnet/android-libzipsharp@fdabcda: Fix Windows to look in Assembly Directory for 32bit dll.
  * dotnet/android-libzipsharp@b332af0: Fix Native Crash on Windows. (#67)
  * dotnet/android-libzipsharp@96eb5e3: Use DefaultDllImportSearchPathsAttribute (#66)
  * dotnet/android-libzipsharp@755a42a: Bump libzip to 1.7.3 and go back to mingw (#65)
  * dotnet/android-libzipsharp@5ae5e70: Merge pull request #60 from xamarin/msvc-static-link
  * dotnet/android-libzipsharp@30ff680: Build libzip with static CRT and VS2019
  * dotnet/android-libzipsharp@bad320e: Merge pull request #59 from dellis1972/theswitcharoo
  * dotnet/android-libzipsharp@d7bc2c5: Merge pull request #58 from xamarin/optimize-winbuild
  * dotnet/android-libzipsharp@34dc213: Make 64 bit Linux native lib the default.
  * dotnet/android-libzipsharp@d3aad35: fixup! Optimize libzip build
  * dotnet/android-libzipsharp@0970a01: Optimize libzip build
  * dotnet/android-libzipsharp@d321af1: Merge pull request #57 from xamarin/fix-win32-packaging
  * dotnet/android-libzipsharp@80b739d: Fix a typo which caused 64-bit dll to be packaged for 32-bit Windows
  * dotnet/android-libzipsharp@1665db0: Bump the version (to 1.0.12), to prepare to release (#56)
  * dotnet/android-libzipsharp@dd5e939: Throw exception instead of silently failing if zip save/close fails (#54)
  * dotnet/android-libzipsharp@2df5b16: Fix enumerating zip with deleted entries (#53)
  * dotnet/android-libzipsharp@a042554: Add .editorconfig, copied from xamarin-android (#55)
  * dotnet/android-libzipsharp@a0973d4: Bump libzip to 1.6.1 (#49)

Changes: nih-at/libzip@rel-1-5-1...v1.7.3

  * Context: https://libzip.org/news/release-1.7.3.html
  * Context: https://libzip.org/news/release-1.7.2.html
  * Context: https://libzip.org/news/release-1.7.1.html
  * Context: https://libzip.org/news/release-1.7.0.html

Two primary changes of note in this xamarin/LibZipSharp bump:

 1. Bump to `libzip` 1.7.3, which contains numerious fixes and
    improvements over the previously used 1.5.1 release.

 2. Use of `DefaultDllImportSearchPathsAttribute` so that native
    library dependencies are loaded securely, i.e. w/o allowing
    "other" libraries to be loaded from unsafe directories.

Unfortunately, the `libzip` bump itself caused issues, PR #4751 and
PR #4937 each had integration tests "randomly" SIGSEGV.  This was
eventually tracked down to a bug within `libzip` itself, fixed at:

  * nih-at/libzip#202
jonpryor pushed a commit that referenced this pull request Aug 20, 2020
Fixes: dotnet/android-libzipsharp#64

Context: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1139578
Context: https://liquid.microsoft.com/Web/Object/Read/ms.security/Requirements/Microsoft.Security.SystemsADM.10039#guide

Changes: dotnet/android-libzipsharp@1.0.10...1.0.20

  * dotnet/android-libzipsharp@1752f95: Statically Linux libzip.so (#70)
  * dotnet/android-libzipsharp@2b1762f: Fix Dll SearchPath to include Assembly Directory. (#69)
  * dotnet/android-libzipsharp@28b4639: Merge pull request #68 from dellis1972/fixwindowssearch
  * dotnet/android-libzipsharp@fdabcda: Fix Windows to look in Assembly Directory for 32bit dll.
  * dotnet/android-libzipsharp@b332af0: Fix Native Crash on Windows. (#67)
  * dotnet/android-libzipsharp@96eb5e3: Use DefaultDllImportSearchPathsAttribute (#66)
  * dotnet/android-libzipsharp@755a42a: Bump libzip to 1.7.3 and go back to mingw (#65)
  * dotnet/android-libzipsharp@5ae5e70: Merge pull request #60 from xamarin/msvc-static-link
  * dotnet/android-libzipsharp@30ff680: Build libzip with static CRT and VS2019
  * dotnet/android-libzipsharp@bad320e: Merge pull request #59 from dellis1972/theswitcharoo
  * dotnet/android-libzipsharp@d7bc2c5: Merge pull request #58 from xamarin/optimize-winbuild
  * dotnet/android-libzipsharp@34dc213: Make 64 bit Linux native lib the default.
  * dotnet/android-libzipsharp@d3aad35: fixup! Optimize libzip build
  * dotnet/android-libzipsharp@0970a01: Optimize libzip build
  * dotnet/android-libzipsharp@d321af1: Merge pull request #57 from xamarin/fix-win32-packaging
  * dotnet/android-libzipsharp@80b739d: Fix a typo which caused 64-bit dll to be packaged for 32-bit Windows
  * dotnet/android-libzipsharp@1665db0: Bump the version (to 1.0.12), to prepare to release (#56)
  * dotnet/android-libzipsharp@dd5e939: Throw exception instead of silently failing if zip save/close fails (#54)
  * dotnet/android-libzipsharp@2df5b16: Fix enumerating zip with deleted entries (#53)
  * dotnet/android-libzipsharp@a042554: Add .editorconfig, copied from xamarin-android (#55)
  * dotnet/android-libzipsharp@a0973d4: Bump libzip to 1.6.1 (#49)

Changes: nih-at/libzip@rel-1-5-1...v1.7.3

  * Context: https://libzip.org/news/release-1.7.3.html
  * Context: https://libzip.org/news/release-1.7.2.html
  * Context: https://libzip.org/news/release-1.7.1.html
  * Context: https://libzip.org/news/release-1.7.0.html

Two primary changes of note in this xamarin/LibZipSharp bump:

 1. Bump to `libzip` 1.7.3, which contains numerious fixes and
    improvements over the previously used 1.5.1 release.

 2. Use of `DefaultDllImportSearchPathsAttribute` so that native
    library dependencies are loaded securely, i.e. w/o allowing
    "other" libraries to be loaded from unsafe directories.

Unfortunately, the `libzip` bump itself caused issues, PR #4751 and
PR #4937 each had integration tests "randomly" SIGSEGV.  This was
eventually tracked down to a bug within `libzip` itself, fixed at:

  * nih-at/libzip#202
@github-actions github-actions bot locked and limited conversation to collaborators Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants