Extension Naming Validation Bug

[MB1211]

This is NOT fixed by #366 but is very similar and is further evidence that this is a valid bug. This issue has nothing to do with the length of the extension name - it needs to address the valid characters that make up the extension name. Currently it only allows lowercase letters (a-z) and numbers (0-9). Extension names need to allow at the very least '-' and '_'

Using JsonFormat, I am getting exceptions thrown for any extension name that is not strictly alphanumeric. I don't think this issue is necessarily specific to JsonFormat though

The spec states "CloudEvents attribute names MUST consist of lower-case letters ('a' to 'z') or digits ('0' to '9') from the ASCII character set. Attribute names SHOULD be descriptive and terse and SHOULD NOT exceed 20 characters in length."

But the code BaseCloudEventBuilder to validate the extension name (which references https://github.com/cloudevents/spec/blob/master/spec.md#attribute-naming-convention) seems to be misinterpreting this. It interprets it as MUST ONLY, because the method to validate characters is the following:

private static boolean isValidChar(char c) {
        return (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9');
    }

Unless I'm mistaken, this is not the intent, and a fairly problematic bug. It's not really possible to be descriptive with no way of delimiting words. Elsewhere in the spec, and in the code itself there are mentions of extensions with -'s and '''s so these at the very least should be allowed (ce-, '' ce).

[MB1211]

#271 is more relevant.
Is there any room for interpretation here? Could someone weight in on why the decision to only allow a-z and 0-9 was made? It seems excessively restrictive, but maybe there's a good reason?

[slinkydeveloper]

Sorry for closing the previous issue, I misunderstood it, my bad!

It interprets it as MUST ONLY, because the method to validate characters is the following:

What you're saying about MUST and MUST ONLY may be right, but I'm pretty sure the intention of that spec sentence is to strictly limit the characters used to name a spec field.

Could someone weight in on why the decision to only allow a-z and 0-9 was made?

I think this was discussed quite some times on the spec repo, e.g. cloudevents/spec#265. Maybe @duglin can give you more input about that.

Elsewhere in the spec, and in the code itself there are mentions of extensions with -'s and '''s so these at the very least should be allowed (ce-, '' ce).

Strictly speaking, prefixes like ce_ and ce- are not part of the attribute name, in fact the sdk strips the attribute name before passing it to the BaseCloudEventBuilder. If, for example, you get an incoming http request with a cloudevent containing the header ce-my-extension, the sdk will pass my-extension to BaseCloudEventBuilder, which will fail because of that check on the alphanumeric characters.

[MB1211]

No problem, thanks for the link to that discussion.
The reason we were running into this is we had some teams using version 1.2.0, and since this allowed -'s in the extension names, they ended up putting -'s in their extension names. We are adding a new project which consumes a kafka topic, and the message body is these cloud events (which are I suppose not technically cloud events because of the -'s). We like the new version of the API much better, but it throws exceptions during deserialization.

I wonder if there would be any value in creating some type of configuration which would allow a warning instead of an exception for -'s in the extension name? Our teams are moving toward getting rid of the -'s so it will hopefully not be necessary.

[slinkydeveloper]

I wonder if there would be any value in creating some type of configuration which would allow a warning instead of an exception for -'s in the extension name?

I recall a while ago I discussed about having some "less strict" parsing mode, where the parser can "guess", or better "correct" the incoming event, but TBH I never worked through it, IMO is also quite outside the scope of this project at the moment, given our goal is to provide a reference implementation. Such mode is very complicated to design, and needs to take in account a wide variety of "problems" and define some tweakable "corrections" to the input in order to make everybody happy.

[pierDipi]

The goal of Cloudevents is to make systems interoperable; a less strict parsing mode goes against that goal because we start sending and receiving events that are not Cloudevents.

[pierDipi]

I'm closing this issue, for now.
Please reopen if you want to discuss it more.