Skip to content

feat: Patch rnfs to handle 303 http code on download - #638

Merged
doubleface merged 1 commit into
masterfrom
feat/rnfs303patch
Mar 6, 2023
Merged

feat: Patch rnfs to handle 303 http code on download#638
doubleface merged 1 commit into
masterfrom
feat/rnfs303patch

Conversation

@doubleface

Copy link
Copy Markdown
Contributor

This patch will handle 303 response on download when using the local stack with a http url and not https.

An issue has been created on the react-native-fs : itinance/react-native-fs#1153

Checklist

Before merging this PR, the following things must have been done:

  • Faithful integration of the mockups at all screen sizes
  • Tested on iOS
  • Tested on Android
  • Localized in English and French
  • All changes have test coverage
  • Updated README & CHANGELOG, if necessary

@Ldoppea Ldoppea left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your patch contains way too many lines, it should contains only something like:

--- a/node_modules/react-native-fs/android/src/main/java/com/rnfs/Downloader.java
+++ b/node_modules/react-native-fs/android/src/main/java/com/rnfs/Downloader.java
@@ -67,6 +67,7 @@ public class Downloader extends AsyncTask<DownloadParams, long[], DownloadResult
         (
           statusCode == HttpURLConnection.HTTP_MOVED_PERM ||
           statusCode == HttpURLConnection.HTTP_MOVED_TEMP ||
+          statusCode == HttpURLConnection.HTTP_SEE_OTHER ||
           statusCode == 307 ||
           statusCode == 308
         )

It seems like your node_modules were not up-to-date when you run the patch command.

If you do a yarn install and run the patch command again it should generate the correct diff file.

@doubleface

Copy link
Copy Markdown
Contributor Author

@Ldoppea I generated it with patch-package. Now I modified it manually to only let what is wanted

@doubleface
doubleface requested a review from Ldoppea March 6, 2023 08:43
@doubleface

Copy link
Copy Markdown
Contributor Author

@Ldoppea Now the current patch is generated after a

rm -rf node_modules && yarn install

And this output the correct patch

@Ldoppea

Ldoppea commented Mar 6, 2023

Copy link
Copy Markdown
Contributor

Nice :)

@doubleface
doubleface merged commit 4387cc2 into master Mar 6, 2023
@doubleface
doubleface deleted the feat/rnfs303patch branch March 6, 2023 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants