Skip to content
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

Fixed [p]ban raising an unhandled error if an ID too large is provided #6486

Merged
merged 2 commits into from
Dec 24, 2024

Conversation

cdaman3141
Copy link
Contributor

Description of the changes

Closes #6431

Added ID validation within RawUserIdConverter, and adjusted Regex matching to fit standard Discord ID format. Also switched usage of regex.match to regex.fullmatch so IDs aren't truncated. Should no longer give error in console when using [p]ban with an ID that is too large.

Have the changes in this PR been tested?

Yes

…ching to fit standard Discord ID format. Also switched usage of regex.match to regex.fullmatch so IDs aren't truncated. Should no longer give error in console when using [p]ban with an ID that is too large
@github-actions github-actions bot added the Category: Core - API - Commands Package This is related to the `redbot.core.commands` package or `redbot.core.checks` module. label Dec 9, 2024
Copy link
Member

@Jackenmen Jackenmen left a comment

Choose a reason for hiding this comment

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

The max length of a snowflake is 20 characters, not 19. The max value is 2**64 - 1 (i.e. UINT64_MAX), not 2 ** 63 - 1 (which would be the max for a signed 64-bit integer).

Please put the max value as a private (prefixed with _) constant at the module-level and define it as 2 ** 64 - 1 rather than as a magic number.

@Jackenmen Jackenmen added the Type: Bug Unexpected behavior, result, or exception. In case of PRs, it is a fix for the foregoing. label Dec 24, 2024
@Jackenmen
Copy link
Member

Apparently, I need to read the traceback with the API response, not Discord's own documentation... 2 ** 63 - 1 is correct.

@Jackenmen Jackenmen added this to the 3.5.14 milestone Dec 24, 2024
Copy link
Member

@Jackenmen Jackenmen left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!

@Jackenmen Jackenmen enabled auto-merge (squash) December 24, 2024 01:11
@Jackenmen Jackenmen merged commit 1506925 into Cog-Creators:V3/develop Dec 24, 2024
16 checks passed
@red-githubbot red-githubbot bot added the Changelog Entry: Pending Changelog entry for this PR hasn't been added by repo maintainers yet. label Dec 24, 2024
@Jackenmen Jackenmen added Changelog Entry: Added Changelog entry for this PR has already been added to changelog PR. and removed Changelog Entry: Pending Changelog entry for this PR hasn't been added by repo maintainers yet. labels Dec 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Core - API - Commands Package This is related to the `redbot.core.commands` package or `redbot.core.checks` module. Changelog Entry: Added Changelog entry for this PR has already been added to changelog PR. Type: Bug Unexpected behavior, result, or exception. In case of PRs, it is a fix for the foregoing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Command raises an unhandled error if an ID too large is provided
2 participants