fix: File upload fails when uploading base64 data #1578

musthafa1996 · 2022-10-21T18:24:55Z

New Pull Request Checklist

I am not disclosing a vulnerability.
I am creating this PR in reference to an issue.

Issue Description

Parse server throws the following error while uploading files:
The first argument must be of type string or an instance of Buffer, ArrayBuffer, or Array or an Array-like Object. Received undefined

Related issue: #1579, parse-community/parse-server#8253
Closes #1579, parse-community/parse-server#8253

Approach

Add in the missing assignment of _data property with the uploaded file data in ParseFile.js

TODOs before merging

Add tests
Remove overriding of Parse server in package.json
Rollback the content of package-lock.json
Add entry to changelog
Add changes to documentation (guides, repository pages, in-code descriptions)x

parse-github-assistant · 2022-10-21T18:24:58Z

Thanks for opening this pull request!

🎉 We are excited about your hands-on contribution!

parse-github-assistant · 2022-10-26T14:36:30Z

I will reformat the title to use the proper commit message syntax.

musthafa1996 · 2022-10-27T08:38:24Z

Hi @mtrezza! All the test cases are now passing. File uploading works now.

The overriding of Parse SDK using the git repo git@github.com:musthafa1996/Parse-SDK-JS.git#file-upload-issue for Parse server was necessary to make sure Parse Server uses the Parse SDK with the file upload fix.

Once approved we’ll need to do the following:

Remove overriding of Parse SDK for Parse Server
Rollback the content of package-lock.json
Make sure Parse Server uses the Parse SDK with file upload fix.

integration/test/ParseFileTest.js

musthafa1996 · 2022-10-31T13:55:55Z

I have also replaced the Parse SDK overriding repo ssh URL to https repo URL to avoid build issues in CI.

mtrezza · 2022-10-31T14:28:48Z

Great, could you solve the conflict so we can see whether the CI passes? I think the next step then would be to revert the changes in package and package-lock and commit the fix, right?

musthafa1996 · 2022-10-31T14:35:15Z

I've resolved the conflicts @mtrezza.
Yes, the next step would be to revert the changes in package and package-lock.

musthafa1996 · 2022-10-31T14:46:43Z

@mtrezza, did you just run the CI. Looks like the old SSH repo URL (git@github.com:musthafa1996/Parse-SDK-JS.git#file-upload-issue) was being used instead of HTTPS repo URL (git+https://github.com/musthafa1996/Parse-SDK-JS.git#file-upload-issue). Is there a way we can ensue the latter is used?

mtrezza · 2022-10-31T16:19:38Z

Is the change necessary for this PR? If not, let's keep it as is and revert any changes made in package and package-lock.

musthafa1996 · 2022-10-31T16:21:51Z

@mtrezza, did you just run the CI. Looks like the old SSH repo URL (git@github.com:musthafa1996/Parse-SDK-JS.git#file-upload-issue) was being used instead of HTTPS repo URL (git+https://github.com/musthafa1996/Parse-SDK-JS.git#file-upload-issue). Is there a way we can ensue the latter is used?

This change is necessary. Otherwise you'll keep getting Permission denied error while accessing the git repo.

mtrezza · 2022-10-31T16:22:30Z

Can you try reverting the package and package-lock please

mtrezza · 2022-10-31T16:23:47Z

Also, I noticed that there is no test in the JS SDK for this change. Is it possible to add a test that fails without the change?

musthafa1996 · 2022-10-31T16:23:48Z

You mean reverting the overrides for Parse SDK too?

mtrezza · 2022-10-31T16:25:12Z

Yes, the only file that this PR should change is src/ParseFile.js. And possibly a test file if we add a test.

musthafa1996 · 2022-10-31T16:27:16Z

Also, I noticed that there is no test in the JS SDK for this change. Is it possible to add a test that fails without the change?

The failing test case wouldn't be possible unless you force Parse server to use the latest alpha version of the Parse SDK. The issue was induce in the recent updates to Parse SDK.

mtrezza

If this was not a lint change, we should also keep this line as is

src/ParseFile.js

codecov · 2022-11-03T08:59:45Z

Codecov Report

Base: 99.93% // Head: 99.93% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (8cfeee5) compared to base (8f94427).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@           Coverage Diff           @@
##            alpha    #1578   +/-   ##
=======================================
  Coverage   99.93%   99.93%           
=======================================
  Files          61       61           
  Lines        5963     5966    +3     
  Branches     1364     1366    +2     
=======================================
+ Hits         5959     5962    +3     
  Misses          4        4

Impacted Files	Coverage Δ
src/ParseFile.js	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

mtrezza · 2022-11-03T10:31:50Z

Great, so if you now removed the fix, the tests would fail, even without the override of Parse Server, correct?

musthafa1996 · 2022-11-03T10:32:44Z

Great, so without the fix, the tests would now fail, even without the override of Parse Server, correct?

Yes, that’s right.

mtrezza · 2022-11-03T10:42:34Z

@dplewis Not sure about the override suggestion maybe that would just complicate things? I think the sustainable solution is to move all into a monorepo. We are planning to do that in 2023. For this PR, all looks good now.

musthafa1996 · 2022-11-03T10:49:08Z

Anything pending here?

dplewis · 2022-11-03T19:08:56Z

@musthafa1996 I added a few more tests. @mtrezza I think this is ready for review

musthafa1996 · 2022-11-03T19:12:39Z

Any idea why we get this error in Github CI?

musthafa1996 · 2022-11-03T19:17:41Z

Rerunning the CI multiple times would get the integration test to succeed.

dplewis · 2022-11-03T19:21:39Z

@musthafa1996 Its a flaky test should be fixed when parse-community/parse-server#8232 gets merged in. Just reran the tests and everything checked out

musthafa1996 · 2022-11-03T19:23:47Z

Got it. Thanks 😊

mtrezza

Thanks everyone, looks good!

# [3.5.0-alpha.8](3.5.0-alpha.7...3.5.0-alpha.8) (2022-11-03) ### Bug Fixes * File upload fails when uploading base64 data ([#1578](#1578)) ([03ee3ff](03ee3ff))

parseplatformorg · 2022-11-03T19:28:51Z

🎉 This change has been released in version 3.5.0-alpha.8

dplewis · 2022-11-03T19:32:25Z

@musthafa1996 Thank you for looking into this _data was supposed to be internal on the JS side. I didn't know it was used on the server side. I'll add an addtional check and performance improvements on the server side

musthafa1996 · 2022-11-03T19:35:01Z

I’m glad that i could help fix the issue 😊

musthafa1996 · 2022-11-03T19:38:00Z

The issue was kind of tricky to find too due to parse server using an older version of the SDK. I’m really looking forward to mono-repo. Development and bug finding will be much easier.

## [3.5.1-beta.1](3.5.0...3.5.1-beta.1) (2022-11-03) ### Bug Fixes * File upload fails when uploading base64 data ([#1578](#1578)) ([03ee3ff](03ee3ff)) * React Native build does not maintain arrow functions and causes error with AsyncStorage ([#1587](#1587)) ([8aeaa4f](8aeaa4f))

parseplatformorg · 2022-11-03T19:41:30Z

🎉 This change has been released in version 3.5.1-beta.1

## [3.5.1-alpha.1](3.5.0...3.5.1-alpha.1) (2022-11-03) ### Bug Fixes * File upload fails when uploading base64 data ([#1578](#1578)) ([03ee3ff](03ee3ff)) * React Native build does not maintain arrow functions and causes error with AsyncStorage ([#1587](#1587)) ([8aeaa4f](8aeaa4f))

parseplatformorg · 2022-11-03T20:04:05Z

🎉 This change has been released in version 3.5.1-alpha.1

## [3.5.1](3.5.0...3.5.1) (2022-11-26) ### Bug Fixes * File upload fails when uploading base64 data ([#1578](#1578)) ([03ee3ff](03ee3ff)) * React Native build does not maintain arrow functions and causes error with AsyncStorage ([#1587](#1587)) ([8aeaa4f](8aeaa4f)) * SDK builds incorrectly since release 3.5.0 causing various bugs ([#1600](#1600)) ([f15154f](f15154f))

parseplatformorg · 2022-11-26T23:51:32Z

🎉 This change has been released in version 3.5.1

Override parse SDK to v3.5.0-alpha.6 in parse-server

6cf4173

musthafa1996 mentioned this pull request Oct 21, 2022

Issue uploading files #1579

Closed

4 tasks

musthafa1996 added 3 commits October 26, 2022 18:15

Add failing test case

6c08d86

Fix file upload

0f2626b

Override Parse SDK for Parse server using git repo

c8c1157

musthafa1996 changed the title ~~Override parse SDK to v3.5.0-alpha.6 in parse-server~~ fix: File file upload issue Oct 26, 2022

parse-github-assistant bot changed the title ~~fix: File file upload issue~~ fix: file file upload issue Oct 26, 2022

musthafa1996 commented Oct 27, 2022

View reviewed changes

integration/test/ParseFileTest.js Outdated Show resolved Hide resolved

musthafa1996 and others added 4 commits October 31, 2022 18:51

Merge branch 'alpha' into file-upload-issue

277b430

Remove unwanted test case in ParseFileTest.js

375e949

refactor: bump parse-server from b54af6b to 4918665 (#1585)

e041706

Override Parse SDK for Parse server using git https repo URL

f2eb1ae

mtrezza changed the title ~~fix: file file upload issue~~ fix: file upload issue Oct 31, 2022

mtrezza changed the title ~~fix: file upload issue~~ fix: file upload fails when uploading base64 data Oct 31, 2022

Merge branch 'alpha' into file-upload-issue

2e3d80e

mtrezza reviewed Oct 31, 2022

View reviewed changes

src/ParseFile.js Show resolved Hide resolved

musthafa1996 requested a review from dplewis November 3, 2022 10:57

add file unsaved tests

8cfeee5

mtrezza approved these changes Nov 3, 2022

View reviewed changes

mtrezza merged commit 03ee3ff into parse-community:alpha Nov 3, 2022

parseplatformorg pushed a commit that referenced this pull request Nov 3, 2022

chore(release): 3.5.0-alpha.8 [skip ci]

1687d19

# [3.5.0-alpha.8](3.5.0-alpha.7...3.5.0-alpha.8) (2022-11-03) ### Bug Fixes * File upload fails when uploading base64 data ([#1578](#1578)) ([03ee3ff](03ee3ff))

parseplatformorg added the state:released-alpha Released as alpha version label Nov 3, 2022

dplewis deleted the file-upload-issue branch November 3, 2022 19:30

parseplatformorg added the state:released-beta Released as beta version label Nov 3, 2022

musthafa1996 mentioned this pull request Nov 4, 2022

File upload fails using REST API --data-raw parse-community/parse-server#8253

Closed

parseplatformorg added the state:released Released as stable version label Nov 26, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: File upload fails when uploading base64 data #1578

fix: File upload fails when uploading base64 data #1578

musthafa1996 commented Oct 21, 2022 •

edited

Loading

parse-github-assistant bot commented Oct 21, 2022 •

edited

Loading

parse-github-assistant bot commented Oct 26, 2022

musthafa1996 commented Oct 27, 2022

musthafa1996 commented Oct 31, 2022

mtrezza commented Oct 31, 2022 •

edited

Loading

musthafa1996 commented Oct 31, 2022

musthafa1996 commented Oct 31, 2022 •

edited

Loading

mtrezza commented Oct 31, 2022 •

edited

Loading

musthafa1996 commented Oct 31, 2022

mtrezza commented Oct 31, 2022

mtrezza commented Oct 31, 2022

musthafa1996 commented Oct 31, 2022

mtrezza commented Oct 31, 2022 •

edited

Loading

musthafa1996 commented Oct 31, 2022

mtrezza left a comment

codecov bot commented Nov 3, 2022 •

edited

Loading

mtrezza commented Nov 3, 2022 •

edited

Loading

musthafa1996 commented Nov 3, 2022

mtrezza commented Nov 3, 2022 •

edited

Loading

musthafa1996 commented Nov 3, 2022

dplewis commented Nov 3, 2022

musthafa1996 commented Nov 3, 2022 •

edited

Loading

musthafa1996 commented Nov 3, 2022

dplewis commented Nov 3, 2022

musthafa1996 commented Nov 3, 2022

mtrezza left a comment

parseplatformorg commented Nov 3, 2022

dplewis commented Nov 3, 2022

musthafa1996 commented Nov 3, 2022

musthafa1996 commented Nov 3, 2022

parseplatformorg commented Nov 3, 2022

parseplatformorg commented Nov 3, 2022

parseplatformorg commented Nov 26, 2022

fix: File upload fails when uploading base64 data #1578

fix: File upload fails when uploading base64 data #1578

Conversation

musthafa1996 commented Oct 21, 2022 • edited Loading

New Pull Request Checklist

Issue Description

Approach

TODOs before merging

parse-github-assistant bot commented Oct 21, 2022 • edited Loading

Thanks for opening this pull request!

parse-github-assistant bot commented Oct 26, 2022

musthafa1996 commented Oct 27, 2022

musthafa1996 commented Oct 31, 2022

mtrezza commented Oct 31, 2022 • edited Loading

musthafa1996 commented Oct 31, 2022

musthafa1996 commented Oct 31, 2022 • edited Loading

mtrezza commented Oct 31, 2022 • edited Loading

musthafa1996 commented Oct 31, 2022

mtrezza commented Oct 31, 2022

mtrezza commented Oct 31, 2022

musthafa1996 commented Oct 31, 2022

mtrezza commented Oct 31, 2022 • edited Loading

musthafa1996 commented Oct 31, 2022

mtrezza left a comment

Choose a reason for hiding this comment

codecov bot commented Nov 3, 2022 • edited Loading

Codecov Report

mtrezza commented Nov 3, 2022 • edited Loading

musthafa1996 commented Nov 3, 2022

mtrezza commented Nov 3, 2022 • edited Loading

musthafa1996 commented Nov 3, 2022

dplewis commented Nov 3, 2022

musthafa1996 commented Nov 3, 2022 • edited Loading

musthafa1996 commented Nov 3, 2022

dplewis commented Nov 3, 2022

musthafa1996 commented Nov 3, 2022

mtrezza left a comment

Choose a reason for hiding this comment

parseplatformorg commented Nov 3, 2022

dplewis commented Nov 3, 2022

musthafa1996 commented Nov 3, 2022

musthafa1996 commented Nov 3, 2022

parseplatformorg commented Nov 3, 2022

parseplatformorg commented Nov 3, 2022

parseplatformorg commented Nov 26, 2022

musthafa1996 commented Oct 21, 2022 •

edited

Loading

parse-github-assistant bot commented Oct 21, 2022 •

edited

Loading

mtrezza commented Oct 31, 2022 •

edited

Loading

musthafa1996 commented Oct 31, 2022 •

edited

Loading

mtrezza commented Oct 31, 2022 •

edited

Loading

mtrezza commented Oct 31, 2022 •

edited

Loading

codecov bot commented Nov 3, 2022 •

edited

Loading

mtrezza commented Nov 3, 2022 •

edited

Loading

mtrezza commented Nov 3, 2022 •

edited

Loading

musthafa1996 commented Nov 3, 2022 •

edited

Loading