-
-
Notifications
You must be signed in to change notification settings - Fork 775
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
makes the profile urls.py entry way more restrictive #5454
Conversation
Codecov Report
@@ Coverage Diff @@
## stable #5454 +/- ##
==========================================
+ Coverage 29.67% 29.88% +0.21%
==========================================
Files 242 242
Lines 20652 20757 +105
Branches 2985 3003 +18
==========================================
+ Hits 6128 6203 +75
- Misses 14272 14298 +26
- Partials 252 256 +4
Continue to review full report at Codecov.
|
app/app/urls.py
Outdated
re_path(r'^(.*)', dashboard.views.profile, name='profile_min'), | ||
re_path(r'^([a-z|A-Z|0-9|\.]+)/([a-z|A-Z|0-9|\.]+)/?$', dashboard.views.profile, name='profile_min'), | ||
|
||
re_path(r'^([a-z|A-Z|0-9|\.]+)/?$', dashboard.views.profile, name='profile_min'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still open to dropping the short url to profile :P
just saying
@owocki mind if I switch this to master ? We are deploying in less than 24 hours |
app/app/urls.py
Outdated
@@ -610,8 +610,9 @@ | |||
] | |||
|
|||
urlpatterns += [ | |||
re_path(r'^(.*)/(.*)?', dashboard.views.profile, name='profile_min_by_tab'), | |||
re_path(r'^(.*)', dashboard.views.profile, name='profile_min'), | |||
re_path(r'^([a-z|A-Z|0-9|\.]+)/([a-z|A-Z|0-9|\.]+)/?$', dashboard.views.profile, name='profile_min'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the name should be profile_min_by_tab
here right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested on my local working fine with /username /username/area profile/username and profile/username/area
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@danlipert yes
note to self : figure out what non alphanumeric chars we want to support so that usernames like http://localhost:8000/virtual-face work.. maybe just anything thats not a forward slash?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/shinnn/github-username-regex /^[a-z\d](?:[a-z\d]|-(?=[a-z\d])){0,38}$/i
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
true slash profiles will fail here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have a fix commiting here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description
makes the profile urls.py entry way more restrictive
Refers/Fixes
see octavio msg https://gitcoincore.slack.com/archives/CC24APWGN/p1572989388030500
and likely other profile handle URL things we haven't figured out already!
Testing
tested against http://localhost:8000/media/avatars/72f4f6d365732c50762730b1bc630379/octavioamu.png
http://localhost:8000/owocki
http://localhost:8000/owocki/resume