Exclude minimessage colors/tags when validating title & surname length#8092
Exclude minimessage colors/tags when validating title & surname length#8092Veyronity wants to merge 3 commits intoTownyAdvanced:masterfrom
Conversation
|
I'm not sure if this can have side effects like people sneaking in click/hover events since there's no limit for minimessage tags now, but I hope that's already filtered in status screen backend |
It currently does not filter that. |
LlmDl
left a comment
There was a problem hiding this comment.
I'm curious as to why we are allowing non-colour/font tags?
We don't want to store anything else, do we?
|
I wouldn't think so. Is there anything else other than escaping hovers too though? |
|
I could also add a config/hardcoded limit for max size even with colors, incase of issues saving or something |
| * Check and perform regex on Titles and Surnames given to residents. | ||
| * | ||
| * @param words an Array of strings that make up the title or surname. | ||
| * @param countColors whether to count minimessage tags in the string length |
There was a problem hiding this comment.
Something like this should do both minimessage and legacy &a/§a style colours in order to be truly useful.
It looks like you can use Colors.strip(title) for that.
|
|
||
| testForConfigBlacklistedName(title); | ||
|
|
||
| int textLength = countColors ? title.length() : TownyComponents.plain(TownyComponents.miniMessage(title)).length(); |
There was a problem hiding this comment.
| int textLength = countColors ? title.length() : TownyComponents.plain(TownyComponents.miniMessage(title)).length(); | |
| int textLength = countColors ? title.length() : Colors.strip(title).length(); |
| throw new InvalidNameException(Translatable.of("msg_err_name_validation_title_too_long", title)); | ||
|
|
||
| return title; | ||
| return TownyComponents.stripClickTags(title); |
There was a problem hiding this comment.
This should probably also strip any hover tags but @Warriorrrr might want to leave them in.
There was a problem hiding this comment.
I think hovers should be stripped as well.
|
|
||
| testForConfigBlacklistedName(title); | ||
|
|
||
| int textLength = countColors ? title.length() : Colors.strip(title).length(); |
There was a problem hiding this comment.
Ideally this would only strip tags that will be parsed in the actual title/surname, #8096 adds a new minimessage instance with a limited amount of tags that will be allowed to be used, once that's merged you'd be able to do this:
| int textLength = countColors ? title.length() : Colors.strip(title).length(); | |
| int textLength = countColors ? title.length() : TownyComponents.USER_SAFE.stripTags(Colors.translateLegacyCharacters(title)).length(); |
| throw new InvalidNameException(Translatable.of("msg_err_name_validation_title_too_long", title)); | ||
|
|
||
| return title; | ||
| return TownyComponents.stripClickTags(title); |
There was a problem hiding this comment.
I think hovers should be stripped as well.
| * @throws InvalidNameException if the title or surname is invalid. | ||
| */ | ||
| public static String checkAndFilterTitlesSurnameOrThrow(String[] words) throws InvalidNameException { | ||
| public static String checkAndFilterTitlesSurnameOrThrow(String[] words, boolean countColors) throws InvalidNameException { |
There was a problem hiding this comment.
countColors is currently always false, this new overload isn't used anywhere yet.
Description:
Resident titles & surnames support minimessage when displayed in
/residentstatus screens. The configured max length is easily reached when using tags such as or multiple colors (King)By making this pull request, I represent that I have the right to waive copyright and related rights to my contribution, and agree that all copyright and related rights in my contributions are waived, and I acknowledge that the TownyAdvanced organization has the copyright to use and modify my contribution under the Towny License for perpetuity.