Skip to content
This repository has been archived by the owner on Sep 26, 2022. It is now read-only.

(iOS) Fix: Allow non-object request bodies #141

Merged

Conversation

emily-curry
Copy link
Contributor

@emily-curry emily-curry commented Jul 3, 2021

This change moves parsing the "data" arg from JSValue into more specific types further down into the content-type specific handlers. The abstract capacitor JSValue is suitable for passing to the JSON serializer, so by doing this, we fix POST/PATCH/etc on iOS for data structures other than objects/dictionaries. This was important to me as I need to send an array as my POST body.

Additionally, when the request body sent to this plugin is improperly formed, we now explicitly throw before even making the request. This is both to ensure we don't send a malformed request, and also to provide the developer with some insight as to where and why the request is failing (specifically, I always want to know if the plugin is generating the error, or if the server is generating the error. Setting a null request body and then sending the request and getting a 500 is misleading to the developer). As part of this change, we also now always try to parse the request body as a string as a fallback, even if it's not of a whitelisted content-type.

Depends on #139, as there would be merge conflicts otherwise, and I don't see that one being problematic.
Fixes #43 (unless this is also an issue on android, I haven't checked yet)

@emily-curry
Copy link
Contributor Author

Oops, I did not even know about #128, or I'd have based this PR on that.

I do hope this will still be reviewed and considered:

  • The approach that I have taken will handle more than just arrays (as silly as it is, plain primitives are valid JSON).
  • Throwing errors early is more useful to the developer than sending a malformed request.
  • For non-explicitly-supported content-types, being able to send a string you've constructed in JS is better than not being able to send that request at all.
    • It might even be reasonable to remove multipart/form-data support from the library entirely, and expect the client to send the data as a string and do that formatting itself in JS (though personally I think the library is already "helping" send json and form-urlencoded, and multipart/form-data seems basically equivalent and therefore appropriate for this library to do).

@thomasvidas
Copy link
Contributor

Thanks for your multiple contributions @emily-curry 😄 These all are great!

@thomasvidas thomasvidas changed the base branch from master to 1.1.0 July 15, 2021 14:28
@thomasvidas thomasvidas merged commit 73f1033 into capacitor-community:1.1.0 Jul 15, 2021
This was referenced Jul 15, 2021
thomasvidas added a commit that referenced this pull request Jul 15, 2021
* feat: Add support for CJS
* feat(android): Set "GET" as default HTTP function in `downloadFile` function
* feat(android):  Updated downloadFile method on RequestHandler to use GET by default
* feat: Adding support for posting json arrays. (#128)
* Android - Prevent extra new line in text response.
* Android - Support json array requests.
* Android - Add error property on JSObject output.
* ios - Support array request.
* feat(web): Add additional logic for FormData typed request bodies (#126)
* fix(android): fix bug on android when receiving plain boolean values (#145)
* feat(iOS) Fix: Allow non-object request bodies (#141)
* feat: implement getRequestDataAsMultipartFormData
* fix: allow non-object http body
* feat(iOS): Add fallback to always try to parse the request data no matter the content type
* feat(iOS): implement getRequestDataAsMultipartFormData (#139)
* fix(android): Allow non-object request bodies (#143)
* feat: implement getRequestDataAsMultipartFormData
* fix: allow non-object http body
* feat(iOS): Add fallback to always try to parse the request data no matter the content type
* feat(android): non-object post body
* fix: bug where httpconn was being written to after it was closed
* fix(android): add in missing get, post, put, patch, and del functions to public API
* fix(ios): Publicly expose get, post, put, patch, del functions on Http plugin object
* chore(example): Update example project lockfiles
* fix(android): Fix issues with timeouts and remove unused imports
* chore: Cleanup .gitignore
* chore: Bump version to 1.1.0

Co-authored-by: ultimate-tester <ultimate-tester@users.noreply.github.com>
Co-authored-by: Joel Nieto <joelnietotec@outlook.com>
Co-authored-by: Adrian Sanchez <sgzadrian@gmail.com>
Co-authored-by: milanc <8333458+milanc@users.noreply.github.com>
Co-authored-by: herecoulbeyourname <57253976+herecoulbeyourname@users.noreply.github.com>
Co-authored-by: Frank608 <56638143+Frank608@users.noreply.github.com>
Co-authored-by: Emily Curry <emily@curry.sh>
@emily-curry emily-curry deleted the fix/ios-post-body-non-obj branch July 16, 2021 15:50
@emily-curry
Copy link
Contributor Author

@thomasvidas You're welcome! And thank you for maintaining this project! It has been hugely beneficial to the app my team is working on.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Send string in POST request
2 participants