-
Notifications
You must be signed in to change notification settings - Fork 2.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
Do not allow white space at the start or end of a group name #39540
Conversation
I have set this to "Draft" because we do not want to merge stuff to master right now. But I would appreciate a review of this. There is currently edge-case differences between oC10 (with MySQL/MariaDb) and oCIS, particularly for the case of groups "finance" and "finance " oC10 effectively treats those names as "the same thing". oCIS treats them as separate groups. (And I think oC10 with pgsql treats them as separate groups) This PR defines the behavior for this crud. So please review and give your feedback. If we agree with what I have done here, then I can cherry-pick this into #39514 (where I am accumulating stuff that is waiting to go in |
Kudos, SonarCloud Quality Gate passed! |
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.
LGTM, I think it's a good thing as well.
Merged to master in PR #39612 |
Description
Do not allow a group to be created with a name that has white space:
This is on top of PR #39530 which demonstrates the current behavior for groups with white space at the beginning or end of the group name.
The Provisioning API acts as if "finance" and "finance " are the same group, because of:
https://stackoverflow.com/questions/17876478/why-the-sql-server-ignore-the-empty-space-at-the-end-automatically
https://support.microsoft.com/en-us/topic/inf-how-sql-server-compares-strings-with-trailing-spaces-b62b1a2d-27d3-4260-216d-a605719003b0
If I create group "finance" first, then try to create group "finance ", the groupExists checks find that "finance " exists already, and refuses to create it.
If I create group "finance " first, then try to create group "finance", the groupExists checks find that "finance" exists already, and refuses to create it.
Item (1) seems a reasonable "accident" of the code - it prevents creating an extra group name with a space on the end.
Item (2) is a bit odd - if we accidentally create group "finance " then we are stuck with "finance " in the database. And when we do an API request like:
We get elements with that space at the end of the name like:
And to query the group directly we have to:
Also: https://jira.atlassian.com/browse/CWD-4290
That claims that pgsql will behave differently than mySQL/MariaDb, and likely happily add separate table rows for "finance" and "finance " groups, so they will be treated as different groups in an oC10 installation using pgsql.
So that is another reason to refuse to create groups (or any string data) with trailing spaces - different databases, and implementations not using an SQL database, might behave differently.
Therefore this PR prevents creation of a group with a name that ends in white space, including a group name that is only white space.
It seems silly to allow group names that start with white space, like " finance", that would be strangely inconsistent with banning white space at the end of the name. So this PR also prevents white space at the start of a group name.
As well as unit test cases, I have added API acceptance test scenarios, because we want oCIS to behave the same. IMO oCIS currently allows spaces at the beginning and end, and considers all combinations like " finance", "finance" and "finance " to be separate valid groups.
Related Issue
https://github.com/owncloud/enterprise/issues/4890
#39533 - sorts out the requirements/rules for that issue.
How Has This Been Tested?
CI
Types of changes
Checklist: