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

strengthen labeled-response guarantee #485

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

slingamn
Copy link
Contributor

This follows up from #422. Here's my understanding of the state of play there:

  1. The current "best-effort" language is very weak and allows for various forms of malicious compliance by servers, for example, labeling only ACKs.
  2. At the same time, it is either not feasible or not desirable for most servers to guarantee that all responses MUST be labeled.

This change does the following:

  1. Strengthens the labeled-response guarantee to require labeling in the absence of "exceptional conditions". This should rule out the possibility of compliant implementations that routinely fail to label their responses. The objective here is to give client developers (in a greenfield setting where the server is guaranteed to implement labeled-response) the confidence to remove fallback routing of unlabeled responses.
  2. Describes explicitly (in a non-normative section) the possibility of such a client discarding/ignoring unlabeled responses.

From discussion, this would be considered an erratum and it would be necessary to confirm that existing server implementations comply with the strengthened guarantee.

@emersion
Copy link
Contributor

As a client dev, "exceptional" sounds a lot more reliable than "when feasible". "exceptional" makes it clear that a hard timeout for the pathological cases is enough.

Additionally, without this patch it is tempting to add labeled-response support to a bouncer even if the upstream server doesn't support it. The bouncer could have the hardcoded "if I sent a JOIN, the response will be JOIN or ERR_NOSUCHCHANNEL or…" logic and manually insert labels on replies. This doesn't work for echo-message though, and this new wording makes it clear it's a bad idea.

tl;dr +1 from me!

@slingamn
Copy link
Contributor Author

slingamn commented Dec 15, 2021

To clarify what's at stake here, we'd want to go through the current support table, which lists (for servers):

  • Ergo
  • IRCCloud Teams
  • InspIRCd
  • UnrealIRCd

and verify that there is no routine circumstance under which they do not label a response. For example, it would be a problem if remote WHOIS is never labeled.

@slingamn
Copy link
Contributor Author

From discussion with InspIRCd, the stronger version of the guarantee is not supported there, and cannot be for architectural reasons. So this goes back to an approach similar to #422 (adding a new flag for advertising the stricter behavior). However, the guarantee here is weaker than it was on #422; there it was an unconditional MUST, here's it's a MUST modulo "exceptional runtime conditions".

@SadieCat
Copy link
Contributor

This feels like a solution in search of a problem. Have any existing client implementations actually asked for this?

@slingamn
Copy link
Contributor Author

This feels like a solution in search of a problem. Have any existing client implementations actually asked for this?

I think this reverses the order of things. Existing client implementations don't need this --- they've already built routing logic for numerics that works around the lack of labeled-response. The objective here is to make possible a greenfield IRCv3 client that relies on labeled-response because it doesn't have those workarounds.

@SadieCat
Copy link
Contributor

SadieCat commented Dec 17, 2021

Those implementations will still have to deal with the reality that:

  1. Legacy servers not supporting labelled responses exist and will always exist.
  2. All but a few niche single-node implementations like Ergo can not make this strictness guarantee.

@kylef
Copy link
Contributor

kylef commented Dec 26, 2021

I wonder if an alternative might suffice, where we have an explicit labeled response messages which indicates labeled replies are unsupported for this request. Instead of indicating the implementation is strict, we instead indicate when a message won't get a labeled response clearly and consistently for clients.

C: @label=a WHOIS doe
S: @label=a FAIL ... :Labeled response for this request is not supported.

S: .. an unlabeled WHOIS response ..

Potentially, there could be room for another failure case where we started sending the labeled response but then an error condition occured such as a netsplit

C: @label=a WHOIS doe
S: @label=a ... some whois numerics (not RPL END)
S: @label=a FAIL ... :some explanation of the error condition

The benefit here, is sometimes the server implementation may be able to provide labeled response (local whois, or no such nick, error cases etc).

Ultimately, I think the problem from a client perspective is the lack of uncertainty in the response to labeled requests. Having a strict mode combats knowing the response will always be strict in a subset of servers, but does not solve the complexity of many different fallback branches for different servers that implement differently. Having a clear mechanism in place to expose a reply won't be labeled seems more desirable to me.

@slingamn
Copy link
Contributor Author

slingamn commented Jan 2, 2022

This doesn't really address my main objective for this (making greenfield clients with no fallback routing logic possible).

@progval
Copy link
Contributor

progval commented Aug 27, 2023

@SadieCat So in your opinion, at worst this spec change is at worst useless, right? So, as some other people find it useful, we should probably go forward with this change anyway.

@spb
Copy link

spb commented Aug 29, 2023

This seems to be a no brainer - it doesn't invalidate any existing implementations but still provides a way forward for clients that want to work with servers which can provide the strict behaviour.

@jwheare
Copy link
Member

jwheare commented Sep 1, 2023

I feel that by default the labeled-response spec should be stricter and specific about requiring labels on echo-message. This was a primary motivation for the spec after all. I'd support something along the lines of #528 but on the label spec instead of echo.

I have yet to hear a convincing concrete example of how a "full strict" guarantee would help clients. The main argument seems to be for green field clients that have no intention of supporting existing servers that don't support label at all. This feels pretty antithetical to the backwards compatibility ethos of IRCv3 so I don't think it makes sense to enshrine in the spec, even as an optional cap value.

@slingamn
Copy link
Contributor Author

The main argument seems to be for green field clients that have no intention of supporting existing servers that don't support label at all. This feels pretty antithetical to the backwards compatibility ethos of IRCv3 so I don't think it makes sense to enshrine in the spec, even as an optional cap value.

Next-generation mobile clients (e.g. Goguma) already effectively require a modern server that supports CHATHISTORY. I understand the requirement of strict backwards compatibility to apply to servers rather than clients. I have always seen the goal of building a greenfield modern client that explicitly requires a modern server as valid.

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.

None yet

7 participants