Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Display name limit doesn't apply to per-room display names #9236

Closed
babolivier opened this issue Jan 27, 2021 · 12 comments · Fixed by #10654
Closed

Display name limit doesn't apply to per-room display names #9236

babolivier opened this issue Jan 27, 2021 · 12 comments · Fixed by #10654
Labels
good first issue Good for newcomers S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. Z-Help-Wanted We know exactly how to fix this issue, and would be grateful for any contribution

Comments

@babolivier
Copy link
Contributor

As per discussion in Matrix HQ, it seems like Synapse's limit to display name length doesn't apply to per-room display names.

@anoadragon453
Copy link
Member

This would just be adding a length check to update_membership_locked that would fail the request if the length was too long. Similar to where we prevent per-room profile changes entirely:

if (
not self.allow_per_room_profiles and not is_requester_server_notices_user
) or requester.shadow_banned:
# Strip profile data, knowing that new profile data will be added to the
# event's content in event_creation_handler.create_event() using the target's
# global profile.
content.pop("displayname", None)
content.pop("avatar_url", None)

@anoadragon453 anoadragon453 added good first issue Good for newcomers S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. Z-Help-Wanted We know exactly how to fix this issue, and would be grateful for any contribution labels Feb 1, 2021
@b-thebest
Copy link

Can I work on this issue ??

@anoadragon453
Copy link
Member

@b-thebest Of course!

@b-thebest
Copy link

b-thebest commented Mar 4, 2021 via email

@arya2331
Copy link
Contributor

is someone still working on it? or I can do this?

@b-thebest
Copy link

b-thebest commented Apr 15, 2021 via email

@richvdh
Copy link
Member

richvdh commented Apr 15, 2021

@b-thebest do you have any idea of when you expect to make some progress on it? I don't think it's a big job and if others are ready to pick it up it might be nice to let them do so.

@b-thebest
Copy link

b-thebest commented Apr 15, 2021 via email

@anoadragon453
Copy link
Member

Hey @b-thebest, have you been able to make any progress on this?

@b-thebest
Copy link

Yes I was working on it. Sorry for being late, during past days my health conditions were not good that's why I was unable to complete it. From today I will take it on and try to complete it as soon as possible

@richvdh
Copy link
Member

richvdh commented May 24, 2021

As is normal in open-source projects, we don't generally "assign" issues to individual contributors, and you shouldn't consider this a prerequisite to start work on it.

@lukasdenk
Copy link
Contributor

@Azrenbeth Should this issue still be open? According to #10654 the issue is solved.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue Good for newcomers S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. Z-Help-Wanted We know exactly how to fix this issue, and would be grateful for any contribution
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants