-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Mitigate encoding issue with user principal uri #22770
Conversation
@MorrisJobke @rullzer I would appreciate if we could backport this |
e91618a
to
fcddf8b
Compare
Fixed a typo in the comment |
🤖 beep boop beep 🤖 Here are the logs for the failed build: Status of 32951: failureacceptance-access-levels
Show full log
acceptance-app-files
Show full log
acceptance-app-files-sharing
Show full log
acceptance-app-files-sharing-link
Show full log
acceptance-app-files-tags
Show full log
acceptance-app-theming
Show full log
acceptance-header
Show full log
acceptance-login
Show full log
acceptance-users
Show full log
acceptance-apps
Show full log
|
Signed-off-by: Georg Ehrke <developer@georgehrke.com>
fcddf8b
to
b5204a7
Compare
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.
code looks good 👍
/backport to stable19 |
/backport to stable18 |
/backport to stable17 |
$user = $this->userManager->get($name); | ||
// Depending on where it is called, it may happen that this function | ||
// is called either with a urlencoded version of the name or with a non-urlencoded one. | ||
// The urldecode function replaces %## and +, both of which are forbidden in usernames. |
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.
This assumption is not correct. For historic reasons there still are usernames containing "+".
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.
=> Discussion at #23315
To test this:
Master:
This branch:
Disclaimer:
The problem is bigger than this and requires a better fix in the long run. It is necessary, to have consistent, url-encoded principal uris everywhere.