fix: read directory entries in name order so packs are reproducible - #93
Open
duranaaron wants to merge 1 commit into
Open
fix: read directory entries in name order so packs are reproducible#93duranaaron wants to merge 1 commit into
duranaaron wants to merge 1 commit into
Conversation
Member
|
lgtm |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DirectoryFileTreeReaderwalks a folder withFile.listFiles(), which returns entries in whatever order the file system stores them. That order is a property of the file system rather than of the files, so two machines holding the same folder can read it in two different orders.ResourceContainerImplkeeps its resources inLinkedHashMaps andMinecraftResourcePackWriterwrites them in insertion order, so the read order becomes the zip entry order. Two servers with identical files then build packs that have identical content but different bytes, and so different hashes.I hit this on a Minecraft network where two backends held byte-identical resource files and produced a different pack hash each. The two servers had separate ext4 file systems, and ext4 orders directory entries by a hash of the file name seeded with a random value written at mkfs time, so the two orders never matched. It does not reproduce on a single machine, because both directories are then on the same file system.
This sorts each folder's entries by name before reading them, so the read order depends only on the file names.
The added test writes files in a non-alphabetical order and checks that the reader returns them sorted. Without the change it fails with:
All 41 tests in
creative-serializer-minecraftpass with the change.One unrelated note:
creative-apidoes not compile onmainright now, becauseSpecialRender.playerHead()is declared twice, at lines 167 and 277. I patched that locally so I could run the tests, and left it out of this branch.