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

Ensure wallet addresses are case insensitive #2815

Open
1 task
mkurapov opened this issue Jul 18, 2024 · 7 comments · May be fixed by #2833
Open
1 task

Ensure wallet addresses are case insensitive #2815

mkurapov opened this issue Jul 18, 2024 · 7 comments · May be fixed by #2833
Assignees
Labels
good first issue Good for newcomers pkg: backend Changes in the backend package.

Comments

@mkurapov
Copy link
Contributor

Context

Because wallet addresses (and payment pointers) are in fact URLs, they should be case INsensitive. Currently, we are storing them in the DB simply as unique string without doing any cleanup beforehand, meaning, they are technically case-sensitive now.

Todos

  • Make sure the wallet address URLs we store in backend are case-insensitive
@mkurapov mkurapov added pkg: backend Changes in the backend package. good first issue Good for newcomers labels Jul 18, 2024
@s100tist
Copy link

s100tist commented Jul 18, 2024

Is there anyone working on this yet? I'd like to try it:)

@golobitch
Copy link
Collaborator

Is there anyone working on this yet? I'd like to try it:)

Go for it :)

@s100tist
Copy link

Hi, I looked at the code and then I thought it would be enough modifying the wallet addres url to lower case before it got registered in the database.

When i tested the wallet address creation in Rafiki Admin it stored the wallet address url in lower case successfully.
Then I tested a wallet address creation via bruno scenarios and the wallet address was succesfully created as well. However the sucessive events failed; this because the middleware was still case sensitive.

By this time i left my code letting both the storage and the getWalletAddressUrl using the 'toLowerCase' function. This seems to patch the case-sensitiveness in the storing and in the looking. I don't know if the modified storage would affect any other scenarios. What would your feedback be?

@mkurapov
Copy link
Contributor Author

When i tested the wallet address creation in Rafiki Admin it stored the wallet address url in lower case successfully.

Even if you had upper case characters in the URL, correct?

However the sucessive events failed; this because the middleware was still case sensitive.

What exactly did you end up trying? So I can understand correctly

@s100tist
Copy link

That's correct, it stores the url in lower case even if it has a upper case characters.

In first place i just convert to lowercase the url
image
And then i added this to the methods where we obtain the url
image

@s100tist
Copy link

I made the middleware modifications because when i did some tests rafiki looked for the wallet address as it was firstly written it looked for it using the url with the upper case, when they're only stored in lower case

@sabineschaller
Copy link
Member

@s100tist feel free to open a draft PR such that we can see what you did 🙂

@s100tist s100tist linked a pull request Aug 1, 2024 that will close this issue
2 tasks
@mkurapov mkurapov linked a pull request Sep 10, 2024 that will close this issue
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers pkg: backend Changes in the backend package.
Projects
Status: Ready for Review
Development

Successfully merging a pull request may close this issue.

4 participants