Skip to content

[font] throw when loading empty font file #38229

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

Merged
merged 1 commit into from
Jul 22, 2025

Conversation

vonovak
Copy link
Contributor

@vonovak vonovak commented Jul 22, 2025

Why

This PR adds a check to prevent loading empty font files on Android. This normally wouldn't happen but if it did, Android TypeFace throws ArrayIndexOutOfBoundsException which is pretty useless for debugging.

How

Added a simple check in the FontLoaderModule.kt file that verifies if the font file has a non-zero length before attempting to create a Typeface from it. If the file is empty (length is 0), it throws a CodedException with a error message that's better than ArrayIndexOutOfBoundsException.

Test Plan

  • bare expo

Checklist

Copy link
Contributor Author

vonovak commented Jul 22, 2025

@expo-bot
Copy link
Collaborator

expo-bot commented Jul 22, 2025

The Pull Request introduced fingerprint changes against the base commit: acc2abd

Fingerprint diff
[
  {
    "op": "changed",
    "beforeSource": {
      "type": "dir",
      "filePath": "../../packages/expo-font/android",
      "reasons": [
        "expoAutolinkingAndroid"
      ],
      "hash": "c2e452a3401e195afc849d4c6d3ac6161a8f89d4"
    },
    "afterSource": {
      "type": "dir",
      "filePath": "../../packages/expo-font/android",
      "reasons": [
        "expoAutolinkingAndroid"
      ],
      "hash": "4a66a1b5b2ac878f4eaf482f021cfeb9ec9f36f3"
    }
  }
]

Generated by PR labeler 🤖

@vonovak vonovak force-pushed the vonovak/_font_throw_when_loading_empty_font_file branch 2 times, most recently from 54424ba to 01567ea Compare July 22, 2025 11:09
@vonovak vonovak requested a review from aleqsio July 22, 2025 11:11
@vonovak vonovak marked this pull request as ready for review July 22, 2025 11:11
Copy link
Contributor

Subscribed to pull request

File Patterns Mentions
packages/expo-asset/** @aleqsio, @wschurman
packages/expo-font/** @alanjhughes

Generated by CodeMention

Copy link
Contributor

@aleqsio aleqsio left a comment

Choose a reason for hiding this comment

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

looks good

@vonovak vonovak requested a review from aleqsio July 22, 2025 11:19
Copy link
Contributor Author

vonovak commented Jul 22, 2025

Merge activity

  • Jul 22, 11:52 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jul 22, 11:54 AM UTC: Graphite rebased this pull request as part of a merge.
  • Jul 22, 11:55 AM UTC: @vonovak merged this pull request with Graphite.

@vonovak vonovak changed the base branch from vonovak/_asset_warn_when_copied_asset_is_empty to graphite-base/38229 July 22, 2025 11:52
@vonovak vonovak changed the base branch from graphite-base/38229 to main July 22, 2025 11:53
@vonovak vonovak force-pushed the vonovak/_font_throw_when_loading_empty_font_file branch from 01567ea to 48b6844 Compare July 22, 2025 11:53
@expo-bot
Copy link
Collaborator

Hi there! 👋 I'm a bot whose goal is to ensure your contributions meet our guidelines.

I've found some issues in your pull request that should be addressed (click on them for more details) 👇

⚠️ Suggestion: Missing changelog entries


Your changes should be noted in the changelog, e.g.:
- throw when loading empty font file ([#38229](https://github.com/expo/expo/pull/38229) by [@vonovak](https://github.com/vonovak))
Read Updating Changelogs guide and consider adding an appropriate entry to the following changelogs:


Generated by ExpoBot 🤖 against 48b6844

@vonovak vonovak merged commit 56c336f into main Jul 22, 2025
10 of 27 checks passed
@vonovak vonovak deleted the vonovak/_font_throw_when_loading_empty_font_file branch July 22, 2025 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants