String encode rules

[bswartz]

The current spec doesn't say anything about whether Unicode characters are allowed in various string fields (most notably the volume name), and if so, whether UTF-8 encoding is required or whether other encodings are allowed.

Also, it's not clear whether whitespace, newlines, tabs, or control characters including backspaces, NULs, EOFs, BELLs, etc should be allowed in string fields.

Some fields clearly specify alphanumeric values, and we could probably expand this language to other fields, or put some general language about all strings in the document.

[jdef]

re: "all strings in the document" -- not sure that I agree with this point. opaque strings are just that - opaque. format limitations were introduced for some fields to ensure compatibility w/ certain CO's. plugin vendors are all going to have different requirements for what's "reasonable" and it's going to be difficult to satisfy every vendor. so we decided a while back to not try very hard and just declare certain strings as opaque, pushing some amount of burden to the plugin. there's a much smaller number of CO's to satisfy, so agreeing on reasonable limits for things that COs need to ingest seems like an easier hurdle to jump. protobuf3 specifies UTF-8 for strings, so encoding is already defined for us since we're using gRPC. CreateVolumeRequest.name is a *suggested* name that the plugin could consider as the volume identifier, but it's not required to do so. why should this field have special formatting limits outside of the UTF-8 spec? what makes this field more special than other strings that CO's feed to the plugins?

…

On Thu, Aug 30, 2018 at 12:20 PM Ben Swartzlander ***@***.***> wrote: The current spec doesn't say anything about whether Unicode characters are allowed in various string fields (most notably the volume name), and if so, whether UTF-8 encoding is required or whether other encodings are allowed. Also, it's not clear whether whitespace, newlines, tabs, or control characters including backspaces, NULs, EOFs, BELLs, etc should be allowed in string fields. Some fields clearly specify alphanumeric values, and we could probably expand this language to other fields, or put some general language about all strings in the document. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#267>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACPVLD3__Js2eVT5TDVLuFu8VK7t8qUNks5uWBDLgaJpZM4WTzIG> .

[bswartz]

You're right about UTF-8 encoding. And I'm fine with the idea of allowing Unicode characters in CreateVolumeRequest.name, but then there should be test cases to make sure that plugin authors don't make bad assumptions about the incoming names.

I'm less okay with allowing weird control characters in the suggested name, because there's no valid reason for it. Nobody is going to name their CSI volume "\x07".

[saad-ali]

Conclusion on CSI call was to add limits for "obviously" not useful characters e.g. NUL, BEL, etc. but not block whitespace, etc. Driver is always responsible for validating input.

[saad-ali]

@bswartz are you working on this? We'd like to get all 1.0 changes merged by EOD Friday Nov 9.