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

Patcher needs to support Mac bundles #1461

Closed
colincornaby opened this issue Aug 12, 2023 · 15 comments
Closed

Patcher needs to support Mac bundles #1461

colincornaby opened this issue Aug 12, 2023 · 15 comments

Comments

@colincornaby
Copy link
Contributor

colincornaby commented Aug 12, 2023

I'm opening this as an issue - but it's more of a design proposal.

Mac Bundles

Mac applications ship as bundles. Bundles are essentially folders that appear as files on macOS. On any other system, and from within a Mac terminal and the file system itself, they will appear as folders.

Below the layout on disk of a recent Mac build of UruExplorer.app. Even though this looks like an application to a Mac user - it is actually a folder.

C:\USERS\COLIN\DOWNLOADS\PLASMA-MACOS-X64-EXTERNAL-RELEASE\PLASMA-MACOS-X64-EXTERNAL-RELEASE\CLIENT\URUEXPLORER.APP
└───Contents
    │   Info.plist
    │   PkgInfo
    │
    ├───MacOS
    │       UruExplorer
    │
    ├───Resources
    │   │   AppIcon.icns
    │   │   Assets.car
    │   │   banner.tiff
    │   │   Dirt.ICO
    │   │   MainMenu.nib
    │   │   PLSPatcherWindowController.nib
    │   │
    │   └───PLSLoginWindowController.nib
    │           keyedobjects-101300.nib
    │           keyedobjects.nib
    │
    └───_CodeSignature

Note: Bundles can contain other bundles. Within the .app bundle, there is PLSLoginWindowController.nib, which in itself is a bundle.

Code Signing

Bundles can also be code signed. For the code signature to be valid - the bundle must be exactly in the same state as it was when signed. No files can be added and no files can be removed.

The problem

The Uru patcher was never designed to deal with bundles. It can patch flat files - but not bundles.

Technically - the Uru patcher could approach a bundle as just another folder. But code signing would become an issue. The Uru patcher can add new files to a folder - but it cannot delete files that should no longer be present. This could cause the patcher to merge different versions of a bundle - which would lead to code signing errors.

Possible solution

I would propose adding a "Mac bundle" flag to the manifest. When the Mac bundle flag is encountered - instead of hashing based on a file, Uru will do a folder based hash. It will traverse the tree in alphabetical order and incrementally build a hash of the entire tree.

If the hash is found not to match the hash in the manifest, the client will delete the entire folder, download a zipped folder pointed to by the manifest, and expand it in place. (For self patching - this would might be alter in order to manipulate the application in the file system however is required to self patch.)

For example, the following line in the manifest could indicate a bundle to be patched by folder hashing:
UruExplorer.app,client\mac\external\UruExplorer.app.gz,cxb6af634b6de96985fb8defc83401d9,20dbasf1dc1e4ab7bata0888f700780b,16090624,6928132,24

Bundles should never be merged - so the patcher would need to make sure the resulting bundle is exactly what is contained in the gzip. No extra files should remain from the previous bundle.

Impact of change

  • The server architecture should remain unchanged. The server just needs to host the manifest - only the client needs to care how to patch.
  • UruPatcher would need some changes to generate the new manifest.
  • Uru itself would need to changes to the patcher to understand the new flag. A folder hasher would need to be added.
  • The flag would not be backwards compatible with existing clients. Fortunately the Mac client is a new client. The flag should only be used with the Mac client.

The dylib version of Python also adds at least a few hundred files to the Uru application bundle. This is because it stores each dylib for each of it's modules seperately. We should avoid the dynamic version of Python for Mac after this change because it will add a lot of overhead to patching.

Alternatives

Letting the patcher handle each file in the bundle as individual files was considered. However - the patcher is not designed to delete old files that are no longer used. This raises the prospect that an old file inside the bundle could be left behind invalidating code signing. It's also a less rich user experience to see each file inside the application bundle patch individually. To the user - the application bundle looks like a single file and Uru should treat it that way.

It might be possible to leave behind a UUID inside the Mac bundle instead of hashing each file. The patcher could compare this UUID to the hash in the manifest - and then judge when to patch.. However - this requires a build system to insert a correct and unique ID each build. This could be difficult if the Mac client was built outside of H'uru. It's different than how the Windows client works adding overhead. It would require more custom logic in UruManifest. And the overhead of hashing all the files within the Mac bundle should be fairly low. The Mac client is only 35 megabytes unzipped.

@dpogue
Copy link
Member

dpogue commented Aug 12, 2023

It might be possible to leave behind a UUID inside the Mac bundle instead of hashing each file.

This might be a terrible suggestion, but the _CodeSignature file is literally a cryptographic signature of the bundle contents and should always be present, right? If the contents change, then the signature should change... is it feasible to just use the hash of the _CodeSignature file?

