Skip to content

Validate container names on creation. Fixes #3138#3197

Merged
creack merged 1 commit into
moby:masterfrom
ajhager:3138-names
Dec 14, 2013
Merged

Validate container names on creation. Fixes #3138#3197
creack merged 1 commit into
moby:masterfrom
ajhager:3138-names

Conversation

@ajhager

@ajhager ajhager commented Dec 12, 2013

Copy link
Copy Markdown
Contributor

This adds a check in the runtime when creating a container to make sure the name only contains alpha-numeric characters and underscore. Fixes #3138.

Comment thread runtime.go Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you move this variable out of the method so that it's not created every time? At the top of this file is fine.

@tianon

tianon commented Dec 12, 2013

Copy link
Copy Markdown
Member

Can we have hyphens too please?

@vreon

vreon commented Dec 12, 2013

Copy link
Copy Markdown
Contributor

+1 for hyphens.

@ajhager

ajhager commented Dec 13, 2013

Copy link
Copy Markdown
Contributor Author

I added hyphens to the regex and moved the variable to the top of the file.

@crosbymichael

Copy link
Copy Markdown
Contributor

LGTM

Comment thread runtime.go Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why the parenthesis? We don't use the captured result anywhere, do we?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Originally, I was trying to capture that group so the error message wouldn't contain the '/' that is sometimes at the start of a passed in name, but isn't actually a part of the name. I can safely remove the parenthesis.

Move valid container name regex to the top of the file

Added hyphen as a valid rune in container names.

Remove group in valid container name regex.
@ajhager

ajhager commented Dec 13, 2013

Copy link
Copy Markdown
Contributor Author

The group has now been removed from the regexp.

@tianon

tianon commented Dec 13, 2013

Copy link
Copy Markdown
Member

+1 me gusta :)

@creack

creack commented Dec 14, 2013

Copy link
Copy Markdown
Contributor

LGTM

creack added a commit that referenced this pull request Dec 14, 2013
Validate container names on creation. Fixes #3138
@creack
creack merged commit a6928e7 into moby:master Dec 14, 2013
@ajhager
ajhager deleted the 3138-names branch December 14, 2013 01:44
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.

Restrict allowed characters in container names

5 participants