Should authParamsString for Authentication support another format ?

[yush1ga]

Now, when parsing authParamsString, keys or values containing ":" or "," cannot be parsed correctly.

However, C++ Athenz Plugin requires for URL (containing ":") as a parameter.
Moreover, other 3rd party or official auth plugins may require parameters containing ":" in near future.

I have 3 ideas to resolve this problem.

1. Enable users to set delimiters.

Users can set delimiters freely if they don't want to use ":" and ",".

2. Support JSON

In addition to current format, enable JSON to be used.

3. Ignore second and subsequent ":"

e.g.
When the string like url:http://hoge.com:4080/ is gotten, it is processed like following.

paramString = "url:http://hoge.com:4080/"
String[] kv = paramString.split(":")
paramMap[kv[0]] = "";
for (int i = 1; i < kv.size(); i++) {
    paramMap[kv[0]] += kv[i];
}

※ In this case, parameter containing "," cannot be parsed.

[maskit]

I think the entire parameter value should be parsed on plugins side. Then plugins can choose any format they want. Some plugin may need just a keyword, some other may need binary data. This would break compatibility though. I'd say the current interface could be better. (When can we make incompatible changes btw?)

If we need to keep compatibility as possible as we can, 4th option would be "Encode values" (== don't allow to use ":" and ","). Any encodings which doesn't use ":" and "," (e.g. base64, URL encode) are fine, and we can (should) delegate the decoding to plugins.

[nkurihar]

@maskit

I think the entire parameter value should be parsed on plugins side. Then plugins can choose any format they want.

Thank you for your comments and I agree.
Then, how about modifying by 2 steps?

Step 1.
Add URL-decoding logic to C++ Athenz plugin (Morespecifically, ZTSClient.cc):
https://github.com/apache/incubator-pulsar/blob/master/pulsar-client-cpp/lib/auth/athenz/ZTSClient.cc#L76
This change keeps the compatibility.

Step 2.
Move parsing logic from AuthenticationFactory to each Authentication plugin.
https://github.com/apache/incubator-pulsar/blob/master/pulsar-client/src/main/java/org/apache/pulsar/client/api/AuthenticationFactory.java#L44-L53
This change breaks the compatibility.

[maskit]

We can manage the compatibility if we provided an extra interface below.

/**
 * Plugins which use ":" and/or "," in a configuration parameter value need to implement this interface.
 * This interface will be integrated into Authentication interface and be required for all plugins on version x.y.
 */
public interface EncodedAuthenticationParameterSupport {
    void configure(String encodedAuthParamString);
}

URL encoding is just a hack for the compatibility. We can use any encode once we added new interface. So, I think the step 1 is not mandatory. Please keep in mind that the configuration value will be written by humans. Support for double-quotation is much easier to use than URL encoding.

[tkb77]

My name is Yuta Takaba, a software engineer at Yahoo Japan Corporation.
Please call me TKB!
Anyway, I try to address this issue by the following:

1: Adding configure(String authParamString) to Authentication instead of making new interface for simplicity

2: Support JSON for configure.

• Because, JSON is popular, widely used.
• For keeping compatibility, using current parsing logic(separated by “:” and “,”) if an argument is not JSON.

[sijie]

This is fixed by #721