Skip to content

Ignore secret values in ChatThrottleLib hooks - #26

Merged
Nevcairiel merged 2 commits into
WoWUIDev:masterfrom
Nnoggie:fix/chat-throttle-secret-values
Aug 5, 2026
Merged

Ignore secret values in ChatThrottleLib hooks#26
Nevcairiel merged 2 commits into
WoWUIDev:masterfrom
Nnoggie:fix/chat-throttle-secret-values

Conversation

@Nnoggie

@Nnoggie Nnoggie commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Skip bandwidth accounting when chat hooks receive secret text or destinations.
  • Avoid accessing chat values while aura secrets are active.
  • Keep Classic and older clients working when secret APIs are unavailable.
  • Bump ChatThrottleLib to version 32.

Why

ChatThrottleLib's secure hooks convert message text and destination to strings to estimate bandwidth. In restricted encounters, either value can be secret, and converting it raises an error. Ignoring those hook calls prevents the error without changing queued message delivery.

@Nnoggie
Nnoggie marked this pull request as ready for review July 16, 2026 20:30
@Nevcairiel

Copy link
Copy Markdown
Member

What exactly is the purpose of the ShouldAurasBeSecret call? All we care about is checking if the values are secret and thus strlen fails, nothing else.

@Nnoggie

Nnoggie commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

Yeah this was a copy paste leftover from a similar fix for another AddOn. Will slim down and check for the existence of issecretvalue as well.

@Nnoggie
Nnoggie force-pushed the fix/chat-throttle-secret-values branch from ecd3ae0 to f0675fc Compare July 16, 2026 21:13
@funkydude

Copy link
Copy Markdown
Collaborator

You'll need to add issecretvalue to the .luacheckrc file so this PR passes the checks.

@SpiritOfSilvermoon

Copy link
Copy Markdown

A few remarks ... :

I recently had a report of what appeared to be this issue from someone using an addon that I maintain (apparently it's a 100% repro when whispering while in a PVP instance as long as any addons that use CTL are active, if you need a real-world test case; I'm a bit surprised at the apparent scarcity of reports given how broadly CTL is used).

Semantically, it seems better to use canaccessvalue and not issecretvalue. In practice, I'm not aware of there being any difference for tainted code, but the semantics would be incorrect to use issecretvalue were the code untainted (of course no third party addon code is untainted).

if not canaccessallvalues(text, destination) then ... could be used to test both values in one call.

The change to ChatThrottleLib.Hook_SendAddonMessage appears superfluous to me, in that the documentation metadata says that even untainted code can't pass secrets to C_ChatInfo.SendAddonMessage. But that part of the change is probably not harmful, if it remains unnecessary.

Are there any plans to merge this fix as version 32? I'd like to pick up a fix for the person that reported this problem, but I'm hesitant about private forks of versioned libraries like this one with uncoordinated version numbers.

@Nevcairiel
Nevcairiel merged commit d96aeae into WoWUIDev:master Aug 5, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants