Skip to content

Channel renaming extension#420

Merged
jwheare merged 22 commits intoircv3:masterfrom
jwheare:channel-rename
Sep 2, 2020
Merged

Channel renaming extension#420
jwheare merged 22 commits intoircv3:masterfrom
jwheare:channel-rename

Conversation

@jwheare
Copy link
Copy Markdown
Member

@jwheare jwheare commented Jun 8, 2020

This PR replaces and builds on #308

Changes from the earlier PR so far:

  • Drop MODE message from fallback requirements
  • Switch to standard replies FAIL codes for new error cases
  • Existing error numerics such as ERR_CHANOPRIVSNEEDED are preferred
  • Move normative fallback mechanism to its own section and add an example
  • Mention case changes explicitly
  • Don't use RFC keywords in non-normative security section

Decisions that still need to finalised:

  • Command name. Is RENAME too generic? Should it be CHANRENAME? Counter-point, we use JOIN PART KICK, not JOINCHAN, etc.
  • Cap name. This should probably be changed to channel-rename

Decisions made (Edit 25/8/2020):

  • RENAME is fine
  • channel-rename will be the final (non-draft) cap name

sadiepowell and others added 5 commits June 8, 2020 11:22
* Switch to standard replies FAIL codes for new error cases
* Existing error numerics such as ERR_CHANOPRIVSNEEDED are preferred
* Move normative fallback mechanism to its own section and add an example
* Mention case changes explicitly
* Don't use RFC keywords in non-normative security section
@jwheare
Copy link
Copy Markdown
Member Author

jwheare commented Jul 30, 2020

Anyone have any opinions on the unresolved decisions mentioned here?

@sadiepowell
Copy link
Copy Markdown
Contributor

Command name. Is RENAME too generic? Should it be CHANRENAME? Counter-point, we use JOIN PART KICK, not JOINCHAN, etc.

I think its fine?

Cap name. This should probably be changed to channel-rename

Sounds good to me.

@dequis
Copy link
Copy Markdown
Contributor

dequis commented Jul 30, 2020

How illegal would it be if I also allowed renaming of users through this?

Bitlbee has this thing as a control channel command (not as a server command)

19:59 <@dx> help rename
19:59 <@root> Syntax: rename <oldnick> <newnick>
19:59 <@root> Syntax: rename -del <nick>
19:59 <@root>
19:59 <@root> Renick a user in your buddy list. Very useful, in fact just very important, if you got a lot of people with stupid account names (or hard ICQ numbers).
19:59 <@root>
19:59 <@root> rename -del can be used to erase your manually set nickname for a contact and reset it to what was automatically generated.
19:59 <@root>
19:59 <@root> Example:
19:59 <@root> <itsme> rename itsme_ you
19:59 <@root> * itsme_ is now known as you

Which doesn't support channels, but probably should some day. And the last time I saw this spec I was thinking that my implementation of this command could just quietly accept nicks in addition to channels, and do what the other rename command normally does with nicks.

@progval
Copy link
Copy Markdown
Contributor

progval commented Jul 30, 2020

Sorry if this was mentioned before, but:

To help clients that weren't present in the channel during the name change, server implementations MAY implement JOIN redirection from the old channel to the new channel for as long as is deemed necessary.

Shouldn't it mention that, if the client negotiated the cap, the server should send RENAME instead of doing a JOIN redirection?

@jwheare
Copy link
Copy Markdown
Member Author

jwheare commented Jul 30, 2020

@dequis: imo that's no more illegal than all the other cool tricks in bitlbee

@jwheare
Copy link
Copy Markdown
Member Author

jwheare commented Jul 30, 2020

@progval I don't think so, no. JOIN redirection with 470 ERR_LINKCHANNEL happens before a JOIN happens. RENAME is for when you're in the channel at the time.

@progval
Copy link
Copy Markdown
Contributor

progval commented Jul 30, 2020

So how should clients know the channel was RENAMEd if they weren't in the channel at the time?

@jwheare
Copy link
Copy Markdown
Member Author

jwheare commented Jul 30, 2020

470 has a free form text field.

@progval
Copy link
Copy Markdown
Contributor

progval commented Jul 30, 2020

but it doesn't solve the clients issues this spec aims to solve (moving logs, etc.). And bots can't understand the free form text

@jwheare
Copy link
Copy Markdown
Member Author

jwheare commented Jul 30, 2020

Those are fair points. Are you imagining the RENAME being sent after an initial JOIN+burst to the old channel?

@progval
Copy link
Copy Markdown
Contributor

progval commented Jul 30, 2020

I think that would be perfect, yes.

@jwheare
Copy link
Copy Markdown
Member Author

jwheare commented Jul 30, 2020

It might be simpler if rename supporting clients were just given a standard FAIL command defined for this purpose, e.g. CHANNEL_RENAMED, so that they can try again, rather than joining the old channel and then renaming it. Fallback can still be 470.

@progval
Copy link
Copy Markdown
Contributor

progval commented Jul 30, 2020

Sounds good too!

… mechanism. Clarify old name registration restriction.
@jwheare
Copy link
Copy Markdown
Member Author

jwheare commented Jul 30, 2020

Doing it this way also avoids any issues with clients renaming old channels to new ones twice, potentially corrupting logs or causing conflicts.

@jwheare
Copy link
Copy Markdown
Member Author

jwheare commented Jul 30, 2020

Will aim to get this merged to the site as draft next week. Further comments most welcome.

@slingamn
Copy link
Copy Markdown
Contributor

slingamn commented Aug 4, 2020

Sorry about the late comment. I have a question about the rationale for this change:

This fallback SHOULD NOT be used if the rename only changes the case of the channel name.

Oragono implements #272 for case canonicalization, which (AFAIK) all current client software ignores. Moreover, my belief is that there's no practical way to specify a Unicode casefolding algorithm that the client and server can fully agree on, since even if both are using PRECIS, it matters which version of the Unicode standard their PRECIS implementations are aware of. My sense is that the most promising approach is for clients to treat server-sent identifiers (in particular, nicknames and channel names) as opaque byte strings. (The most problematic area for this so far is MONITOR, since MONITOR + has to accept nicknames in non-canonical forms, and then MONITOR L and MONITOR S have to replay them.)

I don't fully understand the claim in this comment that clients doing this are broken. Is it mostly just about the prior existence of m_changecap? I think a reasonable outcome of the rename spec would be that m_changecap is deprecated, and all renaming (including case changes) should use RENAME, which should send PART and JOIN to fallback clients even for a case change.

@slingamn
Copy link
Copy Markdown
Contributor

slingamn commented Aug 4, 2020

From discussion: the intent here is to refer only to the CASEMAPPING canonicalization behavior, which has a straightforward definition and which clients are expected to implement.

This doesn't seem to pose any concerns. Oragono will send the fallback for case changes (whether they fall under CASEMAPPING or UTF8MAPPING), but that's OK because this is just a SHOULD.

hhirtz added a commit to hhirtz/oragono that referenced this pull request Aug 4, 2020
Always send the JOIN-PART couple because oragono might consider channels
name to be equivalent while clients would not. See
<ircv3/ircv3-specifications#420 (comment)>

This reverts commit 0103e14.
@jwheare
Copy link
Copy Markdown
Member Author

jwheare commented Aug 4, 2020

Does that clear up any confusion?

@slingamn
Copy link
Copy Markdown
Contributor

slingamn commented Aug 4, 2020

Yep, thanks.


## Channel redirection

To help clients that weren't present in the channel during the name change, server implementations MAY keep track of renames and send a `FAIL JOIN CHANNEL_RENAMED` message to clients attempting to join the old channel name, for as long as is deemed necessary by the implementation.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can servers automatically join the new channel when client tries to join the old channel?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point is to make it explicit to the client that a rename has happened. An automatic join doesn't give the chance to rename logs first.

hhirtz added a commit to hhirtz/oragono that referenced this pull request Aug 5, 2020
Link to the new draft PR:
<ircv3/ircv3-specifications#420>

Changes in the spec:

- Use standard replies instead of numerics:
  <https://github.com/ircv3/ircv3-specifications/pull/420/files#diff-70e90beef48dc9cf5d784d1e179ea822R44>
- Allow RENAME to a different case:
  <https://github.com/ircv3/ircv3-specifications/pull/420/files#diff-70e90beef48dc9cf5d784d1e179ea822R42>

This commit makes oragono send the PART-JOIN fallback even on case-only
changes. This is so that clients don't have to worry about oragono's
UTF8 casefolding. See the following comments for further info:
<ircv3/ircv3-specifications#420 (comment)>

Misc fixes:

- Remove unused variable,
- Add missing calls to utils.SafeErrorParam,
- Don't fill replies with the user-provided "oldName", for the same
  reason as sending the PART-JOIN fallback.
@jwheare
Copy link
Copy Markdown
Member Author

jwheare commented Aug 11, 2020

Any further comments or things to resolve or can this be merged as draft?

@jwheare
Copy link
Copy Markdown
Member Author

jwheare commented Aug 13, 2020

Will merge next week if no further comments.

@jwheare
Copy link
Copy Markdown
Member Author

jwheare commented Aug 24, 2020

Latest commits make a few changes that go beyond language style:

  • 3rd reason param is always required when sending RENAME server -> client.
  • Soften stance on changing prefix types. There may be valid reasons to allow these, but servers shouldn't feel like they have to allow them. This also avoids the weird "servers: don't do this, clients: support it anyway" stuff.

@jwheare
Copy link
Copy Markdown
Member Author

jwheare commented Aug 24, 2020

Will leave this open for another week to allow any reviews of these changes.

@jwheare jwheare merged commit ee6aaa5 into ircv3:master Sep 2, 2020
@jwheare jwheare deleted the channel-rename branch September 2, 2020 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants