Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow control characters in name fields #303

Conversation

bswartz
Copy link
Contributor

@bswartz bswartz commented Nov 7, 2018

Earlier versions of the spec put no limits on name fields, but
indicate that names should be usable as identifiers on storage
backends. Control characers obviously conflict with this purpose,
so specify that they're prohibitted, while making it clear that
all other valid Unicode strings are allowed.

Fixes #267

@jdef
Copy link
Member

jdef commented Nov 7, 2018

Since we're considering limits for 1 of the name fields in the spec, should we consider a general limit for "name"-like fields throughout the spec?

@bswartz
Copy link
Contributor Author

bswartz commented Nov 7, 2018

@jdef I think I got them all. There are only 2 name fields sent in the CO->Plugin direction. There's another name field in the Plugin->CO direction which already has very stringent restrictions. What other name-like fields did you have in mind?

@jdef
Copy link
Member

jdef commented Nov 7, 2018

@bswartz I think you're right. I don't see anything else.

Copy link
Member

@saad-ali saad-ali left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Needs rebase.

Earlier versions of the spec put no limits on name fields, but
indicate that names should be usable as identifiers on storage
backends. Control characers obviously conflict with this purpose,
so specify that they're prohibitted, while making it clear that
all other valid Unicode strings are allowed.

Fixes container-storage-interface#267
@bswartz
Copy link
Contributor Author

bswartz commented Nov 8, 2018

Rebased

@jieyu jieyu merged commit 86aa4ce into container-storage-interface:master Nov 8, 2018
@bswartz bswartz deleted the disallow-control-characters branch December 30, 2019 03:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants