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 empty tag-only message support and an increased size limit for tags #287

Merged
merged 25 commits into from Feb 16, 2017

Conversation

Projects
None yet
9 participants
@jwheare
Member

jwheare commented Dec 20, 2016

  • Specs a TAGMSG tag-only event. Enables things like content-less reactions/faving, message deletion, message seen notifications (ircv3/ircv3-ideas#1)
  • Increases the size limit for tags to 4096
  • Cap name version bump to draft/message-tags-0.2

@jwheare jwheare added this to the Roadmap milestone Dec 20, 2016

Show outdated Hide outdated core/message-tags-3.3.md
Show outdated Hide outdated core/message-tags-3.3.md
@jwheare

This comment has been minimized.

Show comment
Hide comment
@jwheare

jwheare Jan 1, 2017

Member

Any more input on this?

Member

jwheare commented Jan 1, 2017

Any more input on this?

@attilamolnar

This comment has been minimized.

Show comment
Hide comment
@attilamolnar

attilamolnar Jan 2, 2017

Member

The version of the draft cap should be bumped so early implementations can discover if a server is capable of empty PRIVMSG support

Member

attilamolnar commented Jan 2, 2017

The version of the draft cap should be bumped so early implementations can discover if a server is capable of empty PRIVMSG support

@DarthGandalf

This comment has been minimized.

Show comment
Hide comment
@DarthGandalf

DarthGandalf Jan 2, 2017

Member

Or even :#channel

Member

DarthGandalf commented on core/message-tags-3.3.md in 3a249a7 Jan 2, 2017

Or even :#channel

This comment has been minimized.

Show comment
Hide comment
@grawity

grawity Jan 2, 2017

Contributor

That's already covered by the core protocol; also, no need to make people accidentally think you can have spaces in that parameter.

Contributor

grawity replied Jan 2, 2017

That's already covered by the core protocol; also, no need to make people accidentally think you can have spaces in that parameter.

Show outdated Hide outdated core/message-tags-3.3.md
Show outdated Hide outdated core/message-tags-3.3.md
Show outdated Hide outdated core/message-tags-3.3.md
@attilamolnar

LGTM

@ProgVal

This comment has been minimized.

Show comment
Hide comment
@ProgVal

ProgVal Jan 10, 2017

Contributor

Enables things like content-less reactions/faving, message deletion, message seen notifications

Why do you want to use tags instead of a verb for these?
They clearly are actions, as much as AWAY, MODE, TOPIC, etc.

Contributor

ProgVal commented Jan 10, 2017

Enables things like content-less reactions/faving, message deletion, message seen notifications

Why do you want to use tags instead of a verb for these?
They clearly are actions, as much as AWAY, MODE, TOPIC, etc.

@attilamolnar

This comment has been minimized.

Show comment
Hide comment
@attilamolnar

attilamolnar Jan 10, 2017

Member

Why do you want to use tags instead of a verb for these?
They clearly are actions, as much as AWAY, MODE, TOPIC, etc.

So features like what you quoted won't depend on server updates once client to client tags are supported.

Member

attilamolnar commented Jan 10, 2017

Why do you want to use tags instead of a verb for these?
They clearly are actions, as much as AWAY, MODE, TOPIC, etc.

So features like what you quoted won't depend on server updates once client to client tags are supported.

@ProgVal

This comment has been minimized.

Show comment
Hide comment
@ProgVal

ProgVal Jan 10, 2017

Contributor

Message deletion and seen notifications will have to be supported by the server, for #292, and maybe faving too.
It may make sense for reactions to always be attached to a PRIVMSG, since they are a message; but it would have to be discussed in a separate spec.

I'm worried about the two possibilities discussed here, because not having a command would break one the most basic conventions of IRC, and using a content-less PRIVMSG would strip PRIVMSG of its meaning (why PRIVMSG and not NOTICE or MODE?).

From a less philosophical point of view, I am worried that using an empty PRIVMSG would either break third-party plugins of my bot which do not handle that case (if the bot negotiates the capability) or prevent other plugins from using the empty PRIVMSG from usin the feature (if the bot does not negotiate it).
And not using a command would break any third-party plugin that assumes the parser outputs a command.

What would you think of adding new verb(s?) for the sole purpose of carrying tags, on a channel, user-to-user, or from a network admin to all the users?

Contributor

ProgVal commented Jan 10, 2017

Message deletion and seen notifications will have to be supported by the server, for #292, and maybe faving too.
It may make sense for reactions to always be attached to a PRIVMSG, since they are a message; but it would have to be discussed in a separate spec.

I'm worried about the two possibilities discussed here, because not having a command would break one the most basic conventions of IRC, and using a content-less PRIVMSG would strip PRIVMSG of its meaning (why PRIVMSG and not NOTICE or MODE?).

From a less philosophical point of view, I am worried that using an empty PRIVMSG would either break third-party plugins of my bot which do not handle that case (if the bot negotiates the capability) or prevent other plugins from using the empty PRIVMSG from usin the feature (if the bot does not negotiate it).
And not using a command would break any third-party plugin that assumes the parser outputs a command.

What would you think of adding new verb(s?) for the sole purpose of carrying tags, on a channel, user-to-user, or from a network admin to all the users?

@jwheare

This comment has been minimized.

Show comment
Hide comment
@jwheare

jwheare Jan 10, 2017

Member
  1. Can you explain why deletion/seen/faving would require server support? The chathistory can just store the empty messages and clients can replay them as normal.

  2. What does this mean?

not having a command would break one the most basic conventions of IRC

  1. If you're concerned, your bot plugin framework could require plugins to enable support for the CAP via some internal config, and then you can forward/filter them to individual plugins as necessary.

  2. Reasons for not adding a new verb have been discussed a few times already in this thread (search in page for "METAMSG")

Member

jwheare commented Jan 10, 2017

  1. Can you explain why deletion/seen/faving would require server support? The chathistory can just store the empty messages and clients can replay them as normal.

  2. What does this mean?

not having a command would break one the most basic conventions of IRC

  1. If you're concerned, your bot plugin framework could require plugins to enable support for the CAP via some internal config, and then you can forward/filter them to individual plugins as necessary.

  2. Reasons for not adding a new verb have been discussed a few times already in this thread (search in page for "METAMSG")

@ProgVal

This comment has been minimized.

Show comment
Hide comment
@ProgVal

ProgVal Jan 10, 2017

Contributor
Contributor

ProgVal commented Jan 10, 2017

@attilamolnar

This comment has been minimized.

Show comment
Hide comment
@attilamolnar

attilamolnar Jan 10, 2017

Member

@ProgVal I don't understand the issue. The bot can have two events, one for regular PRIVMSG the other one for tag only PRIVMSG. Pass regular PRIVMSGs to the first, tag only PRIVMSGs to the second. Exactly as you would do with a third verb. Did I miss something?

Member

attilamolnar commented Jan 10, 2017

@ProgVal I don't understand the issue. The bot can have two events, one for regular PRIVMSG the other one for tag only PRIVMSG. Pass regular PRIVMSGs to the first, tag only PRIVMSGs to the second. Exactly as you would do with a third verb. Did I miss something?

@ProgVal

This comment has been minimized.

Show comment
Hide comment
@ProgVal

ProgVal Jan 10, 2017

Contributor

Yes, I could do that. I am not sure how I would name them, because all callbacks have the name of the command, so I can't have two callbacks for the same command with this naming scheme. That's why I would prefer a new verb.

Contributor

ProgVal commented Jan 10, 2017

Yes, I could do that. I am not sure how I would name them, because all callbacks have the name of the command, so I can't have two callbacks for the same command with this naming scheme. That's why I would prefer a new verb.

@jwheare

This comment has been minimized.

Show comment
Hide comment
@jwheare

jwheare Jan 10, 2017

Member

There isn't a deletion spec at the moment, so this is slightly off-topic speculation. If one is proposed that uses client tags it would have to be an honour system. Servers would probably be allowed to delete them from chanhistory, etc, but it wouldn't be enforced. Given it's not possible to delete people's logs remotely, I don't see how any deletion system that doesn't rely on honour would work. The spec would have to caveat that strongly, that it wouldn't be a way to completely redact sensitive data, just allow honourable clients to mitigate the damage, or if nothing else, just a way to clean up cosmetic double posting etc, same with editing.


Re: naming schemes, do you have a separate callback for /me ACTION messages or do all plugins have to parse the CTCP from a PRIVMSG themselves?

In the IRCCloud API, we have PRIVMSG -> buffer_msg, NOTICE -> notice, ACTION -> buffer_me_msg, and empty tag messages -> empty_message. Our mobile apps don't yet support empty_message and so safely ignore them.

The motivation and scope for using empty existing verbs is to make integration at the IRC protocol level simple for a wide range of implementors. Since you have your own API interface, you're well placed to create your own abstractions that are more suited to your specific domain.


Don't servers already factor code for PRIVMSG and NOTICE? If yes, adding a third verb routed exactly like them shouldn't be a problem

It would potentially be a lot of extra clauses in a lot of different places. Not just routing but as mentioned previously "callerid, reg only speak, flood checks, mute" just for it to work. Those cases might need to be reviewed to be tag aware if that becomes an issue in light of new specs, but for base compatibility with this spec, pretty much only the ERR_NOTEXTTOSEND check is absolutely necessary for a server.

Member

jwheare commented Jan 10, 2017

There isn't a deletion spec at the moment, so this is slightly off-topic speculation. If one is proposed that uses client tags it would have to be an honour system. Servers would probably be allowed to delete them from chanhistory, etc, but it wouldn't be enforced. Given it's not possible to delete people's logs remotely, I don't see how any deletion system that doesn't rely on honour would work. The spec would have to caveat that strongly, that it wouldn't be a way to completely redact sensitive data, just allow honourable clients to mitigate the damage, or if nothing else, just a way to clean up cosmetic double posting etc, same with editing.


Re: naming schemes, do you have a separate callback for /me ACTION messages or do all plugins have to parse the CTCP from a PRIVMSG themselves?

In the IRCCloud API, we have PRIVMSG -> buffer_msg, NOTICE -> notice, ACTION -> buffer_me_msg, and empty tag messages -> empty_message. Our mobile apps don't yet support empty_message and so safely ignore them.

The motivation and scope for using empty existing verbs is to make integration at the IRC protocol level simple for a wide range of implementors. Since you have your own API interface, you're well placed to create your own abstractions that are more suited to your specific domain.


Don't servers already factor code for PRIVMSG and NOTICE? If yes, adding a third verb routed exactly like them shouldn't be a problem

It would potentially be a lot of extra clauses in a lot of different places. Not just routing but as mentioned previously "callerid, reg only speak, flood checks, mute" just for it to work. Those cases might need to be reviewed to be tag aware if that becomes an issue in light of new specs, but for base compatibility with this spec, pretty much only the ERR_NOTEXTTOSEND check is absolutely necessary for a server.

@ProgVal

This comment has been minimized.

Show comment
Hide comment
@ProgVal

ProgVal Jan 10, 2017

Contributor

Re: naming schemes, do you have a separate callback for /me ACTION messages or do all plugins have to parse the CTCP from a PRIVMSG themselves?

There is no separate callback, but they can call a function to know whether it's a CTCP.

Since you have your own API interface, you're well placed to create your own abstractions that are more suited to your specific domain.

For now, someone who knows the IRC commands can guess the name of the callback without looking at the doc or the code.
Adding the kind of abstraction you suggest would require a new name for this callback that cannot be guessed without looking at the doc.

It would potentially be a lot of extra clauses in a lot of different places. Not just routing but as mentioned previously "callerid, reg only speak, flood checks, mute" just for it to work. Those cases might need to be reviewed to be tag aware if that becomes an issue in light of new specs, but for base compatibility with this spec, pretty much only the ERR_NOTEXTTOSEND check is absolutely necessary for a server.

Can't you just grep for PRIVMSG and NOTICE and replace all references to them by calls common functions that can handle the three verbs?

Contributor

ProgVal commented Jan 10, 2017

Re: naming schemes, do you have a separate callback for /me ACTION messages or do all plugins have to parse the CTCP from a PRIVMSG themselves?

There is no separate callback, but they can call a function to know whether it's a CTCP.

Since you have your own API interface, you're well placed to create your own abstractions that are more suited to your specific domain.

For now, someone who knows the IRC commands can guess the name of the callback without looking at the doc or the code.
Adding the kind of abstraction you suggest would require a new name for this callback that cannot be guessed without looking at the doc.

It would potentially be a lot of extra clauses in a lot of different places. Not just routing but as mentioned previously "callerid, reg only speak, flood checks, mute" just for it to work. Those cases might need to be reviewed to be tag aware if that becomes an issue in light of new specs, but for base compatibility with this spec, pretty much only the ERR_NOTEXTTOSEND check is absolutely necessary for a server.

Can't you just grep for PRIVMSG and NOTICE and replace all references to them by calls common functions that can handle the three verbs?

@jwheare

This comment has been minimized.

Show comment
Hide comment
@jwheare

jwheare Jan 10, 2017

Member

I don't think ircv3 can commit to only speccing features that are guessable without docs.

Member

jwheare commented Jan 10, 2017

I don't think ircv3 can commit to only speccing features that are guessable without docs.

@ProgVal

This comment has been minimized.

Show comment
Hide comment
@ProgVal

ProgVal Jan 10, 2017

Contributor

My point is that content-less PRIVMSG complicates client APIs.

So from my point of view, METAMSG vs content-less PRIVMSG is: simplicity of client APIs (+ consistency of the semantic of PRIVMSG) vs less code to adapt for existing servers

Contributor

ProgVal commented Jan 10, 2017

My point is that content-less PRIVMSG complicates client APIs.

So from my point of view, METAMSG vs content-less PRIVMSG is: simplicity of client APIs (+ consistency of the semantic of PRIVMSG) vs less code to adapt for existing servers

@jwheare

This comment has been minimized.

Show comment
Hide comment
@jwheare

jwheare Jan 10, 2017

Member

I suppose my point is, some complexity will always exist either way. And it won't necessarily be evenly distributed. I also think it's easy to make assumptions about what will and won't be complex for different software to implement.

Luckily, as this is a draft that relies on implementation before being ratified, we don't need to assume. So for now we should go with majority consensus, and after we've had more implementations we can discuss it more.

Member

jwheare commented Jan 10, 2017

I suppose my point is, some complexity will always exist either way. And it won't necessarily be evenly distributed. I also think it's easy to make assumptions about what will and won't be complex for different software to implement.

Luckily, as this is a draft that relies on implementation before being ratified, we don't need to assume. So for now we should go with majority consensus, and after we've had more implementations we can discuss it more.

@DarthGandalf

This comment has been minimized.

Show comment
Hide comment
@DarthGandalf

DarthGandalf Jan 11, 2017

Member

I would much prefer METAMSG, for the same reasons as @ProgVal but I can accept empty PRIVMSG instead if really needed.

Member

DarthGandalf commented Jan 11, 2017

I would much prefer METAMSG, for the same reasons as @ProgVal but I can accept empty PRIVMSG instead if really needed.

@jwheare jwheare referenced this pull request Jan 11, 2017

Merged

Reaction client tag #289

@jwheare

This comment has been minimized.

Show comment
Hide comment
@jwheare

jwheare Jan 13, 2017

Member

After finding a few bugs in my server implementation where I was sending empty PRIVMSGs to other clients that didn't have the cap enabled, I'm going to try out an implementation using a different verb: TAGMSG.

Member

jwheare commented Jan 13, 2017

After finding a few bugs in my server implementation where I was sending empty PRIVMSGs to other clients that didn't have the cap enabled, I'm going to try out an implementation using a different verb: TAGMSG.

@jwheare

This comment has been minimized.

Show comment
Hide comment
@jwheare

jwheare Jan 13, 2017

Member

I've implemented this and I now agree it's much better and less error prone.

Member

jwheare commented Jan 13, 2017

I've implemented this and I now agree it's much better and less error prone.

@jwheare jwheare requested a review from attilamolnar Jan 13, 2017

@MuffinMedic

This comment has been minimized.

Show comment
Hide comment
@MuffinMedic

MuffinMedic Jan 13, 2017

Contributor

For naming purposes, is this new verb intended to be used only for sending tags without any user-text (TAGMSG) or any instance where we might want to send metadata (METAMSG)?

(If no use cases exist where metadata is sent outside of tags, then this becomes a moot point.)

Contributor

MuffinMedic commented Jan 13, 2017

For naming purposes, is this new verb intended to be used only for sending tags without any user-text (TAGMSG) or any instance where we might want to send metadata (METAMSG)?

(If no use cases exist where metadata is sent outside of tags, then this becomes a moot point.)

@DanielOaks

This comment has been minimized.

Show comment
Hide comment
@DanielOaks

DanielOaks Jan 13, 2017

Member

Metadata's handled with its' own command(s), so that shouldn't be an issue for this verb.

Member

DanielOaks commented Jan 13, 2017

Metadata's handled with its' own command(s), so that shouldn't be an issue for this verb.

@jwheare

This comment has been minimized.

Show comment
Hide comment
@jwheare

jwheare Jan 16, 2017

Member

Barring no other objections, I'll merge this tomorrow.

Member

jwheare commented Jan 16, 2017

Barring no other objections, I'll merge this tomorrow.

Show outdated Hide outdated core/message-tags-3.3.md
Show outdated Hide outdated core/message-tags-3.3.md
@jwheare

This comment has been minimized.

Show comment
Hide comment
@jwheare

jwheare Jan 19, 2017

Member

Added some examples and links. If there are no further comments, I'll merge this on Monday.

Member

jwheare commented Jan 19, 2017

Added some examples and links. If there are no further comments, I'll merge this on Monday.

@jwheare

This comment has been minimized.

Show comment
Hide comment
@jwheare

jwheare Jan 24, 2017

Member

@attilamolnar happy to approve these changes?

Member

jwheare commented Jan 24, 2017

@attilamolnar happy to approve these changes?

@attilamolnar

This comment has been minimized.

Show comment
Hide comment
@attilamolnar

attilamolnar Jan 25, 2017

Member

@jwheare Please add 2 more examples with TAGMSG interacting with labeled-response and echo-message in the normal msg case and in the self message case for complete clarity

Edit: there is already an example for echo-message, never mind that part of my comment.

Member

attilamolnar commented Jan 25, 2017

@jwheare Please add 2 more examples with TAGMSG interacting with labeled-response and echo-message in the normal msg case and in the self message case for complete clarity

Edit: there is already an example for echo-message, never mind that part of my comment.

@jwheare

This comment has been minimized.

Show comment
Hide comment
@jwheare

jwheare Jan 25, 2017

Member

Is a repetition of the label/echo/self messages examples really needed here? The labeled-response spec is not specific to PRIVMSG/NOTICE, but should apply to all messages. I don't think we need to document it on every new event we define.

Member

jwheare commented Jan 25, 2017

Is a repetition of the label/echo/self messages examples really needed here? The labeled-response spec is not specific to PRIVMSG/NOTICE, but should apply to all messages. I don't think we need to document it on every new event we define.

@jwheare

This comment has been minimized.

Show comment
Hide comment
@jwheare

jwheare Jan 25, 2017

Member

I added a @label tag to the echo-message example, but I don't think the obscure and hard to describe self-message corner case needs repeating here.

Member

jwheare commented Jan 25, 2017

I added a @label tag to the echo-message example, but I don't think the obscure and hard to describe self-message corner case needs repeating here.

@jwheare

This comment has been minimized.

Show comment
Hide comment
@jwheare

jwheare Jan 28, 2017

Member

TODO

  • Add error numeric for tag length limit exceeded.
  • Swap "event" for "command"
Member

jwheare commented Jan 28, 2017

TODO

  • Add error numeric for tag length limit exceeded.
  • Swap "event" for "command"
@jwheare

This comment has been minimized.

Show comment
Hide comment
@jwheare

jwheare Jan 28, 2017

Member
  1. The word "command" and "message" are used somewhat interchangeably in the RFCs ("event" is not). The distinction seems to be that a "command" refers to the numeric/verb itself or the act of issuing or processing it, while a "message" refers to the complete message with prefix and parameters and the act of delivering it. Somewhat subtle but I tried to use them appropriately.

  2. I changed the link to 2812 for PRIVMSG examples because it has a better definition of what a <msgtarget> is.

  3. Added an explicit error numeric for tags that exceed the limit. ERR_INPUTTOOLONG 417 is used in ircu and derivatives for lines that exceed the traditional untagged 512 limit. I think it's much more robust to explicitly error on this case instead of having rules or undefined server behaviour around truncation, leading to arbitrary data loss.

Member

jwheare commented Jan 28, 2017

  1. The word "command" and "message" are used somewhat interchangeably in the RFCs ("event" is not). The distinction seems to be that a "command" refers to the numeric/verb itself or the act of issuing or processing it, while a "message" refers to the complete message with prefix and parameters and the act of delivering it. Somewhat subtle but I tried to use them appropriately.

  2. I changed the link to 2812 for PRIVMSG examples because it has a better definition of what a <msgtarget> is.

  3. Added an explicit error numeric for tags that exceed the limit. ERR_INPUTTOOLONG 417 is used in ircu and derivatives for lines that exceed the traditional untagged 512 limit. I think it's much more robust to explicitly error on this case instead of having rules or undefined server behaviour around truncation, leading to arbitrary data loss.

@dequis

This comment has been minimized.

Show comment
Hide comment
@dequis

dequis Jan 28, 2017

Contributor

Added an explicit error numeric for tags that exceed the limit. ERR_INPUTTOOLONG 417 is used in ircu and derivatives for lines that exceed the traditional untagged 512 limit. I think it's much more robust to explicitly error on this case instead of having rules or undefined server behaviour around truncation, leading to arbitrary data loss.

For a sec I thought this was #281, where requiring that numeric may or may not happen. But it seems uncontroversial in the context of the new message-tags, with all its new required behaviors, and particularly about exceeding the tag length limit. So yeah, sure.

I think @DanielOaks was interested in reviewing the usage of that numeric first.

Contributor

dequis commented Jan 28, 2017

Added an explicit error numeric for tags that exceed the limit. ERR_INPUTTOOLONG 417 is used in ircu and derivatives for lines that exceed the traditional untagged 512 limit. I think it's much more robust to explicitly error on this case instead of having rules or undefined server behaviour around truncation, leading to arbitrary data loss.

For a sec I thought this was #281, where requiring that numeric may or may not happen. But it seems uncontroversial in the context of the new message-tags, with all its new required behaviors, and particularly about exceeding the tag length limit. So yeah, sure.

I think @DanielOaks was interested in reviewing the usage of that numeric first.

@jwheare

This comment has been minimized.

Show comment
Hide comment
@jwheare

jwheare Jan 31, 2017

Member

I've discovered a slightly edge casey issue while implementing the ERR_INPUTTOOLONG error. A client might send a message that comes in under the limit, but the server then adds tags that push it over the limit.

The server could respond with the numeric anyway, but there are a few problems:

  1. The server needs to precalculate the length of any tags it might add. If limit checking happens at the raw input parsing stage, this adds complexity further down the process.
  2. The server can't wait until it has generated the full response to detect this and convert to an error, because side effects like forwarding the message to other clients will have already happened.
  3. But, there may theoretically be tags introduced in future that mean it's not always possible to know the length of all the tags until those messages have been delivered, or at least checked, creating more complexity. e.g. (off the top of my head) if we introduce a tag that indicates how many targets received the message, or if different recipients get different tag sets.

Because of this, we have a situation where tags might again need to be truncated unexpectedly.

One solution is to allow for truncation in the spec, perhaps adding a @truncated=<length> tag at output processing time to let clients know what happened. This would of course result in even more truncation. It also weakens the point of having an error numeric in the first place.

Another idea is that we separate the limits. So, for example the total limit could be 4608, with 512 reserved for server tags and 4096 for client tags. It allows input parsing to remain simpler, safe and at the edges for both client and server with no unexpected data loss due to truncation.

Any issues with that? Also, any thoughts on the numbers? Perhaps we still need more than 512 for server tags, but client tags are the main motivation for increasing the limit.

Member

jwheare commented Jan 31, 2017

I've discovered a slightly edge casey issue while implementing the ERR_INPUTTOOLONG error. A client might send a message that comes in under the limit, but the server then adds tags that push it over the limit.

The server could respond with the numeric anyway, but there are a few problems:

  1. The server needs to precalculate the length of any tags it might add. If limit checking happens at the raw input parsing stage, this adds complexity further down the process.
  2. The server can't wait until it has generated the full response to detect this and convert to an error, because side effects like forwarding the message to other clients will have already happened.
  3. But, there may theoretically be tags introduced in future that mean it's not always possible to know the length of all the tags until those messages have been delivered, or at least checked, creating more complexity. e.g. (off the top of my head) if we introduce a tag that indicates how many targets received the message, or if different recipients get different tag sets.

Because of this, we have a situation where tags might again need to be truncated unexpectedly.

One solution is to allow for truncation in the spec, perhaps adding a @truncated=<length> tag at output processing time to let clients know what happened. This would of course result in even more truncation. It also weakens the point of having an error numeric in the first place.

Another idea is that we separate the limits. So, for example the total limit could be 4608, with 512 reserved for server tags and 4096 for client tags. It allows input parsing to remain simpler, safe and at the edges for both client and server with no unexpected data loss due to truncation.

Any issues with that? Also, any thoughts on the numbers? Perhaps we still need more than 512 for server tags, but client tags are the main motivation for increasing the limit.

@jwheare

This comment has been minimized.

Show comment
Hide comment
@jwheare

jwheare Feb 11, 2017

Member

Added the split limit details in dba7cd5

Member

jwheare commented Feb 11, 2017

Added the split limit details in dba7cd5

@jwheare jwheare merged commit c7ab1c4 into ircv3:master Feb 16, 2017

@jwheare jwheare deleted the jwheare:message-tags-empty+size branch Feb 16, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment