Skip to content
This repository has been archived by the owner on Jun 1, 2023. It is now read-only.

Do not replace_asset_urls in font bodies #2721

Merged
merged 2 commits into from
Jan 30, 2023
Merged

Do not replace_asset_urls in font bodies #2721

merged 2 commits into from
Jan 30, 2023

Conversation

Poitrin
Copy link
Contributor

@Poitrin Poitrin commented Jan 26, 2023

WHY are these changes introduced?

Fixes Shopify/cli#1199

Starting with Ruby 3.2 – Homebrew formula just says depends_on "ruby", so that’s why such kind of issues appear now – body.join.gsub raises the error invalid byte sequence in UTF-8 when body.join is a font body (which of course doesn’t contain any URLs or other readable characters).

WHAT is this pull request doing?

  1. Does not call replace_asset_urls when path starts with /fonts.
  2. In case other "non-gsubable" strings are being gsubed and the same ArgumentError containing the message invalid byte sequence is raised, body will simply be returned without any replacements.

How to test your changes?

Test it with both Ruby 3.1 and 3.2:

  1. cd into a theme directory
  2. Call DEBUG=true shopify theme serve
  3. Open http://127.0.0.1:9292/
  4. Verify that no Internal server error is thrown

Post-release steps

  • CLI 3.x update

Update checklist

  • I've added a CHANGELOG entry for this PR (if the change is public-facing)
  • I've considered possible cross-platform impacts (Mac, Linux, Windows).
  • I've left the version number as is (we'll handle incrementing this when releasing).
  • I've included any post-release steps in the section above (if needed).

@Poitrin Poitrin requested a review from a team January 26, 2023 10:08
@Poitrin Poitrin requested review from a team, Arkham and pepicrft and removed request for a team January 26, 2023 10:09
Copy link
Contributor

@karreiro karreiro left a comment

Choose a reason for hiding this comment

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

Thank you, @Poitrin!

@Poitrin Poitrin merged commit b665243 into main Jan 30, 2023
@Poitrin Poitrin deleted the fix-cli-1199 branch January 30, 2023 08:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Font files 500 (Internal Server Error)
2 participants