Fix crash by conditional value of aspectRatio style value (#35858) #35859

mym0404 · 2023-01-17T16:11:21Z

Summary

fix #35858

Changelog

Handle not number | string value passed to aspectRatio
Add some tests

Test Plan

Sample

Sample Repository

Video

…5858)

facebook-github-bot · 2023-01-17T16:11:26Z

Hi @mym0404!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

mym0404 · 2023-01-17T16:14:37Z

I have signed CLA now.

facebook-github-bot · 2023-01-17T18:09:37Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

facebook-github-bot · 2023-01-17T18:09:40Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

facebook-github-bot · 2023-01-18T14:10:30Z

@NickGerleman has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

NickGerleman · 2023-01-18T14:15:47Z

FYI @gabrieldonadel

gabrieldonadel · 2023-01-18T14:21:00Z

@NickGerleman but do we want this to silently fail without warning the user that they've inputted an invalid aspectRatio? I remember discussing this with @necolas a while ago

Libraries/StyleSheet/processAspectRatio.js

Co-authored-by: Gabriel Donadel Dall'Agnol <donadeldev@gmail.com>

mym0404 · 2023-01-18T15:44:10Z

@gabrieldonadel I handled non-string truthy values like this and add more tests.

  if (typeof aspectRatio !== 'string') {
    if (__DEV__) {
      invariant(
        !aspectRatio,
        'aspectRatio must either be a number, a ratio string or `auto`. You passed: %s',
        aspectRatio,
      );
    }
    return;
  }

gabrieldonadel

LGTM

necolas · 2023-01-18T18:07:50Z

Libraries/StyleSheet/__tests__/processAspectRatio-test.js

@@ -47,4 +47,18 @@ describe('processAspectRatio', () => {
    expect(() => processAspectRatio('1 / 1 1')).toThrowErrorMatchingSnapshot();
    expect(() => processAspectRatio('auto 1/1')).toThrowErrorMatchingSnapshot();
  });
+
+  it('should not accept non string falsy types', () => {
+    const invalidThings = [undefined, null, false];


Isn't null an allowed value for all properties?

It is allowed and ignored silently in this code.

do you mean typing also should be changed to value?: number? | string??

Perhaps we could rename this test to it('should ignone non string falsy types', () => {

Done name is changed.

facebook-github-bot · 2023-01-19T12:41:48Z

@NickGerleman has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Libraries/StyleSheet/__tests__/processAspectRatio-test.js

facebook-github-bot · 2023-01-20T23:23:14Z

@NickGerleman has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

analysis-bot · 2023-01-20T23:46:44Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,465,307	+3,438
android	hermes	armeabi-v7a	7,786,077	+3,615
android	hermes	x86	8,938,970	+3,851
android	hermes	x86_64	8,796,812	+3,513
android	jsc	arm64-v8a	9,650,659	+2,571
android	jsc	armeabi-v7a	8,385,140	+2,547
android	jsc	x86	9,712,860	+2,536
android	jsc	x86_64	10,190,037	+2,591

Base commit: 8fbcb5c
Branch: main

facebook-github-bot · 2023-01-26T01:19:32Z

@NickGerleman merged this pull request in a8166bd.

…35859) Summary: fix #35858 ## Changelog 1. Handle not `number` | `string` value passed to `aspectRatio` 2. Add some tests  Pull Request resolved: #35859 Test Plan: ## Sample [Sample Repository](https://github.com/mym0404/rn-aspect-ratio-crash-sample) Video ![1](https://user-images.githubusercontent.com/33388801/212956921-94b21cda-d841-4588-a05a-d604a82e204c.gif) Reviewed By: necolas Differential Revision: D42575942 Pulled By: NickGerleman fbshipit-source-id: 2f7f46e6e3af85146e4042057477cb6d63b3b279

…5858) (facebook#35859) Summary: fix facebook#35858 ## Changelog 1. Handle not `number` | `string` value passed to `aspectRatio` 2. Add some tests  Pull Request resolved: facebook#35859 Test Plan: ## Sample [Sample Repository](https://github.com/mym0404/rn-aspect-ratio-crash-sample) Video ![1](https://user-images.githubusercontent.com/33388801/212956921-94b21cda-d841-4588-a05a-d604a82e204c.gif) Reviewed By: necolas Differential Revision: D42575942 Pulled By: NickGerleman fbshipit-source-id: 2f7f46e6e3af85146e4042057477cb6d63b3b279

Fix crash by conditional value of aspectRatio style value (facebook#3…

66278fc

…5858)

mym0404 closed this Jan 17, 2023

mym0404 reopened this Jan 17, 2023

Add more processAspectRatio tests

ed5a0de

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 17, 2023

NickGerleman approved these changes Jan 18, 2023

View reviewed changes

gabrieldonadel reviewed Jan 18, 2023

View reviewed changes

Libraries/StyleSheet/processAspectRatio.js Outdated Show resolved Hide resolved

gabrieldonadel reviewed Jan 18, 2023

View reviewed changes

Libraries/StyleSheet/processAspectRatio.js Show resolved Hide resolved

mym0404 and others added 2 commits January 19, 2023 00:35

Filter out only not string and falsy value in processAspectRatio

1839593

Co-authored-by: Gabriel Donadel Dall'Agnol <donadeldev@gmail.com>

Handle not string truthy values of aspectRatio and Add more tests

b855ea4

gabrieldonadel reviewed Jan 18, 2023

View reviewed changes

necolas reviewed Jan 18, 2023

View reviewed changes

mym0404 commented Jan 20, 2023

View reviewed changes

Libraries/StyleSheet/__tests__/processAspectRatio-test.js Outdated Show resolved Hide resolved

Update Libraries/StyleSheet/__tests__/processAspectRatio-test.js

07fc573

NickGerleman reviewed Jan 20, 2023

View reviewed changes

Libraries/StyleSheet/__tests__/processAspectRatio-test.js Outdated Show resolved Hide resolved

fix typo

62b764a

facebook-github-bot closed this in a8166bd Jan 26, 2023

facebook-github-bot added the Merged This PR has been merged. label Jan 26, 2023

snyk-bot mentioned this pull request Feb 23, 2023

[Snyk] Upgrade react-native from 0.63.3 to 0.71.2 NOUIY/react-native-macos#987

Open

snyk-bot mentioned this pull request Mar 8, 2023

[Snyk] Upgrade react-native from 0.63.3 to 0.71.3 NOUIY/react-native-macos#1002

Open

cipolleschi mentioned this pull request Aug 16, 2023

cipolleschi/add maven polling #39039

Merged

cipolleschi mentioned this pull request Sep 21, 2023

[LOCAL] Copy hermes ruby files to hermes folder in local E2E test #39577

Merged

cipolleschi mentioned this pull request Oct 11, 2023

cipolleschi/fix CI #40793

Merged

cipolleschi mentioned this pull request Nov 1, 2023

[RN][CI]Update node installation on debian (0.71) #41277

Merged

This was referenced Jan 8, 2024

[0.71][RN][General] Migrate boost download url away from JFrog #42177

Merged

[RN][iOS] Fix symbol not found _jump_fcontext with use_frameworks! #42229

Merged

cipolleschi mentioned this pull request Mar 6, 2024

[RN][iOS][0.71] Fix flipper for Xcode 15.3 #43345

Merged

cipolleschi mentioned this pull request Apr 5, 2024

cipolleschi/fix yoga archive 0.71 #43895

Closed

cipolleschi mentioned this pull request Apr 26, 2024

[iOS][Privacy Manifest] Set up privacy manifest for React Native 0.71 #44281

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix crash by conditional value of aspectRatio style value (#35858) #35859

Fix crash by conditional value of aspectRatio style value (#35858) #35859

mym0404 commented Jan 17, 2023 •

edited

facebook-github-bot commented Jan 17, 2023

mym0404 commented Jan 17, 2023 •

edited

facebook-github-bot commented Jan 17, 2023

facebook-github-bot commented Jan 17, 2023

facebook-github-bot commented Jan 18, 2023

NickGerleman commented Jan 18, 2023

gabrieldonadel commented Jan 18, 2023 •

edited

mym0404 commented Jan 18, 2023

gabrieldonadel left a comment

necolas Jan 18, 2023

mym0404 Jan 18, 2023 •

edited

gabrieldonadel Jan 19, 2023

mym0404 Jan 20, 2023 •

edited

facebook-github-bot commented Jan 19, 2023

facebook-github-bot commented Jan 20, 2023

analysis-bot commented Jan 20, 2023

facebook-github-bot commented Jan 26, 2023

Fix crash by conditional value of aspectRatio style value (#35858) #35859

Fix crash by conditional value of aspectRatio style value (#35858) #35859

Conversation

mym0404 commented Jan 17, 2023 • edited

Summary

Changelog

Test Plan

Sample

facebook-github-bot commented Jan 17, 2023

Action Required

Process

mym0404 commented Jan 17, 2023 • edited

facebook-github-bot commented Jan 17, 2023

facebook-github-bot commented Jan 17, 2023

facebook-github-bot commented Jan 18, 2023

NickGerleman commented Jan 18, 2023

gabrieldonadel commented Jan 18, 2023 • edited

mym0404 commented Jan 18, 2023

gabrieldonadel left a comment

Choose a reason for hiding this comment

necolas Jan 18, 2023

Choose a reason for hiding this comment

mym0404 Jan 18, 2023 • edited

Choose a reason for hiding this comment

gabrieldonadel Jan 19, 2023

Choose a reason for hiding this comment

mym0404 Jan 20, 2023 • edited

Choose a reason for hiding this comment

facebook-github-bot commented Jan 19, 2023

facebook-github-bot commented Jan 20, 2023

analysis-bot commented Jan 20, 2023

facebook-github-bot commented Jan 26, 2023

mym0404 commented Jan 17, 2023 •

edited

mym0404 commented Jan 17, 2023 •

edited

gabrieldonadel commented Jan 18, 2023 •

edited

mym0404 Jan 18, 2023 •

edited

mym0404 Jan 20, 2023 •

edited