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

Maybe Fix the non escaped password in the logi request body #22

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bin-bash-mak
Copy link

I am really sorry but I don't have the means to currently test the changes on an actual esp but the code builds

The issue with current approach is that maybe the json becomes invalid if a " is present in the password. the issue should also happen if the user includes a " in the email / phone.

Small question, is there any reason why we're not using the ArduinoJson's to serialize the json.

Also if I have maybe some changes in the future to to deduplicate some shared logic between the realtime and supabase classes and add some functionality for using the refresh token functionality as an example . Is it ok to open issues and feature requests to collaborate on implementing the changes in a clean interface that is inline with the collaborator's vision for the library?

@@ -30,7 +30,10 @@ int SupabaseRealtime::_login_process()
Loginhttps.addHeader("apikey", key);
Loginhttps.addHeader("Content-Type", "application/json");

String query = "{\"" + loginMethod + "\": \"" + phone_or_email + "\", \"password\": \"" + password + "\"}";
String escapedPassword = password;
escapedPassword.replace("\"", "\\\"");
Copy link
Author

Choose a reason for hiding this comment

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

I really don't like this. Can we consider the use of ArduinoJson's Serialization

@bin-bash-mak bin-bash-mak changed the title Maybe Fix the non escaped password in Maybe Fix the non escaped password in the logi request body Oct 30, 2024
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.

1 participant