SET_CLIENT_ID accepts *ANYTHING*

[SpamapS]

I am messing around with Rustygear's SET_CLIENT_ID and workers admin protocol implementation and I noticed that you can send anything. \0 and \n are particularly problematic.

1. DEBUG Logs that are being emitted with \n as the delimiter, get messed up. I don't consider DEBUG critical, but it's still annoying and will break any log parsers.
2. The workers command gets corrupted if you put \n in the client ID, as this is the delimiter for the command.
3. gearmand uses %s on the client ID while emitting the workers command, so it never prints anything after any nulls.

I think the right thing to do is make a protocol refinement which will suggest that the content of client ID has to be UTF-8, and then update gearmand to reject non-UTF-8 content with an ERROR packet.

[esabol]

Well, those are good reasons to not allow \n and \0, certainly. I'm not sure about the "has to be UTF-8" part though. The impact of that is not clear to me. Can you elaborate on why you feel that is needed, @SpamapS ?

[SpamapS]

Well, those are good reasons to not allow \n and \0, certainly. I'm not sure about the "has to be UTF-8" part though. The impact of that is not clear to me. Can you elaborate on why you feel that is needed, @SpamapS ?

It's the standard way to encode text in 2024, and it will allow wide characters making it more accessible to more languages.

[esabol]

I guess. But what's the impact on gearmand if we start allowing (or requiring?) strings to be UTF-8?

[esabol]

I guess what I'm asking is whether we need to change any of the the string functions in gearmand to support UTF-8?

[SpamapS]

I think we'd only need to change the SET_CLIENT_ID handler to check the input and reject anything that isn't valid UTF-8. There are a few options for doing this in a lightweight manner. Other than that we might need to change some format strings to properly handle the multibyte chars but raw %s should just work.

…

On Thu, Sep 19, 2024 at 7:41 PM Ed Sabol ***@***.***> wrote: I guess what I'm asking is whether we need to change any of the the string functions in gearmand to support UTF-8? — Reply to this email directly, view it on GitHub <#394 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADS6YAWQHUVTL7BO33VAT3ZXODLZAVCNFSM6AAAAABJULZQPOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNRSGYYDKNJYGY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[esabol]

Well, if you want to implement those things to support UTF-8 client IDs, that sounds fine to me. I don't have a whole lot of experience working with non-ASCII character sets, tbh, in case that wasn't obvious from my questions.

[SpamapS]

Yeah maybe we can just disallow \n and \0 just to make it better now.