Fix crash by conditional value of aspectRatio style value (#35858) by mym0404 · Pull Request #35859 · react/react-native

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

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

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(typeofaspectRatio!=='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

 });
+
+ 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.

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

Fix crash by conditional value of aspectRatio style value (react#35858)
66278fc

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

Comment threadLibraries/StyleSheet/processAspectRatio.js

gabrieldonadel reviewed Jan 18, 2023
View reviewed changes

Comment threadLibraries/StyleSheet/processAspectRatio.js

mym0404and 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

Comment threadLibraries/StyleSheet/__tests__/processAspectRatio-test.js Outdated

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

NickGerleman reviewed Jan 20, 2023
View reviewed changes

Comment threadLibraries/StyleSheet/__tests__/processAspectRatio-test.js Outdated

fix typo
62b764a

facebook-github-bot closed this in a8166bdJan 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

coderabbitaiBot mentioned this pull request Jul 29, 2026
Rendering Improvements + Bugfixes jchu634/EdGo#24
Merged

Uh oh!

Conversation

mym0404 commented Jan 17, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changelog

Test Plan

Sample

Uh oh!

facebook-github-bot commented Jan 17, 2023

Action Required

Process

Uh oh!

mym0404 commented Jan 17, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Jan 17, 2023

Uh oh!

facebook-github-bot commented Jan 17, 2023

Uh oh!

facebook-github-bot commented Jan 18, 2023

Uh oh!

NickGerleman commented Jan 18, 2023

Uh oh!

gabrieldonadel commented Jan 18, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mym0404 commented Jan 18, 2023

Uh oh!

gabrieldonadel left a comment

Choose a reason for hiding this comment

Uh oh!

necolasJan 18, 2023

Choose a reason for hiding this comment

Uh oh!

mym0404Jan 18, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gabrieldonadelJan 19, 2023

Choose a reason for hiding this comment

Uh oh!

mym0404Jan 20, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Jan 19, 2023

Uh oh!

Uh oh!

Uh oh!

facebook-github-bot commented Jan 20, 2023

Uh oh!

analysis-bot commented Jan 20, 2023

Uh oh!

facebook-github-bot commented Jan 26, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mym0404 commented Jan 17, 2023•
edited
Loading

mym0404 commented Jan 17, 2023•
edited
Loading

gabrieldonadel commented Jan 18, 2023•
edited
Loading

mym0404Jan 18, 2023•
edited
Loading

mym0404Jan 20, 2023•
edited
Loading