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

fix(storage): Fixing watchOS crash when dealing with big files #3389

Merged
merged 1 commit into from
Dec 4, 2023

Conversation

ruisebas
Copy link
Member

Issue #

Description

This PR changes Int to UInt64 in several Storage objects when representing bytes.
watchOS's Int is actually a Int32 due to its arm64_32 architecture, so attempting to convert >= 2GB into bytes results in an overflown error.

The only special handing is our usage of FileHandle.read(upToCount:), which we use to split a file into parts for multipart uploads.
Since the count attribute is an Int, but the theoretical maximum size for a part is 5GB, attempting to directly cast it could crash. In practice, this very unlikely to happen as having a part with size >= 2GB with the current implementation can only happen for files of size bigger than 20TB.

We can argue that by the time an Apple Watch can handle 20TB files, then the arm64_32 architecture would likely be long gone. But anyways, just to be extra cautious, I've prevented this situation by implementing a simple check that will "split" reads that exceed Int.max bytes.

General Checklist

  • Added new tests to cover change, if needed
  • Build succeeds with all target using Swift Package Manager
  • All unit tests pass
  • All integration tests pass
  • Security oriented best practices and standards are followed (e.g. using input sanitization, principle of least privilege, etc)
  • Documentation update for the change if required
  • PR title conforms to conventional commit style
  • New or updated tests include Given When Then inline code documentation and are named accordingly testThing_condition_expectation()
  • If breaking change, documentation/changelog update with migration instructions

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@ruisebas ruisebas requested a review from a team as a code owner November 29, 2023 20:18
@ruisebas ruisebas merged commit 71c600d into main Dec 4, 2023
79 checks passed
@ruisebas ruisebas deleted the fix/watchos_storage branch December 4, 2023 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants