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

Jump to bottom

[android] update base64 decoding options #7841

Merged
merged 20 commits into from Apr 16, 2020
Merged

[android] update base64 decoding options #7841

merged 20 commits into from Apr 16, 2020

Conversation

jarvisluong
Copy link
Contributor

Why

This PR is for aligning the base64 decoding output format for Android, iOS and Web in certain modules: ImagePicker, Camera, ImageManipulator, Print and SecureStore

Linked issue: #7837

How

I was using ImageManipulator and noticed that the output of base64 result was not similar from Android comparing iOS and web, and learned that the default decoding options for Android was the cause.

Test Plan

I updated the e2e test to check for the appearance of newline and \r character

@jarvisluong jarvisluong changed the title Patch/base64 decode nowrap [android] update base64 decoding options Apr 14, 2020
bbarthec
bbarthec reviewed Apr 15, 2020
View reviewed changes
apps/test-suite/tests/Camera.js Outdated Show resolved Hide resolved
packages/expo-camera/CHANGELOG.md Outdated Show resolved Hide resolved
packages/expo-image-manipulator/CHANGELOG.md Outdated Show resolved Hide resolved
packages/expo-image-picker/CHANGELOG.md Outdated Show resolved Hide resolved
packages/expo-secure-store/CHANGELOG.md Outdated Show resolved Hide resolved
jarvisluong and others added 5 commits April 15, 2020 11:50
@jarvisluong @bbarthec
Update apps/test-suite/tests/Camera.js
b1d0db6 
Co-Authored-By: Bartłomiej Bukowski <bartlomiejbukowski.b@gmail.com>
@jarvisluong @bbarthec
Update packages/expo-image-manipulator/CHANGELOG.md
77d84b1 
Co-Authored-By: Bartłomiej Bukowski <bartlomiejbukowski.b@gmail.com>
@jarvisluong @bbarthec
Update packages/expo-secure-store/CHANGELOG.md
7d6aaea 
Co-Authored-By: Bartłomiej Bukowski <bartlomiejbukowski.b@gmail.com>
@jarvisluong @bbarthec
Update packages/expo-camera/CHANGELOG.md
bc0fe1e 
Co-Authored-By: Bartłomiej Bukowski <bartlomiejbukowski.b@gmail.com>
@jarvisluong @bbarthec
Update packages/expo-image-picker/CHANGELOG.md
6bc8610 
Co-Authored-By: Bartłomiej Bukowski <bartlomiejbukowski.b@gmail.com>
lukmccall
lukmccall approved these changes Apr 15, 2020
View reviewed changes
Copy link
Contributor

@lukmccall lukmccall left a comment

Choose a reason for hiding this comment

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

LGTM 🎉
Just one comment ;)

packages/expo-print/CHANGELOG.md Outdated Show resolved Hide resolved
@jarvisluong @lukmccall
Update packages/expo-print/CHANGELOG.md
b279e3c 
Co-Authored-By: Łukasz Kosmaty <lukasz.kosmaty@student.uj.edu.pl>
@bbarthec bbarthec merged commit 101a207 into expo:master Apr 16, 2020
@cruzach cruzach mentioned this pull request Apr 16, 2020
Closed
@jarvisluong jarvisluong deleted the patch/base64-decode-nowrap branch May 26, 2020 19:52
@sofianear
Copy link

sofianear commented Dec 19, 2020 •
edited

Hello,
I have a question about expo-secure-store in sdk 38 and I hope it's the right place to ask it (sorry if it's not).
I am currently using expo-secure-store under expo sdk 37 and I'd like to upgrade to sdk 38.
It is mentioned in the CHANGELOG of the sdk 38 that this pull request has introduced breaking changes:

The base64 output will no longer contain newline and special character

So my question is: how will these changes affect my usage of expo-secure-store module and what should I do to fix compatibility issues if any.
I have actually went through the API reference of expo-secure-store (for both sdk 37 and sdk 38) and I haven't seen anything about base64 encoding/decoding there. So I really don't know what to do, and I don't want to upgrade to sdk 38 only to find issues that I don't know how to fix.

@jarvisluong
Copy link
Contributor Author

@sofianear As far as I understand, if you don't care about newline characters and /r character you are good to go. Do you send the base64 output to the server? I believe most base64 decoder now handle just fine a standard base64 output without special characters in it.

@sofianear
Copy link

sofianear commented Dec 19, 2020 •
edited

@jarvisluong
I am storing a string in the local storage using expo-secure-store. I also extract it and send it to a remote server. So I suppose that when I extract the string from local storage, it is decoded and therefore not in base64. Is that right ?

@jarvisluong
Copy link
Contributor Author

@sofianear That should be right 🙂

@sofianear
Copy link

@jarvisluong Thank you.

prakashbask pushed a commit to prakashbask/expo that referenced this pull request Mar 16, 2022
@jarvisluong @bbarthec @lukmccall
[android] update base64 decoding options (expo#7841)
dced656 
* [android] [camera] use .NO_WRAP to encode base64 output

* [android] [image-manipulator] use .NO_WRAP to encode base64 output

* [android] [image-picker] use .NO_WRAP to encode base64 output

* [android] [print] use .NO_WRAP to encode base64 output

* [android] [secure-store] use .NO_WRAP to encode base64 output

Co-authored-by: Bartłomiej Bukowski <bartlomiejbukowski.b@gmail.com>
Co-authored-by: Łukasz Kosmaty <lukasz.kosmaty@student.uj.edu.pl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukmccall lukmccall lukmccall approved these changes

@bbarthec bbarthec bbarthec approved these changes

@mczernek mczernek Awaiting requested review from mczernek

@sjchmiela sjchmiela Awaiting requested review from sjchmiela

@tsapeta tsapeta Awaiting requested review from tsapeta

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@jarvisluong @sofianear @lukmccall @bbarthec