Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FlipperOkHttpInterceptor blocks multipart/form-data requests #993

Closed
aDavidaIsNoOne opened this issue Apr 8, 2020 · 24 comments
Closed

FlipperOkHttpInterceptor blocks multipart/form-data requests #993

aDavidaIsNoOne opened this issue Apr 8, 2020 · 24 comments

Comments

@aDavidaIsNoOne
Copy link

🐛 Bug Report

Network plugin is blocking multipart/form-data requests.

To Reproduce

See: facebook/react-native#28551

Environment

React Native 0.62.0
flipper-network-plugin 0.33.1

@passy
Copy link
Member

passy commented Apr 9, 2020

This appears to be happening only on Android based on the upstream report. Thanks for flagging!

@passy
Copy link
Member

passy commented Apr 9, 2020

@cekkaewnumchai Looking at the interceptor, I wonder if there's a case that could raise an exception we're not handling right now. Perhaps we be more defensive there in general and have a try/catch that calls out to chain.proceed(request) in case something trips up?

@cekkaewnumchai
Copy link
Contributor

I tried to repro directly from our sample app with similar request, but it seems this worked fine.

I will try to repro from React Native app directly.
Screenshot 2020-04-14 at 11 57 08

@omerman
Copy link

omerman commented Apr 17, 2020

@cekkaewnumchai
You are welcome to use my repro repo :)

https://github.com/omerman/React-native-android-upload-file-errors

facebook-github-bot pushed a commit that referenced this issue Apr 22, 2020
Summary:
Original Issue: #993

The exception occurs when OkHttp and we try to read request body twice, while it can be only read once.

Hence, we will read request body after it is processed by OkHttp.

Tradeoff for this is requests on Flipper will not appear immediately after fired, but they will appear together with their responses.

There are ways we can get rid of the tradeoff. For example, as demonstrated in D21167308, OkHttp ^3.14.0 contains method `isOneShot`, which can be used to check if we can read request body more than once. Another example is to change server side to accept nullable variable so that we can send request body and others separately.

Reviewed By: passy

Differential Revision: D21175341

fbshipit-source-id: 053789a2c2f28cd8149ea1bb36fd0cfe1c668df7
@cekkaewnumchai
Copy link
Contributor

cekkaewnumchai commented Apr 22, 2020

closed #993

The fix has landed in master

@aprilmintacpineda
Copy link

Has the fix been released? How do we upgrade?

@cekkaewnumchai
Copy link
Contributor

cekkaewnumchai commented Apr 27, 2020

This should be fixed in version 0.39.0. To upgrade, edit android > gradle.properties

27  # Version of flipper SDK to use with React Native
28  FLIPPER_VERSION=0.39.0  // edit this manually

@aprilmintacpineda
Copy link

Thanks so much for your kindness @cekkaewnumchai!

@jimberlage
Copy link

jimberlage commented Jun 10, 2020

This is still happening for me on RN 0.62.2, Flipper 0.46.0, on an ejected Expo app.

  // Turn the recording into a multipart form request with recording = the contents of our file.
  let fileUri = recording.getURI();
  if (!fileUri) {
    throw new Error('fileUri is null, it should always be initialized before reaching this point.');
  }

  let body = new FormData();
  body.append('recording', {
    uri: fileUri,
    name: 'voice-command.mp4',
    type: 'audio/mp4'
  });

  // Kick off the transcription job.
  var response;
  try {
    response = await fetch(
      'http://localhost:5000/api/v0/commands/transcription/start',
      {
        method: 'POST',
        headers: {
          'Accept': 'application/json',
        },
        body,
      },
    );
  } catch (error) {
    console.error(error);
    throw error;
  }

TypeError: Network request failed
http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false:32085:31
dispatchEvent@http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false:34645:31
setReadyState@http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false:33729:33
__didCompleteResponse@http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false:33556:29
emit@http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false:9744:42
__callFunction@http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false:5491:49
http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false:5213:31
__guard@http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false:5445:15
callFunctionReturnFlushedQueue@http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false:5212:21
callFunctionReturnFlushedQueue@[native code]

Info.plist is set up to allow localhost requests.

	<key>NSAppTransportSecurity</key>
	<dict>
		<key>NSAllowsArbitraryLoads</key>
		<true/>
		<key>NSExceptionDomains</key>
		<dict>
			<key>localhost</key>
			<dict>
				<key>NSExceptionAllowsInsecureHTTPLoads</key>
				<true/>
			</dict>
		</dict>
	</dict>

My package.json:

{
  "name": "EstimateMobileApp",
  "version": "0.0.1",
  "private": true,
  "scripts": {
    "start": "react-native start",
    "ios": "react-native run-ios",
    "android": "react-native run-android",
    "test": "jest"
  },
  "dependencies": {
    "@react-native-community/masked-view": "^0.1.10",
    "@react-navigation/native": "^5.4.2",
    "@react-navigation/stack": "^5.3.9",
    "@types/react-native-vector-icons": "^6.4.5",
    "expo": "^37.0.12",
    "expo-av": "^8.1.0",
    "react": "^16.11.0",
    "react-native": "^0.62.2",
    "react-native-gesture-handler": "^1.6.1",
    "react-native-linear-gradient": "^2.5.6",
    "react-native-reanimated": "^1.8.0",
    "react-native-safe-area-context": "^1.0.2",
    "react-native-screens": "^2.7.0",
    "react-native-unimodules": "^0.9.1",
    "react-native-vector-icons": "^6.6.0",
    "tailwind-rn": "^1.1.0"
  },
  "devDependencies": {
    "@babel/core": "^7.9.0",
    "@babel/runtime": "^7.9.0",
    "@react-native-community/eslint-config": "^0.0.5",
    "@types/jest": "^25.2.3",
    "@types/react": "^16.9.35",
    "@types/react-native": "^0.62.10",
    "@types/react-test-renderer": "^16.9.2",
    "babel-jest": "^24.9.0",
    "eslint": "^6.5.1",
    "jest": "^24.9.0",
    "metro-react-native-babel-preset": "^0.58.0",
    "react-test-renderer": "16.11.0",
    "tailwindcss": "^1.4.6",
    "typescript": "^3.9.3"
  },
  "jest": {
    "preset": "react-native",
    "transformIgnorePatterns": [
      "node_modules/(?!(jest-)?react-native|react-(native|universal|navigation)-(.*)|@react-native-community/(.*)|@react-navigation/(.*))"
    ]
  }
}

And my Podfile:

platform :ios, '10.0'
require_relative '../node_modules/@react-native-community/cli-platform-ios/native_modules'
require_relative '../node_modules/react-native-unimodules/cocoapods.rb'

def flipper_pods()
  flipperkit_version = '0.46.0'
  pod 'FlipperKit', '~>' + flipperkit_version, :configuration => 'Debug'
  pod 'FlipperKit/FlipperKitLayoutPlugin', '~>' + flipperkit_version, :configuration => 'Debug'
  pod 'FlipperKit/SKIOSNetworkPlugin', '~>' + flipperkit_version, :configuration => 'Debug'
  pod 'FlipperKit/FlipperKitUserDefaultsPlugin', '~>' + flipperkit_version, :configuration => 'Debug'
  pod 'FlipperKit/FlipperKitReactPlugin', '~>' + flipperkit_version, :configuration => 'Debug'
end

# Post Install processing for Flipper
def flipper_post_install(installer)
  file_name = Dir.glob("*.xcodeproj")[0]
  app_project = Xcodeproj::Project.open(file_name)
  app_project.native_targets.each do |target|
    target.build_configurations.each do |config|
      cflags = config.build_settings['OTHER_CFLAGS'] || '$(inherited) '
      unless cflags.include? '-DFB_SONARKIT_ENABLED=1'
        puts 'Adding -DFB_SONARKIT_ENABLED=1 in OTHER_CFLAGS...'
        cflags << '-DFB_SONARKIT_ENABLED=1'
      end
      config.build_settings['OTHER_CFLAGS'] = cflags
    end
    app_project.save
  end
  installer.pods_project.targets.each do |target|
    if target.name == 'YogaKit'
      target.build_configurations.each do |config|
        config.build_settings['SWIFT_VERSION'] = '4.1'
      end
    end
  end
  installer.pods_project.save
end

