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

Bad URI parsing strategy #233

Closed
RiccardoM opened this issue Jul 17, 2020 · 0 comments · Fixed by #235
Closed

Bad URI parsing strategy #233

RiccardoM opened this issue Jul 17, 2020 · 0 comments · Fixed by #235
Assignees
Labels
kind/bug Something isn't working
Milestone

Comments

@RiccardoM
Copy link
Contributor

Bug description

Taking a look at Primer entries for Phase 5 challenge "Create or Edit Account", I've noticed that some transactions are failing. Particularly, this one is one of them.

If we take a look at the LCD endpoint we can see that this is failing due to what it appears to be an invalid picture URI:

invalid request: invalid profile picture uri provided: failed to execute message; message index: 0

The problem is that the provided URI is completely valid and successfully redirects to an existing image.

I think this bug is caused by the currently used RegEx to check for an URI validity, which is:

^(?:http(s)?://)[\w.-]+(?:\.[\w.-]+)+[\w\-._~:/?#[\]@!$&'()*+,;=]+$

Proposed solution

To solve this bug, what I propose we do is:

  1. Remove the currently used commonTypes.URIRegEx and profilesTypes.URIRegEx regular expressions.
  2. Define a new commonTypes.IsUriValid method which takes a generic URI and validates it properly.
  3. Inside the newly defined method, use Go's ParseURIRequest method

This will ensure easier to change validation method that is constant across all modules

@RiccardoM RiccardoM added the kind/bug Something isn't working label Jul 17, 2020
@RiccardoM RiccardoM self-assigned this Jul 17, 2020
@RiccardoM RiccardoM added this to the v0.10.0 milestone Jul 17, 2020
RiccardoM added a commit that referenced this issue Jul 17, 2020
Signed-off-by: riccardo.montagnin <riccardo.montagnin@gmail.com>
@RiccardoM RiccardoM mentioned this issue Jul 17, 2020
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant