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

oragono rejects multiline batches with blank lines #1005

Closed
slingamn opened this issue May 13, 2020 · 7 comments
Closed

oragono rejects multiline batches with blank lines #1005

slingamn opened this issue May 13, 2020 · 7 comments
Labels
Milestone

Comments

@slingamn
Copy link
Member

@sumeet reported this via irccloud.

@slingamn slingamn added the bug label May 13, 2020
@slingamn slingamn added this to the v2.1 milestone May 13, 2020
@slingamn
Copy link
Member Author

I disallowed this because it could lead to a DoS attack in the case where max-lines is unset. Questions:

  1. Does a blank line count against max-bytes?
  2. Is a blank line with the concat tag attached invalid?

@slingamn
Copy link
Member Author

  1. How to send a blank line to fallback clients? Normally you can't do this at all (the attempt to send returns a 412 ERR_NOTEXTTOSEND). Should we just omit the line?

@DanielOaks
Copy link
Member

DanielOaks commented May 13, 2020

On (3), can't the server just send the blank line to the client? Just have an empty final param and it'll be fine, e.g. something like

:oragono.io PRIVMSG #whoever :

@slingamn
Copy link
Member Author

The issue is that it's impossible to do this with traditional IRC, so it's not clear that clients can handle it.

@DanielOaks
Copy link
Member

I don't see it being a big issue but could always run some client tests if we really want to

@slingamn
Copy link
Member Author

Summary of discussion thus far:

  1. For purposes of max-bytes enforcement, message length is defined to be the length of the message concatenated ("joined") with \n, except for messages with the concat tag, which have no separator. Note that the final line is not followed by a \n. (This is already implicit in the specification but it could be made a little clearer.)
  2. A blank line with a concat tag should probably be skipped/ignored by the server, rather than rejected.
  3. Legacy clients should probably not ever receive an empty PRIVMSG.
  4. The way to ensure this is probably to skip blank lines when relaying to legacy clients. Client-only tags and the msgid are deferred to the first nonblank line. A message that contains only blank lines (with or without the concat tag) is probably invalid. (@jwheare: seems like a useful simplification here would be to say that the first line MUST NOT be blank? That would entail both of these other properties, and normalizing out any initial blank lines on the client side seems straightforward.)

@jwheare
Copy link

jwheare commented May 14, 2020

PR for these suggestions is here: ircv3/ircv3-specifications#413

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants