Skip to content

Commit

Permalink
Fix crash by conditional value of aspectRatio style value (#35858) (#…
Browse files Browse the repository at this point in the history
…35859)

Summary:
fix #35858

## Changelog

1. Handle not `number` | `string` value passed to `aspectRatio`
2. Add some tests

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[GENERAL] [FIXED] - Message

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

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
  • Loading branch information
mym0404 authored and kelset committed Jan 30, 2023
1 parent c4a995d commit 1f9926f
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 5 deletions.
@@ -1,7 +1,13 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`processAspectRatio should not accept invalid formats 1`] = `"aspectRatio must either be a number, a ratio or \`auto\`. You passed: 0a"`;
exports[`processAspectRatio should not accept invalid formats 1`] = `"aspectRatio must either be a number, a ratio string or \`auto\`. You passed: 0a"`;

exports[`processAspectRatio should not accept invalid formats 2`] = `"aspectRatio must either be a number, a ratio or \`auto\`. You passed: 1 / 1 1"`;
exports[`processAspectRatio should not accept invalid formats 2`] = `"aspectRatio must either be a number, a ratio string or \`auto\`. You passed: 1 / 1 1"`;

exports[`processAspectRatio should not accept invalid formats 3`] = `"aspectRatio must either be a number, a ratio or \`auto\`. You passed: auto 1/1"`;
exports[`processAspectRatio should not accept invalid formats 3`] = `"aspectRatio must either be a number, a ratio string or \`auto\`. You passed: auto 1/1"`;

exports[`processAspectRatio should not accept non string truthy types 1`] = `"aspectRatio must either be a number, a ratio string or \`auto\`. You passed: function () {}"`;

exports[`processAspectRatio should not accept non string truthy types 2`] = `"aspectRatio must either be a number, a ratio string or \`auto\`. You passed: 1,2,3"`;

exports[`processAspectRatio should not accept non string truthy types 3`] = `"aspectRatio must either be a number, a ratio string or \`auto\`. You passed: [object Object]"`;
14 changes: 14 additions & 0 deletions Libraries/StyleSheet/__tests__/processAspectRatio-test.js
Expand Up @@ -47,4 +47,18 @@ describe('processAspectRatio', () => {
expect(() => processAspectRatio('1 / 1 1')).toThrowErrorMatchingSnapshot();
expect(() => processAspectRatio('auto 1/1')).toThrowErrorMatchingSnapshot();
});

it('should ignore non string falsy types', () => {
const invalidThings = [undefined, null, false];
invalidThings.forEach(thing => {
expect(processAspectRatio(thing)).toBe(undefined);
});
});

it('should not accept non string truthy types', () => {
const invalidThings = [() => {}, [1, 2, 3], {}];
invalidThings.forEach(thing => {
expect(() => processAspectRatio(thing)).toThrowErrorMatchingSnapshot();
});
});
});
14 changes: 12 additions & 2 deletions Libraries/StyleSheet/processAspectRatio.js
Expand Up @@ -12,10 +12,20 @@

const invariant = require('invariant');

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

const matches = aspectRatio.split('/').map(s => s.trim());

Expand All @@ -34,7 +44,7 @@ function processAspectRatio(aspectRatio: number | string): ?number {
if (__DEV__) {
invariant(
!hasNonNumericValues && (matches.length === 1 || matches.length === 2),
'aspectRatio must either be a number, a ratio or `auto`. You passed: %s',
'aspectRatio must either be a number, a ratio string or `auto`. You passed: %s',
aspectRatio,
);
}
Expand Down

0 comments on commit 1f9926f

Please sign in to comment.