target 'EstimateMobileApp' do
  # Pods for EstimateMobileApp
  pod 'FBLazyVector', :path => "../node_modules/react-native/Libraries/FBLazyVector"
  pod 'FBReactNativeSpec', :path => "../node_modules/react-native/Libraries/FBReactNativeSpec"
  pod 'RCTRequired', :path => "../node_modules/react-native/Libraries/RCTRequired"
  pod 'RCTTypeSafety', :path => "../node_modules/react-native/Libraries/TypeSafety"
  pod 'React', :path => '../node_modules/react-native/'
  pod 'React-Core', :path => '../node_modules/react-native/'
  pod 'React-CoreModules', :path => '../node_modules/react-native/React/CoreModules'
  pod 'React-Core/DevSupport', :path => '../node_modules/react-native/'
  pod 'React-RCTActionSheet', :path => '../node_modules/react-native/Libraries/ActionSheetIOS'
  pod 'React-RCTAnimation', :path => '../node_modules/react-native/Libraries/NativeAnimation'
  pod 'React-RCTBlob', :path => '../node_modules/react-native/Libraries/Blob'
  pod 'React-RCTImage', :path => '../node_modules/react-native/Libraries/Image'
  pod 'React-RCTLinking', :path => '../node_modules/react-native/Libraries/LinkingIOS'
  pod 'React-RCTNetwork', :path => '../node_modules/react-native/Libraries/Network'
  pod 'React-RCTSettings', :path => '../node_modules/react-native/Libraries/Settings'
  pod 'React-RCTText', :path => '../node_modules/react-native/Libraries/Text'
  pod 'React-RCTVibration', :path => '../node_modules/react-native/Libraries/Vibration'
  pod 'React-Core/RCTWebSocket', :path => '../node_modules/react-native/'

  pod 'React-cxxreact', :path => '../node_modules/react-native/ReactCommon/cxxreact'
  pod 'React-jsi', :path => '../node_modules/react-native/ReactCommon/jsi'
  pod 'React-jsiexecutor', :path => '../node_modules/react-native/ReactCommon/jsiexecutor'
  pod 'React-jsinspector', :path => '../node_modules/react-native/ReactCommon/jsinspector'
  pod 'ReactCommon/callinvoker', :path => "../node_modules/react-native/ReactCommon"
  pod 'ReactCommon/turbomodule/core', :path => "../node_modules/react-native/ReactCommon"
  pod 'Yoga', :path => '../node_modules/react-native/ReactCommon/yoga', :modular_headers => true

  pod 'DoubleConversion', :podspec => '../node_modules/react-native/third-party-podspecs/DoubleConversion.podspec'
  pod 'glog', :podspec => '../node_modules/react-native/third-party-podspecs/glog.podspec'
  pod 'Folly', :podspec => '../node_modules/react-native/third-party-podspecs/Folly.podspec'

  target 'EstimateMobileAppTests' do
    inherit! :complete
    # Pods for testing
  end

  use_native_modules!
  use_unimodules!

  flipper_pods()
  post_install do |installer|
    flipper_post_install(installer)
  end
end

target 'EstimateMobileApp-tvOS' do
  # Pods for EstimateMobileApp-tvOS

  target 'EstimateMobileApp-tvOSTests' do
    inherit! :search_paths
    # Pods for testing
  end
end

@tezka003
Copy link

tezka003 commented Jul 2, 2020

This should be fixed in version 0.39.0. To upgrade, edit android > gradle.properties

27  # Version of flipper SDK to use with React Native
28  FLIPPER_VERSION=0.39.0  // edit this manually

Hi sir @cekkaewnumchai changing the FLIPPER_VERSION gives me a build failed output

@kuldeepworkid
Copy link

still facing network issue in
react-native='62.2'
FLIPPER_VERSION=0.39.0

@MahmonirB
Copy link

I had this problem and solve it via commenting the 43 line in android/src/debug/.../.../ReactNativeFlipper.java

// builder.addNetworkInterceptor(new FlipperOkhttpInterceptor(networkFlipperPlugin));
could you test it?

@siddhantbhatia
Copy link

This should be fixed in version 0.39.0. To upgrade, edit android > gradle.properties

27  # Version of flipper SDK to use with React Native
28  FLIPPER_VERSION=0.39.0  // edit this manually

Thank you! This worked for me

@mauricioblum
Copy link

This should be fixed in version 0.39.0. To upgrade, edit android > gradle.properties

27  # Version of flipper SDK to use with React Native
28  FLIPPER_VERSION=0.39.0  // edit this manually

Thank you! This worked for me

Worked for me too! Thanks!!

@JonatanOrtiz
Copy link

I tried it all, it didn't work.

@Baterka
Copy link

Baterka commented Jul 27, 2020

Not work for me as well:
FLIPPER_VERSION=0.39.0
"react": "16.13.1",
"react-native": "0.63.2",
"react-native-flipper": "^0.39.0",

