Implementation of labeled-response#511
Open
skizzerz wants to merge 11 commits intosolanum-ircd:mainfrom
Open
Conversation
Contributor
|
three bugs found by irctest: Missing echoed messages from ERR_UNKNOWNCOMMAND is missing |
This hook runs after parsing and processing a command, so that any cleanup that needs to happen after we're finished with parsing can happen. Some reorganization happened so that we ensure the hook and any tags processing happens for every event that isn't a parse error. This will be used by labeled-response in order to automatically send ACKs or close labeled-response BATCHes after processing client commands.
The new struct is ABI-compatible with hook_data for the first 3 fields so that modules compiled using the old hook definition will continue to work. The new data enables much richer evaluation during outbound_msgbuf hook functions by exposing the target or channel the message is being sent to (if known) as well as whether the message source is a recipient of the message being sent.
- new struct ResponseInfo that keeps track of all relevant state for a labeled-response (whether local or remote) - event to clean up pending remote labeled-responses that we haven't gotten responses for (perhaps due to a bad network link with the remote server, or it doesn't have labeled-response loaded) - struct Batch grew a new field to track its labeled-response status - struct Client grew a new linked list to track pending remote labeled-responses for it, so we can abort them easily if the client disconnects - struct ListClient grew a new field to track labeled-response status for a SAFELIST iteration - remove arbitrary size limit on batch id generation; it will now fill the passed-in buffer according to the passed-in buffer size, leaving room for the null byte at the end. NULL fields indicate that no repsonses are being labeled. The actual population of those fields will happen in future patches; this one simply provides the core support needed.
CLICAP_SERVONLY is a client capability that no clients will ever have (it is not associated with any CAP string), but is included in the server's client cap mask. Message tags with their capmask set to CLICAP_SERVONLY will only ever be propagated s2s and will never be sent or received c2s. This allows for the easy introduction of s2s-only message tags. Similarly, a new flag for client capabilities defined by modules was introduced name CLICAP_FLAGS_NOPROP which indicates that message tags having a capmask including such a capability MUST NOT be sent s2s. The CLIENT_CAP_MASK macro in send.c was updated to omit capabilities marked as CLICAP_FLAGS_NOPROP from what it will send s2s.
When allocate_batch_message is called on a batch containing valueless tags (i.e. tag value is NULL), the ircd would core because we attempted to call strlen on a NULL pointer. Properly detect NULL values and don't attempt to copy them. Additionally, hooks may set tag keys to be NULL to indicate that tag should be stripped; detect this as well and avoid copying those tags entirely in the newly-allocated batch message. This crash is currently not exploitable because allocate_batch_message() is only called for client-initiated batches and batches sent s2s, of which there are currently none.
Two new lines were added at the end of STATS t output to list the number of pending client-initiated batches (and pending lines in those batches) as well as the number of pending remote labeled-response batches.
This module provides the labeled-response capability as well as an internal non-propagated ?receive_label capability that is dynamically added/removed from the client which should receive a labeled-response. By using a second dummy capability, I don't need to rewrite our msgbuf caching layer which is keyed entirely off the capmasks of the client in order to vary who receives the label/batch tags. It also introduces a s2s-only message tag solanum.chat/response which is used to inform remote servers that their replies should be labelled. Remote responses are always batched, and the batch is generated by the local server to ensure that 100% of incoming labelled commands receive a labeled-response. The solanum.chat/response tag is passed back in replies so the local server can add the appropriate batch tag to the response passed to the client. Remote servers will also send an ENCAP ACK command to indicate they are done responding in the event that they received an incoming s2s command with the solanum.chat/response tag. The module keeps track of outgoing responses and if, by the end of command processing (parse_end hook), it hasn't attached a label to anything it will send a labelled ACK to the client. If it has begun a labeled-response batch, it will also automatically close that batch, reducing the likelihood of programming errors forgetting to close batches.
Add calls to generate local or remote labeled-response batches to all modules which generate multiple lines of response. In general, this is only done once we've confirmed we will be sending multiple lines, to avoid spurious batches which only contain single lines inside of the batch, however some cases will always generate a batch regardless due to ease of implementation. This patch covers all of the "easy" cases; future patches will cover the complicated ones: m_batch, m_message, and m_list.
This concerns client-initiated batches only. Save the labeled-response context as part of the Batch so that we can restore that context later once the batch is complete. The label is added to the batch start message, but we only process batches once we receive the batch complete message (which lacks a label tag), and we could potentially be finishing a batch due to it timing out if the client never sends a batch complete message. By restoring context, we ensure that hooks such as outbound_msgbuf are operating with all of the state they would expect to have in order to properly attach tags to outgoing messages.
With SAFELIST, the labeled-response batch does not necessarily finish right away. As such, save off the context for the client's LIST command and restore it each time we send them more data.
While base cases already worked fine with just cap_labeled_response, a handful of special cases need batched responses, such as multi-target messages. The module also needed updating to work properly with echo-message, as the spec says that the echo is the one that needs the label if the client messages themselves, rather than the main message, and the echo is always sent last. This logic is also applied to mass messages that include the oper sending it. The handle_special method was broken up and made less special, treating the various cases as additional target types rather than processing them right away. The opers@server hack was also removed entirely since it is obviated by other means of messaging opers on a server such as LOCOPS in the event such is even necessary. Or, you know, just use the staff channel.
skizzerz
force-pushed
the
labeled-response
branch
from
bf3610b to
6cbadb3
Compare
skizzerz
changed the title
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
2 participants
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.
This was broken into multiple commits for easier review as there are a number of distinct things changed.