fix: webhook query string, inverted bounds checks and handle leaks - #8
Merged
Conversation
Include (discordWebhookAPI.inc):
- Execute(): the thread_id branch produced "&?wait=true", so the "wait"
query parameter was named "?wait" and Discord never returned the created
message body (200 -> 204), which also broke a subsequent Webhook.Edit().
- Execute(): skip the thread_id query parameter when thread_name is set to
avoid Discord error 220002 (a forum webhook cannot carry both).
- Embed.GetField() / Webhook.GetEmbed(): the bounds check was inverted
(array.Length < index), so every valid index returned null and out-of-range
indices read past the array. Use "index >= 0 && index < Length".
- AddField(), AddEmbed(), GetField(), GetEmbed(): free the JSONArray handle
that was leaked on every call.
- GetFooter/GetImage/GetThumbnail/GetVideo/GetProvider/GetAuthor/GetFields/
GetEmbeds: return null instead of raising a native error when the key is
not set.
- SetTimeStampNow(): drop the malformed "%FT\%T.000%z" format string.
- DEBUG path: this.toString -> this.ToString (did not compile) and pass the
JSON through a "%s" format instead of as the format string itself.
- Size the Execute()/Edit() URL buffers from WEBHOOK_URL_MAX_SIZE.
- SetThreadName(): take const char[]; fix the doc (Discord limit is 100).
- Add Webhook.GetThreadName(); bump version to 1.1.0.
example.sp:
- PrintToServer used %s with an int client argument -> use %d.
- Free the DataPack in both HTTP callbacks.
- PrintToServer(messageId) -> PrintToServer("%s", messageId).
README.md:
- Success check is HTTPStatus_OK (Execute() forces wait=true), delete the
webhook, drop the trailing slash, use discord.com.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
While reviewing the include I found a few real bugs (one of them breaks
Webhook.Edit()), some handle leaks, and a couple of rough edges inexample.sp/README.md. Everything here is behaviour-preserving except where the previous behaviour was broken.Bugs fixed
Webhook.Execute()built an invalid query stringThe
&?wait=truemakes the parameter name?waitinstead ofwait. Discord then treatswaitasfalse, replies204 No Contentinstead of200with the message body, andOnWebHookExecutedcan no longer read the messageid→Webhook.Edit()is unusable whenever a thread ID is passed. Fixed to&wait=true.Also: when
thread_nameis set, thethread_idquery parameter is now omitted, because Discord rejects a forum webhook that carries both (error220002). Previously onlyexample.spworked around this.Embed.GetField()/Webhook.GetEmbed()inverted bounds checkFor an array of length 3,
GetField(0)returnsnullandGetField(5)reads out of bounds. Every valid index fails. Changed toindex >= 0 && index < fields.Length.Handle leaks
AddField(),AddEmbed(),GetField()andGetEmbed()obtain aJSONArrayhandle (new JSONArray()orthis.Get(...), whichjson_increfs) and neverdeleteit. Added the missingdelete.Sub-object getters raised a native error
GetFooter,GetImage,GetThumbnail,GetVideo,GetProvider,GetAuthor,GetFields,GetEmbedscallthis.Get("<key>"), which throwsCould not retrieve value for keywhen the key is absent. They now returnnullfirst viaHasKey.Embed.SetTimeStampNow()malformed format string"%FT\%T.000%z"contains a stray escape (\%). Replaced with an explicit, portable"%Y-%m-%dT%H:%M:%S%z".DEBUGbuild paththis.toString(...)(wrong case, does not compile) →this.ToString(...), and the JSON is now printed withPrintToServer("%s", debug)instead of being used as the format string.Improvements
Execute()/Edit()are sized fromWEBHOOK_URL_MAX_SIZEinstead of a bare1024that a near-maximum-length webhook URL plus?thread_id=…&wait=truecould overflow.Webhook.SetThreadName()now takesconst char[](so string literals are accepted) and its doc reflects the real Discord limit of 100 characters (WEBHOOK_THREAD_NAME_MAX_SIZE), not 1000.Webhook.GetThreadName().DiscordWebhookAPI_VERSIONbumped to1.1.0+ changelog entry.example.spPrintToServer("… n°%s …", client)used%swith anint→%d.DataPackpassed toExecute()/Edit()was never freed in the callbacks.PrintToServer(messageId)→PrintToServer("%s", messageId).README.mdHTTPStatus_NoContent, butExecute()always appendswait=trueso Discord answers200 OK; alsodeletethe webhook, drop the trailing slash (the doc explicitly warns against it) and usediscord.com.Notes
/api/v10/is a behaviour change worth discussing separately — opened as an issue on the upstream repo (issues are disabled here).Testing
example.spstill compiles (CI).GetField/GetEmbed/ the sub-object getters are not exercised byexample.sp; changes there are straightforward.🤖 Generated with Claude Code