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

Make use of Base64 encoding while transferring messages #21

Open
gridhead opened this issue Sep 25, 2020 · 13 comments · May be fixed by #38
Open

Make use of Base64 encoding while transferring messages #21

gridhead opened this issue Sep 25, 2020 · 13 comments · May be fixed by #38
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@gridhead
Copy link
Owner

Base64 does add a bit of redundancy due to padding but it is a much safer and robust way to transfer messages over the network.

@gridhead gridhead added enhancement New feature or request good first issue Good for newcomers hacktoberfest Contribute to the notion of open-source this October! labels Sep 25, 2020
@ShubhamPalriwala
Copy link

Hey !
This might be a long shot but I'd like to try and resolve this issue out.

@gridhead
Copy link
Owner Author

gridhead commented Sep 25, 2020

Cool. I am assigning you this issue. Happy Hacktoberfest! 😄
(Take this. It might come handy)

@gridhead
Copy link
Owner Author

Hi @ShubhamPalriwala, Be sure to get yourself registered at https://organize.mlh.io/participants/events/4659-hack-astrosonic, our official Hacktoberfest event to stay in touch with the fellow contributors of this project.

Also, feel free to leave a star on the project - should you find it worth your time. Happy hacking! 😄

@varolbora5
Copy link

varolbora5 commented Oct 13, 2020

Hi!
I'm seeing the first person to be assigned haven't really done much or at least didn't put a pull request through, could you assign me too?

Edit: Do you want anything that goes through the server to be encoded or only messages users send?

@gridhead
Copy link
Owner Author

Hi @varolbora5. thanks for expressing your interest in the issue. You could tag the assignee to see if they are interested to continue and if not, I can have unassigned in favour for your assignment.

@ShubhamPalriwala
Copy link

@varolbora5 am still trying to figure which layers require the encoding, if you would also like to do it, let both of us do and whosoever does it first, gets to make a PR. What do you say?

If @t0xic0der is okay with this

@varolbora5
Copy link

@ShubhamPalriwala It's ok for me
But I should warn you, I already figured out what to do

@ShubhamPalriwala
Copy link

As far as it helps @t0xic0der am happy for you, just warning you, I might be writing my own Base64 algo too! lol jk

@gridhead
Copy link
Owner Author

As far as it helps @t0xic0der am happy for you, just warning you, I might be writing my own Base64 algo too! lol jk

I'm afraid it might not be needed for a simple project like this. We are preferring utility over innovation when it comes to this issue.

@varolbora5 am still trying to figure which layers require the encoding, if you would also like to do it, let both of us do and whosoever does it first, gets to make a PR. What do you say?

If @t0xic0der is okay with this

Sounds good to me.

@ShubhamPalriwala It's ok for me
But I should warn you, I already figured out what to do

Feel free to make a PR.

@vinmay
Copy link
Contributor

vinmay commented Oct 16, 2020

hey @t0xic0der, I have a change ready for this ! I did not know about the assignment. Let me know if you want me to keep my PR open.

@vinmay
Copy link
Contributor

vinmay commented Oct 16, 2020

@t0xic0der There is one issue I see here when we are trying to encode the messages specifically. When we are encoding the message, it will be converted into bytes, so the approach would be to encode, not just the message but the whole json. This will be in contention with the cphrsuit.cphrsuite requires a str and not bytes. Are we thinking of replacing cphrsuite as well ?

@gridhead
Copy link
Owner Author

hey @t0xic0der, I have a change ready for this ! I did not know about the assignment. Let me know if you want me to keep my PR open.

Keep it open for now. If the assignee does not meet the requirements then I might just assign you and begin reviewing your pull request.

@gridhead
Copy link
Owner Author

@t0xic0der There is one issue I see here when we are trying to encode the messages specifically. When we are encoding the message, it will be converted into bytes, so the approach would be to encode, not just the message but the whole json. This will be in contention with the cphrsuit.cphrsuite requires a str and not bytes. Are we thinking of replacing cphrsuite as well ?

One way to approach this would be to typecast the JSON object to string, and then converting those strings to bytes. I am open to suggestions though if the aforementioned approach makes it slow.

@gridhead gridhead linked a pull request Oct 17, 2020 that will close this issue
@gridhead gridhead removed the hacktoberfest Contribute to the notion of open-source this October! label Nov 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants