-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add validation to blocklists, test that adding doesn't break stuff #40
Conversation
where are blocklists from? is it possible they may include |
@@ -45,7 +58,8 @@ export const blockAllowListRoutes = (cfg: APIConfig, store: Store, apsystem: Act | |||
return await reply.code(403).send('Not Allowed') | |||
} | |||
|
|||
const accounts = request.body.split('\n') | |||
const accounts = request.body.trim().split('\n') |
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.
maybe .map(x => x.trim())
after the split? also remove empty lines
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.
I think empty lines should be an error.
the api client sends accounts separated by newlines to the endpoint. on the ruby client the method takes an array and concatenates items with a newline |
Co-authored-by: Nulo <[email protected]>
Yeah I'd like to avoid adding any fancy syntax like comments or empty lines and stick to valid user input. I added trim to account for newlines at the end messing stuff up since it'd be a likely error when importing a file. |
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.
closes #37
import Store from '../store/index.js' | ||
import ActivityPubSystem from '../apsystem.js' | ||
|
||
export function validateBlocklistFormat (mention: string): void { | ||
const sections = mention.split('@') |
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.
const isInstanceBlock = sections[1] === '*' && sections[0] === '' && sections[2] !== '';
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.
I don't think we need to explicitly handle this case since it's already handled by the other checks.
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.
I'm ok with this even if each item isn't trimmed
Related to #37 where there's an error being swallowed and leading to unexpected behavior.