TXT inconsistencies with RFC6763 about mdns-sd

[dalepsmith]

Having TXT records be HashMap<String, String> is a very nice and logical API.

However, Keys are not utf-8, only printable ascii.
And the Values can be any sequence of bytes (including '='), again not utf-8.

[keepsimple1]

Thanks for the details!

New publish does not have TXT
If start browsing, existing services have TXT record info.
If I publish a service (with avahi-publish-service), the TXT info is missing.
But if I publish BEFORE I start the browse, I do get the TXT info.

This is a bug and worth a separate issue. Would you mind open another issue for it?

[keepsimple1]

However, Keys are not utf-8, only printable ascii.

UTF-8 is backward compatible with ascii. So it's fine to represent ascii using utf-8.

And the Values can be any sequence of bytes (including '='), again not utf-8.

I didn't know that, thanks for pointing it out. Before we change, do you have use cases where you do need to support non-UTF-8 values? I'm trying to see benefit/cost ratio.

[keepsimple1]

For other key/value rules, I agree we should fix the code using this issue.

[dalepsmith]

Yes! (Please excuse the top-post) Sent from my iPhone... Beware of typos!

…

On Nov 10, 2022, at 5:52 PM, keepsimple1 ***@***.***> wrote:  Thanks for the details! New publish does not have TXT If start browsing, existing services have TXT record info. If I publish a service (with avahi-publish-service), the TXT info is missing. But if I publish BEFORE I start the browse, I do get the TXT info. This is a bug and worth a separate issue. Would you mind open another issue for it? — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.

[dalepsmith]

On 11/10/22, keepsimple1 ***@***.***> wrote: > However, Keys are not utf-8, only printable ascii. UTF-8 is backward compatible with ascii. So it's fine to represent ascii using utf-8.

As far as receiving, yes this is fine. However, *publishing* should verify that the key has only the allowed characters (0x20 - 0x7e, except for '=').

> And the Values can be any sequence of bytes (including '='), again not > utf-8. I didn't know that, thanks for pointing it out. Before we change, do you have use cases where you do need to support non-UTF-8 values? I'm trying to see benefit/cost ratio.

I don't know of any specifically. Just noticed it while reading the RFC. I would imagine *most* values are just text, But since the spec allows it, it's probably out there, or will be. Best be prepared to handle it somehow. I don't have a good suggestion. `Vec<u8>` instead of `String`? Maybe an enum? Also, the no-key and empty key cases ("key" and "key=") should be distinguished somehow. Maybe with an `Option<>` ?

…

-Dale

[keepsimple1]

As far as receiving, yes this is fine. However, publishing should verify that the key has only the allowed characters (0x20 - 0x7e, except for '=').

Good point. I think we can check it in ServiceInfo::new().

I don't know of any specifically. Just noticed it while reading the RFC. I would imagine most values are just text, But since the spec allows it, it's probably out there, or will be. Best be prepared to handle it somehow.

Maybe I'm biased, but I think the world is moving towards UTF-8. It is possible that some rare applications use non-UTF-8 in TXT records but I'm willing to postpone this until a concrete case shows up in our context.

(Some other types in Rust ecosystem face / faced the same problem, for example std::path::Path. IMO, the history shows that supporting non-UTF-8 things in a mostly UTF-8 environment can be costly for the end users.)

Also, the no-key and empty key cases ("key" and "key=") should be distinguished somehow. Maybe with an Option<> ?

Yes, that sounds good to me. And maybe we can take this opportunity to define a type X for TXT key/value pairs so that ServiceInfo::new() accepts X or Option<X> instead of the plain HashMap<String, String>. Also, maybe we keep the new() as is (deprecate it later), and add a new / better constructor for ServiceInfo, maybe even using Builder pattern or something?

And with a new type for TXT k/v pairs and a new constructor, it might be easier to be flexible in supporting non-UTF-8 values (now or later).

[keepsimple1]

Two of the problems are resolved in their separated issues:

• Last Key: #76 .
• New publish does not have TXT: #72 .

[keepsimple1]

All issues mentioned above are addressed in Release 0.7.0. If any problems, please open a new issue.

Thanks again @dalepsmith for opening this issue and helping solving them. If anything not working properly, please raise them.