@colincornaby
Copy link
Contributor Author

I considered it.

  • The location of that file is technically a private detail and could change in a future version of code signing so it might be risky to rely on it.
  • I’m not sure the code signature changes for child bundles that might have secondary signing? I’d have to look.
  • Adding folder based hashing is a little bit of work, but not much more than relying on the code signing file.
  • It would require code signing to be able to distribute a client. Technically everyone should be code signing things. But a more generic approach might be better.

@Hoikas
Copy link
Member

Hoikas commented Aug 12, 2023

So, if I'm reading correctly, we basically need to:

  • nuke any file in plClient.app that isn't listed in the macInternal manifest
  • update any files in plClient.app where the hash mismatches

I'm assuming that once we do this self-patch, the client needs to restart. Anyway, just to verify, the .app file is just a plain old zip that I can open with any archive application, right?

@dpogue
Copy link
Member

dpogue commented Aug 12, 2023

On the macOS disk, the .app file is a folder, not a file.
For distribution by the patching server, handling it as a zip file probably makes the most sense as far as downloading it all in one go and treating it like a single file.

@Hoikas
Copy link
Member

Hoikas commented Aug 12, 2023

So the manifesting tool can treat the plClient.app as a directory? I guess I'm a little unsure why we can't just ship plClient.app in one go - probably as a single file.

@colincornaby
Copy link
Contributor Author

A manifesting tool would need to implement a new hashing algorithm to hash a folder - but only for Mac .app bundles.

If the folder hash in the manifest doesn’t match the folder hash on disk, the client will nuke and replace itself.

@Hoikas
Copy link
Member

Hoikas commented Aug 12, 2023

Seems like this could be generalized to work for any entire directory. Not sure why you'd want to do that, but it'd help keep things platform agnostic.

@colincornaby
Copy link
Contributor Author

Seems like this could be generalized to work for any entire directory. Not sure why you'd want to do that, but it'd help keep things platform agnostic.

Definitely could be generic for directories. The backwards compatibility could still be a problem. Existing Windows launchers won't be able to do a folder hash. But - I guess the first thing the launcher does is patch itself so if Windows needed to use this in the future it would be ok.

@dgelessus
Copy link
Contributor

dgelessus commented Aug 12, 2023

Hmm, am I understanding correctly that the only missing feature is the ability to delete files? Seems to me like it would be simpler to implement that and then treat bundles as regular files in regular directories, rather than adding a new kind of "file" that's actually a directory and needs to be handled completely differently on the receiving end. (Also keep in mind that Plasma currently has no library for handling ZIP files or any other kind of archive, unless you count Python's zipfile module, so anything that requires client-side unpacking would need a new dependency or custom code for that.)

I suppose the question is how to represent "deleting files" in the manifest format with minimal/no changes on the server end.

The "simple stupid" solution would be to have an explicit entry for every file that needs to be deleted (either with a new flag bit, or just have the patcher interpret 0-byte files as "delete if present"). This would require no changes on the server side and would even degrade gracefully for old patchers (do we care about backwards compatibility outside the patcher manifest..?). The big disadvantage is that it's terrible for maintainability and robustness. Shard operators would have to list all old files forever, or risk some players ending up with old junk in their installation. It also doesn't help against unexpected files in bundles that the server doesn't know about, but would still break the code signature.

Another solution would be to have a manifest entry representing the directory itself, which serves as a marker to say "inside this directory, delete any files that are not listed in the manifest". This also requires no server changes, has none of the maintainability issues, and solves the code signature problem. The question is how to integrate this into the existing manifest format, because (AFAIK) there are no manifest entries for directories. I suppose we could use a special file name inside a directory to represent the directory itself (e. g. plClient.app/.delete_unknown). This also degrades gracefully for old patchers that don't understand the file's special meaning - they would just download a harmless junk file at that location.

@colincornaby
Copy link
Contributor Author

(Also keep in mind that Plasma currently has no library for handling ZIP files or any other kind of archive, unless you count Python's zipfile module, so anything that requires client-side unpacking would need a new dependency or custom code for that.)

I don't think this is true. The patcher as it exists today supports compressed files. plZlibSteam supports decompression of gzip for the patcher.

For a file deletion approach - this could get tricky as there are multiple manifests so automatic file deletion would have to take into account all manifests. A client could also skip several versions or more if the player has been away for a while. So deleted file records would have to be maintained forever. A tool like UruManifest would have a lot of trouble generating these deleted file records in since it only has access to the current data. It doesn't have a historical view of all data that has ever shipped.

@Hoikas
Copy link
Member

Hoikas commented Aug 12, 2023

A tool like UruManifest would have a lot of trouble generating these deleted file records in since it only has access to the current data. It doesn't have a historical view of all data that has ever shipped.

This isn't correct - UruManifest reads the current set of manifests to understand the current state of the game. So, at generation time, UruManifest can (and actually does know) if a file has been deleted. It does warn, however, that deleted files are not currently supported by any client.

The "simple stupid" solution would be to have an explicit entry for every file that needs to be deleted (either with a new flag bit, or just have the patcher interpret 0-byte files as "delete if present")

I tried this at one point, and the client was quite unhappy with PRPs being deleted out from under it, even if the PRP was unloaded first. 😞

If macOS will let us theoretically download individual files into the bundle, then I think this could the way to go. UruManifest is smart enough to be able to orchestrate things like this... or can be. I'm thinking each bundle directory should pretend to be an empty file in the manifest and be flagged as a bundle directory. We can then give UruManifest the smarts to put all files in the entire bundle directory tree in any manifest that depends on any one of those files. TBH, this should only be mac(Thin)Internal and mac(Thin)External. The downside is that the manifesting tool needs to be smart enough to do that, and, AFAIK, OpenUru is still insisting on building manifest files from hand using excel and a Python script, so it would be trivial to goof that up. But the Windows deployments are still regularly goofed up on that end, so I don't even know if that should be something we care about.

@colincornaby
Copy link
Contributor Author

If macOS will let us theoretically download individual files into the bundle, then I think this could the way to go. UruManifest is smart enough to be able to orchestrate things like this... or can be. I'm thinking each bundle directory should pretend to be an empty file in the manifest and be flagged as a bundle directory. We can then give UruManifest the smarts to put all files in the entire bundle directory tree in any manifest that depends on any one of those files.

Let me think about this. I'll give you my initial reaction of why I'm a little squeamish about this - but my knowledge might be out of date. Thinking about it a bit more I may actually have a more concrete reason why this wouldn't work.

Traditionally - Apple has written some files in a way that are only compatible with Apple file systems. So if you copy an application to a non-Apple file system it might become corrupt. Thats usually why it's best practice to compress an application before storing it on an unknown file system. So my concern would be the server could corrupt the bundle just by storing it openly.

I'm not completely sure that alone is true anymore. Apple did find a lot of users would just plug in Fat32 thumb drives and copy stuff over and any applications copied over would become corrupt. So I feel like they've been working on improvements. They also had to become involved with common compression algorithms to make sure the libraries could compress their data properly. So that may have changed.

What hasn't changed is (ugh) macOS bundles can and commonly do contain symlinks. We've already run into that. Github was mangling our Mac app bundles until we gave it the right flags to handle symlinks. So that would be a huge risk in storing the bundle directly on a server's file system.

There also is the risk that some process on the server might just touch the files and alter them in a way that invalidates code signing.

@dgelessus
Copy link
Contributor

I don't think this is true. The patcher as it exists today supports compressed files. plZlibSteam supports decompression of gzip for the patcher.

Ah, I should have worded that better. Yes, we indeed have zlib for de-/compression, but that only handles single files/data streams with no metadata. What we're missing is un-/archiving of multiple loose files and their metadata as a single archive file (e. g. zip or tar).

For a file deletion approach - this could get tricky as there are multiple manifests so automatic file deletion would have to take into account all manifests.

Yes, that's an issue if we go for the "delete any files not in the manifest" solution. In that case, you'd have to make sure that any manifest that declares such a directory also contains all files that belong in that directory. I suppose that makes it unsuitable for the thin manifests (unless the thin manifest will contain the entire app bundle?).

If macOS will let us theoretically download individual files into the bundle, then I think this could the way to go.

I don't see why it wouldn't work. At the file system level, bundles really are just directories with a specific structure, so you can freely manipulate individual files inside them.

Traditionally - Apple has written some files in a way that are only compatible with Apple file systems. [...] I'm not completely sure that alone is true anymore.

Yes, Apple has been moving away from that. In fact, I think the current notarization process will reject your app if any file has a resource fork or extended attributes (the usual suspects that get lost when copying through Mac-unaware systems), so they already expect modern macOS apps to not rely on such features. You're right that symlinks are still an issue though. The executable attribute also needs to be preserved (this will be relevant for the Linux client as well).

In any case, zipping the bundles is only part of the puzzle. The client still needs to unzip them in a way that preserves all attributes, which would require pulling in a full-featured archive handling library (libarchive, The Unarchiver, or 7-Zip probably?), or calling a macOS-specific API, or lots of custom code. Plus, if we use any kind of "folder hash", that also needs to take the special file metadata into account.

@dpogue
Copy link
Member

dpogue commented Aug 12, 2024

This is considered done, as of #1528, right?

@colincornaby
Copy link
Contributor Author

I would consider this done.

@dpogue dpogue closed this as completed Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants