React-native-web: TouchHistory‘s processing logic caused the Crash in Safari

Created on 19 Jun 2020 · 9Comments · Source: necolas/react-native-web

The problem

In "react-native-web/dist/hooks/useResponderEvents/ResponderTouchHistoryStore.js" , use touchBank tracks the position.
But touchBank's type is Array and touch identifier’s not a number between 0 and 20, It can be very big sometimes，examples: 863683572. It can cause Crash in Safari.

How to reproduce

Simplified test case:
Examples Code:

var arr =[]; arr[863683572]=109; console.log(arr.length);console.log(arr);

You can run it multiple times in Safari to reproduce this problem.

Expected behavior

I think you can change touchBank's type to Object store history of touch position in ResponderTouchHistoryStore.js.

Environment (include versions). Did this work in previous versions?

React Native for Web (version): The latest next branch in the code repository
React (version): 16.8.6
Browser: Safari

Source

relign

👍1

Most helpful comment

This is patched in master

I actually don't see where this patch went 😬 Is anyone still experiencing this issue with 0.13.x?

necolas on 24 Aug 2020

❤1 🎉1 👍1

All 9 comments

@necolas
The same code doesn't cause crash in Chrome, I guess V8 has a few tweaks to the Array.

relign on 19 Jun 2020

I don't understand what you're trying to explain

necolas on 20 Jun 2020

@necolas
Demo Code:

// app.tsx
import React from 'react'
import { View } from 'react-native'

// Fork this sandbox and add example code
// to reproduce the issue.
//
// Please check the versions of dependencies
// in package.json

class App extends React.Component {
    render() {
        return <View style={{ backgroundColor: 'blue', height: 500 }} />
    }
}

export default App

Then, I'll simulate the gesture of double refers to zoom on the mobile Page of React-Native and Web, respectively, to trigger the logic of the touchBank collecting position information.

Use React Native, insert debug log:

// node_modules/react-native/Libraries/Renderer/oss/ReactNativeRenderer-dev.js
function getTouchIdentifier(_ref) {
  var identifier = _ref.identifier;
  console.warn('[Demo] identifier', identifier);
  ...
  return identifier;
}

In iOS React Native App:

In Android React Native App:

Use React Native Web, insert debug log:

// node_modules/react-native-web/dist/hooks/useResponderEvents/ResponderTouchHistoryStore.js
function getTouchIdentifier(_ref) {
  var identifier = _ref.identifier;
  console.warn('[Demo] identifier', identifier);
  ...
  return identifier;
}

In Android Web Page:

But In iOS Web Page:

The identifier value here will be very large In iOS Web Page, If sometimes more touches are triggered, the code about TouchBank will run very slowly or even Crash

ಥ_ಥ... I think touckBank should be stored as Object in Web environment.

relign on 22 Jun 2020

👍3

I see what you're saying, thanks.

It's unfortunate that the touchBank array is part of the RN responder event API, because adding touches to an array like that is a bad code pattern that can create enormous arrays. A Map would be better in this case.

And it's bizarre that safari is still using huge identifiers too. If this bug is not occurring in 0.12 it may be because of this line of code https://github.com/necolas/react-native-web/blob/36dacb2052efdab2a28655773dc76934157d9134/packages/react-native-web/src/modules/normalizeNativeEvent/index.js#L22

necolas on 22 Jun 2020

@necolas
Will we be compatible with this issue in latest next branch?

relign on 23 Jun 2020

This is patched in master

necolas on 26 Jun 2020