Validate vote service site names - #1540
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b580931295
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| public final class ServiceSiteValidator { | ||
| private static final int MAX_LENGTH = 128; | ||
| private static final Pattern VALID_SERVICE_SITE = Pattern | ||
| .compile("[A-Za-z0-9][A-Za-z0-9 ._:/-]{0," + (MAX_LENGTH - 1) + "}"); |
There was a problem hiding this comment.
When an existing VoteSites.yml entry uses a service name such as Site's or https://list.example/vote?id=1, this whitelist causes the standalone, proxy, and backend handlers to reject the vote before the configured exact-match lookup, so every vote and reward for that site is lost after upgrading. ConfigVoteSites#setServiceSite still accepts arbitrary strings and no migration or configuration warning is provided; validate only control/path-dangerous input or apply the restrictive rules solely when deriving generated configuration keys.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dca3e77573
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!ServiceSiteValidator.isValid(siteName)) { | ||
| plugin.getLogger().warning("Unable to generate vote site with unsupported name '" | ||
| + ServiceSiteValidator.sanitizeForLog(siteName) + "'"); | ||
| return; |
There was a problem hiding this comment.
When an administrator enters a rejected name through the middle-click VoteSites creation GUI, this silent return leaves AdminGUI.onInput believing creation succeeded: it reports "Generated site", reloads, then passes the resulting null site to openAdminGUIVoteSiteSite, which immediately dereferences it and throws. Return a success result or make the GUI validate the name before continuing so rejected input produces an error instead of a broken GUI action.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b435fb7e20
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| int type = Character.getType(codePoint); | ||
| return type == Character.CONTROL || type == Character.FORMAT || type == Character.LINE_SEPARATOR | ||
| || type == Character.PARAGRAPH_SEPARATOR || type == Character.SURROGATE; |
There was a problem hiding this comment.
Reject Unicode space separators that String.isBlank() does not recognize, such as U+00A0, U+2007, and U+202F. A Votifier service name consisting solely of one of these characters currently passes validation, and automatic site creation also leaves it unchanged because the normalization regex uses default \s; this creates an effectively invisible VoteSites key instead of rejecting the malformed name.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac7959c0fd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!Character.isWhitespace(codePoint) && !Character.isSpaceChar(codePoint)) { | ||
| hasVisibleCharacter = true; |
There was a problem hiding this comment.
A service name consisting solely of U+FE0F (VARIATION SELECTOR-16), or U+034F (COMBINING GRAPHEME JOINER), reaches this branch because these code points are neither whitespace nor space characters; Java classifies them as non-spacing marks, so isDisallowed also permits them. The validator consequently accepts an effectively invisible name and automatic creation writes an invisible VoteSites key. Reject default-ignorable code points or require at least one actual base/rendering character.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dcbf1e9e90
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return type != Character.NON_SPACING_MARK && type != Character.COMBINING_SPACING_MARK | ||
| && type != Character.ENCLOSING_MARK; |
There was a problem hiding this comment.
When the service name consists only of U+3164 HANGUL FILLER, Java classifies it as OTHER_LETTER, not whitespace or a combining mark, so this method treats it as visible and automatic site creation writes an effectively invisible VoteSites key. Fresh evidence beyond the earlier variation-selector report is that default-ignorable letters such as U+3164, U+115F, and U+1160 remain accepted; reject default-ignorable code points or explicitly require a genuinely rendered character.
Useful? React with 👍 / 👎.
1 participant
Summary
Why
Service-site names are used in matching, storage, logging, and generated configuration paths. A targeted validation rule prevents malformed entries without rejecting legitimate service names or URLs already used by vote sources. Explicit generation results prevent callers from reporting success or opening an editor when no site was created.
Accepted format
Non-blank service-site names up to 2,048 characters. Normal URL punctuation is supported, including
:,/,?,=,&,%,+, and#. For example:https://list.example/vote?id=1&source=proxy#top.Square brackets, single and double ASCII quotes, backticks, backslashes, and invisible/control characters are rejected.
Validation