@SittisukChartrasee
Copy link

This should be fixed in version 0.39.0. To upgrade, edit android > gradle.properties

27  # Version of flipper SDK to use with React Native
28  FLIPPER_VERSION=0.39.0  // edit this manually

Thank you! This worked for me

What your use react native version?

@mweststrate
Copy link
Contributor

Closing as the fix has landed

@joelpiccoli
Copy link

This should be fixed in version 0.39.0. To upgrade, edit android > gradle.properties

27  # Version of flipper SDK to use with React Native
28  FLIPPER_VERSION=0.39.0  // edit this manually

You are a life saver man! Thank you so much! s2 s2 s2 s2

@FaycalBorsali
Copy link

This should be fixed in version 0.39.0. To upgrade, edit android > gradle.properties

27  # Version of flipper SDK to use with React Native
28  FLIPPER_VERSION=0.39.0  // edit this manually

Thank you you are a life saver !

PS: if anyone still has a problem, I had to clean the android project cd android && ./gradlew clean && cd .. for the change to take effect

@surajl11
Copy link

did it ,its not because of the flipper version ,I'm using react -native 0.62.2 and flipper version :0.33.1,commented out line 43 in android/app/src/debug/java/com/maxyride/app/drivers/ReactNativeFlipper.java and made sure the type in upload is specified properly,

chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Oct 21, 2020
See discussion [1], where Greg reported that 7c72715 introduced an
issue where uploading images from the camera was broken on Android.

The issue is facebook/react-native#28551; it seems to be a problem
specifically with Flipper's "network" plugin [2].

The discussion on that issue links to facebook/flipper#993, which
suggests [3] upgrading Flipper to 0.39.0. So, do. On Android, it may
be necessary to do a `cd android && ./gradlew clean` after this
commit.

It's nice that we can upgrade Flipper independently of React Native;
it seems this will stay possible [4].

[1] https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/Flipper/near/1042657
[2] facebook/react-native#28551 (comment)
[3] facebook/flipper#993 (comment)
[4] https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/Flipper/near/1042764
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Oct 21, 2020
See discussion [1], where Greg reported that 7c72715 introduced an
issue where uploading images from the camera was broken on Android.

The issue is facebook/react-native#28551; it seems to be a problem
specifically with Flipper's "network" plugin [2].

The discussion on that issue links to facebook/flipper#993, which
suggests [3] upgrading Flipper to 0.39.0. So, do. On Android, it may
be necessary to do a `cd android && ./gradlew clean` after this
commit.

It's nice that we can upgrade Flipper independently of React Native;
it seems this will stay possible [4].

[1] https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/Flipper/near/1042657
[2] facebook/react-native#28551 (comment)
[3] facebook/flipper#993 (comment)
[4] https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/Flipper/near/1042764
gnprice pushed a commit to chrisbobbe/zulip-mobile that referenced this issue Oct 21, 2020
See discussion [1] and zulip#4281, where Greg reported that 7c72715
introduced an issue where uploading images from the camera was broken
on Android.

The issue is facebook/react-native#28551; it seems to be a problem
specifically with Flipper's "network" plugin [2].

The discussion on that issue links to facebook/flipper#993, which
suggests [3] upgrading Flipper to 0.39.0. So, do. On Android, it may
be necessary to do a `cd android && ./gradlew clean` after this
commit.

It's nice that we can upgrade Flipper independently of React Native;
it seems this will stay possible [4].

[1] https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/Flipper/near/1042657
[2] facebook/react-native#28551 (comment)
[3] facebook/flipper#993 (comment)
[4] https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/Flipper/near/1042764

Fixes: zulip#4281
@RupamShaw
Copy link

I resolved network issue in android in fomdata append uri with file://

formData.append(`video${idx}`, {
            name: img.name,
            type: `video/${ext}`,
            uri: `file://${img.sourceURL}`,
          })

@Aurangempire
Copy link

STILL FACING THE SAME ISSUE :(((

Version of flipper SDK to use with React Native

FLIPPER_VERSION=0.54.0

@mweststrate
Copy link
Contributor

@Aurangempire this issue is closed and the current Flipper version is 0.80. So please file fresh issues on recent versions and follow the template by providing a minimal reproduction so that your issue becomes actionable. We won't be landing fixes on a year old Flipper version and using all caps won't change that.

@facebook facebook locked and limited conversation to collaborators Mar 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests