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

BUG: invalid user|org / project name #154

Closed
3 of 4 tasks
nelsonic opened this issue Mar 11, 2022 · 8 comments
Closed
3 of 4 tasks

BUG: invalid user|org / project name #154

nelsonic opened this issue Mar 11, 2022 · 8 comments

Comments

@nelsonic
Copy link
Member

nelsonic commented Mar 11, 2022

Took a quick pomodoro break and looked at the Hits homepage: https://hits.dwyl.com

Noticed that there are still strange entries, e.g:

Fri, 11 Mar 2022 10:45:02 /' | remove_first: 'http:/' }}{{ page.url }}.svg 3618

What even is that? 🤷‍♂️

Then there are the more innocuous but still incorrect ones:

Fri, 11 Mar 2022 10:43:12 /{mikel-brostrom}/{Yolov5_DeepSort_Pytorch}.svg 3562
Fri, 11 Mar 2022 10:06:13 /{cemac}/{SWIFTDB}.svg 22

image

I think we need to be more strict and actually reject requests that have invalid user|org or repo ... 💭
We should create a new badge e.g: hits-invalid-url

https://img.shields.io/badge/hits-invalid%20url-red?style=flat-square

Todo

  • Write tests that attempt to request an invalid url using the samples above
    • Should return the "invalid url" badge
  • Implement the fix
  • Publish new version
@SimonLab
Copy link
Member

username rules:
image

@nelsonic
Copy link
Member Author

Indeed some form validation would go a long way.
but people are always going to do URL manipulation so we need some server side validation of requested SVGs. 💭

@SimonLab
Copy link
Member

@SimonLab
Copy link
Member

I've created the following function to check the username and repository values:

defp user_valid?(user), do: String.match?(user, ~r/^([[:alnum:]]+-)*[[:alnum:]]+$/)
defp repository_valid?(repo), do: String.match?(repo, ~r/^[[:alnum:]-_.]+$/)

@nelsonic
Copy link
Member Author

Thanks for looking at this. 👍
Definitely add comments for these functions and ref this issue. 💭

@nelsonic
Copy link
Member Author

@SimonLab thanks for creating the PR: #155
I think we need to have a basic 404 page in the app explaining that the URL needs to be valid for the badge to work just to improve the UX for the people who mistakenly created an invalid URL.

@SimonLab
Copy link
Member

I've created a new error page:
image

When a badge is created the link will direct to "/user/repository":
image

On this endpoint we check if the user and repository are valid and if not redirect to the new error page

@nelsonic
Copy link
Member Author

Fixed. #155

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants