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.

Sign up for GitHub

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

fix mac os sandbox check slowness #3879

Merged
merged 4 commits into from
Aug 23, 2024
Merged

fix mac os sandbox check slowness #3879

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
31a8ba4
updated the checks for the receipt signature so that they only run if…
aboedo May 8, 2024
9983c68
added extra consideration for unknown environment
aboedo Aug 22, 2024
078a16d
added tests
aboedo Aug 22, 2024
5908a45
updated tests
aboedo Aug 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions Sources/Misc/SandboxEnvironmentDetector.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,14 @@ final class BundleSandboxEnvironmentDetector: SandboxEnvironmentDetector {
}

#if os(macOS) || targetEnvironment(macCatalyst)
return !self.isProductionReceipt || !self.isMacAppStore
// this relies on an undocumented field in the receipt that provides the Environment.
// if it's not present, we go to a secondary check.
if let isProductionReceipt = self.isProductionReceipt {
return !isProductionReceipt
} else {
return !self.isMacAppStore
}

#else
return path.contains("sandboxReceipt")
#endif
Expand All @@ -73,12 +80,12 @@ extension BundleSandboxEnvironmentDetector: Sendable {}

private extension BundleSandboxEnvironmentDetector {

var isProductionReceipt: Bool {
var isProductionReceipt: Bool? {
do {
return try self.receiptFetcher.fetchAndParseLocalReceipt().environment == .production
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also check if the environment is .unknown? Seems that the catch will happen with any parsing error, not specifically about the environment so I wonder if we will be doing extra operations in those cases... Having said that, parsing errors should be fairly uncommon, so I think it should be ok?

Copy link
Member Author

Choose a reason for hiding this comment

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

Only 3 months late to respond to the comment 😅
Good call, I'll add an extra check for that

Copy link
Member Author

Choose a reason for hiding this comment

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

can't imagine it would happen often that we can correctly parse the receipt and it's production but it doesn't have the environment variable, but I can't discard it either since it's an undocumented field

} catch {
Logger.error(Strings.receipt.parse_receipt_locally_error(error: error))
return false
return nil
}
}

Expand Down