Azure-Samples / MqttApplicationSamples

Samples implementing common PubSub patterns for Edge and Cloud Brokers
MIT License
25 stars 25 forks source link

MqttConnectionSettings.CreateFromEnvVars does not work if the env var value contains '=' #95

Open briancr-ms opened 7 months ago

briancr-ms commented 7 months ago

This issue is for a: (mark with an x)

- [x] bug report -> please search issues before submitting
- [ ] feature request
- [ ] documentation issue or request
- [ ] regression (a behavior that used to work and stopped in a new release)

Minimal steps to reproduce

Call MqttConnectionSettings.CreateFromEnvVars with a file containing a variable that contains an '=' in the value, such as MQTT_USERNAME=reg-cp-1/registrations/device-1/api-version=2023-12-01-preview

Any log messages given by the failure

Silent failure.

Expected/desired behavior

Environment variables with '=' in value are used.

Language

C#

OS and Version?

Any

rido-min commented 7 months ago

what's the reason of passing the api-version in the username? Should it use UserProperties in the connect packet ? https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901054

briancr-ms commented 7 months ago

Some services require this api-version structure in the MQTT connect username, so clients need to be able to provide this.

rido-min commented 7 months ago

Using the USERNAME field to specify api-version seems questionable. Can we discuss with service owners?

btw, I guess you could specify the value with quotes:

MQTT_USERNAME="reg-cp-1/registrations/device-1/api-version=2023-12-01-preview"
briancr-ms commented 7 months ago

But the client doesn't control what the server requires in the connect username field. The "api-version=" is one case I ran across but I suppose a username could have an '=' in it, like a username "foo=bar".

The current implementation does not support surrounding the value with quotes.

If supporting quoted values is the preferred solution to this problem, then let's specify that (and ensure that the implementation supports escaping quotes as well for values that contain quotes).

BTW, is there a spec for .env files that describes what they should and should not support? I couldn't find one with a quick search.

rido-min commented 7 months ago

Yeah, I'd say that allowing quotes is the preferred solution.

AFAIK there is no "formal" spec, see this doc from docker

https://docs.docker.com/compose/environment-variables/env-file/ https://stackoverflow.com/questions/68267862/what-is-an-env-or-dotenv-file-exactly

briancr-ms commented 7 months ago

Okay, so in order to address this issue, the designers of the MQTTnet client extensions library will need to define what features are supported in a .env file:

rido-min commented 7 months ago

well, there is no such extension library, just a project used in these samples :)

We started with .env files to avoid loading connection strings in C (that requires more parsing), other languages/tools have .env files already implemented, so we added basic .NET support based on https://dusted.codes/dotenv-in-dotnet. But it was not a core requirement for .NET.

For .NET I'd recommend using connection strings instead of .env files.

briancr-ms commented 7 months ago

Okay, but is there a standard definition for an MQTT connection string? I couldn't find this, either. For readability, which is probably one of the points of these samples, the .env key=value approach is nice. Either way, the connection settings input format should be specified and then implemented.

Since this issue is narrowly about the .env file and the '=' character, would you prefer that I open an issue that is used for specifying the connection settings for these samples?

rido-min commented 7 months ago

is there a standard definition for an MQTT connection string?

I'm not aware neither, we are defining our own here

Is that document enough? if not, feel free to file a new issue