Skip to content

fix(usermanager): Don't throw when checking if a too long user id is an existing user - #52224

Merged
blizzz merged 2 commits into
masterfrom
bugfix/noid/dont-break-when-checking-if-too-long-user-exists
Apr 17, 2025
Merged

fix(usermanager): Don't throw when checking if a too long user id is an existing user#52224
blizzz merged 2 commits into
masterfrom
bugfix/noid/dont-break-when-checking-if-too-long-user-exists

Conversation

@nickvergessen

@nickvergessen nickvergessen commented Apr 16, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Steps

  • Write a comment or chat message with @ followed by 65+ a-z0-9 characters and enable LDAP
  • When LDAP checks if the user exists, it checks preferences
  • But preferences throws InvalidArgumentException in UserConfig::assertParams although it's not documented
  • Regression from IUserConfig #47658 @ArtificialOwl

Checklist

@nickvergessen nickvergessen added this to the Nextcloud 32 milestone Apr 16, 2025
@nickvergessen nickvergessen self-assigned this Apr 16, 2025
@nickvergessen
nickvergessen requested a review from a team as a code owner April 16, 2025 15:41
@nickvergessen
nickvergessen requested review from Altahrim, ArtificialOwl, icewind1991 and skjnldsv and removed request for a team April 16, 2025 15:41
blizzz
blizzz reviewed Apr 16, 2025
View reviewed changes

@blizzz blizzz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks fine, one suggestion

Comment thread lib/private/User/DisplayNameCache.php Outdated
@nickvergessen
fix(usermanager): Don't throw when checking if a too long user id is …
775ca88 
…an existing user

Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
nickvergessen force-pushed the bugfix/noid/dont-break-when-checking-if-too-long-user-exists branch from 8429eab to 775ca88 Compare April 16, 2025 15:47
@nickvergessen

nickvergessen commented Apr 17, 2025

Copy link
Copy Markdown
Member Author

/backport 775ca88 to stable31

@nickvergessen
nickvergessen requested a review from blizzz April 17, 2025 07:39
@nickvergessen
fix(user): Introduce a public constant for max length of user id
1ada991 
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
nickvergessen force-pushed the bugfix/noid/dont-break-when-checking-if-too-long-user-exists branch from f464541 to 1ada991 Compare April 17, 2025 07:50
@blizzz
blizzz merged commit 8410d67 into master Apr 17, 2025
@blizzz
blizzz deleted the bugfix/noid/dont-break-when-checking-if-too-long-user-exists branch April 17, 2025 14:55
@backportbot backportbot Bot mentioned this pull request Apr 17, 2025
Merged
@nextcloud-bot nextcloud-bot mentioned this pull request Aug 19, 2025
Merged
@skjnldsv skjnldsv modified the milestones: Nextcloud 32, Nextcloud 33 Sep 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@miaulalala miaulalala miaulalala approved these changes
@provokateurin provokateurin provokateurin approved these changes
@come-nc come-nc come-nc approved these changes
@SystemKeeper SystemKeeper Awaiting requested review from SystemKeeper
@icewind1991 icewind1991 Awaiting requested review from icewind1991 icewind1991 is a code owner automatically assigned from nextcloud/server-backend
@Altahrim Altahrim Awaiting requested review from Altahrim Altahrim is a code owner automatically assigned from nextcloud/server-backend
@skjnldsv skjnldsv Awaiting requested review from skjnldsv skjnldsv is a code owner automatically assigned from nextcloud/server-backend
@ArtificialOwl ArtificialOwl Awaiting requested review from ArtificialOwl
@blizzz blizzz Awaiting requested review from blizzz

Assignees

@nickvergessen nickvergessen

Labels

3. to review Waiting for reviews bug feature: users and groups regression

Projects

None yet

Milestone

Nextcloud 32

Development

Successfully merging this pull request may close these issues.

6 participants

@nickvergessen @blizzz @miaulalala @provokateurin @come-nc @skjnldsv