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

Colons in pathnames #8604

Closed
blakery opened this Issue Oct 16, 2014 · 39 comments

Comments

Projects
None yet
@blakery
Contributor

blakery commented Oct 16, 2014

This references issue #8438 : Unable to map docker volume with colon.

In light of the announcement re windows support, the scope of this has changed considerably. As I had posted to the dev group, I see the following solutions (revised for newly available information):

  1. Change nothing, but specify that colons are not accepted in path names. Obviously the simplest solution, as it only involves a sentence or so in the documentation. Before, I didn't like this, because it seemed sloppy and incomplete. While colons are not guaranteed to be portable by Posix, they are not forbidden either, and in practice most operating systems allow it. With the addition of adding windows support, this is now outright impossible (fine by me). C:foo will have to be perfectly valid.
  2. As suggested by the op: Changing the behavior to only split at the start of an absolute pathname. I've looked at this, and it's relatively simple to implement, but does raise some issues about some error conditions and how they're handled. More importantly, with windows support, this no longer makes sense. C:/foo would still appear as "C" "/foo".
  3. Implement escape characters. Before this seemed like overkill. Now it seems like the a very reasonable solution.
  4. Reconfigure how pathname lists are handled entirely. This could be something like using quotes, or allowing (but not requiring) quotes, and ensure that anything within them will be taken as a single path name. Such as: -v="C:/foo":/tmp/foo . As I'm writing this, and thinking it through, this is seeming more plausible.
  5. Any other suggestions?

1 and 2 are no longer plausible. 3 is not bad, but could get cumbersome (especially for windows users). I think that I could implement 4 with little overhead.

  • Blake Geno
@russellrichardson

This comment has been minimized.

Show comment
Hide comment
@russellrichardson

russellrichardson Oct 16, 2014

My vote would be to address the current issue before bringing in the new requirements for windows support. The :/ option seems the most straight forward to me, and it also unblocks those of us trying to use docker with chronos and mesos.

Windows does use backslashes in path names "C:" .. so the :/ separator would work. However, isn't the announcement saying that docker will support windows server containers? Then, -v=C:\foo:C:\foo would also need to be supported. If that's the case the quotes don't seem so bad or separating out the source/destination in to different options and avoid needing a separator.

russellrichardson commented Oct 16, 2014

My vote would be to address the current issue before bringing in the new requirements for windows support. The :/ option seems the most straight forward to me, and it also unblocks those of us trying to use docker with chronos and mesos.

Windows does use backslashes in path names "C:" .. so the :/ separator would work. However, isn't the announcement saying that docker will support windows server containers? Then, -v=C:\foo:C:\foo would also need to be supported. If that's the case the quotes don't seem so bad or separating out the source/destination in to different options and avoid needing a separator.

@blakery

This comment has been minimized.

Show comment
Hide comment
@blakery

blakery Oct 16, 2014

Contributor

I had forgotten that windows uses "" not "/" . But if windows containers will be supported as well, than that eliminates option 3, since absolute path names don't start with a / in windows.

The ability to use quotes for pathnames now seems best to me, especially since it would not invalidate anything, and would allow for nearly anything (except ' " ', unless we want to allow escapes) in path names. And not requiring quotes, except with ":" or any other non-standard characters would not break any backwards compatibility.

Again, I may be missing something more obvious.

Contributor

blakery commented Oct 16, 2014

I had forgotten that windows uses "" not "/" . But if windows containers will be supported as well, than that eliminates option 3, since absolute path names don't start with a / in windows.

The ability to use quotes for pathnames now seems best to me, especially since it would not invalidate anything, and would allow for nearly anything (except ' " ', unless we want to allow escapes) in path names. And not requiring quotes, except with ":" or any other non-standard characters would not break any backwards compatibility.

Again, I may be missing something more obvious.

@russellrichardson

This comment has been minimized.

Show comment
Hide comment
@russellrichardson

russellrichardson Oct 16, 2014

What about detecting what type of path it is and have a graceful degradation in requiring extra things? That way, this -v=/tmp/my:path/here:/other/path .. can be valid and use the :/ separator.

However, if [A-Z]: is present then require quotes. Could output an error message like. "It looks like you are using a windows path, please wrap it in quotes"

russellrichardson commented Oct 16, 2014

What about detecting what type of path it is and have a graceful degradation in requiring extra things? That way, this -v=/tmp/my:path/here:/other/path .. can be valid and use the :/ separator.

However, if [A-Z]: is present then require quotes. Could output an error message like. "It looks like you are using a windows path, please wrap it in quotes"

@cpuguy83

This comment has been minimized.

Show comment
Hide comment
@cpuguy83

cpuguy83 Oct 16, 2014

Contributor

We could probably make it handle escaped chars

Contributor

cpuguy83 commented Oct 16, 2014

We could probably make it handle escaped chars

@erikh

This comment has been minimized.

Show comment
Hide comment
@erikh

erikh Oct 16, 2014

Contributor

I’m wondering if it makes more sense to have it:

  1. Try to split by :
  2. If more than two parts, try to split by =
  3. Error out if more than two parts

This way the user has more than one way to skin a cat. :)

On Oct 16, 2014, at 10:50 AM, Brian Goff notifications@github.com wrote:

We could probably make it handle escaped chars


Reply to this email directly or view it on GitHub.

Contributor

erikh commented Oct 16, 2014

I’m wondering if it makes more sense to have it:

  1. Try to split by :
  2. If more than two parts, try to split by =
  3. Error out if more than two parts

This way the user has more than one way to skin a cat. :)

On Oct 16, 2014, at 10:50 AM, Brian Goff notifications@github.com wrote:

We could probably make it handle escaped chars


Reply to this email directly or view it on GitHub.

@erikh

This comment has been minimized.

Show comment
Hide comment
@erikh

erikh Oct 16, 2014

Contributor

Windows supports / as well, it’s equivalent. Since NT or so.

On Oct 16, 2014, at 10:04 AM, Blake Geno notifications@github.com wrote:

I had forgotten that windows uses "" not "/" . But if windows containers will be supported as well, than that eliminates option 3, since absolute path names don't start with a / in windows.

The ability to use quotes for pathnames now seems best to me, especially since it would not invalidate anything, and would allow for nearly anything (except ' " ', unless we want to allow escapes) in path names. And not requiring quotes, except with ":" or any other non-standard characters would not break any backwards compatibility.

Again, I may be missing something more obvious.


Reply to this email directly or view it on GitHub.

Contributor

erikh commented Oct 16, 2014

Windows supports / as well, it’s equivalent. Since NT or so.

On Oct 16, 2014, at 10:04 AM, Blake Geno notifications@github.com wrote:

I had forgotten that windows uses "" not "/" . But if windows containers will be supported as well, than that eliminates option 3, since absolute path names don't start with a / in windows.

The ability to use quotes for pathnames now seems best to me, especially since it would not invalidate anything, and would allow for nearly anything (except ' " ', unless we want to allow escapes) in path names. And not requiring quotes, except with ":" or any other non-standard characters would not break any backwards compatibility.

Again, I may be missing something more obvious.


Reply to this email directly or view it on GitHub.

@thockin

This comment has been minimized.

Show comment
Hide comment
@thockin

thockin Oct 16, 2014

Contributor

Maybe do it like sed - the first character of the argument becomes the
separator. Would need a new flag, since that is not backwards compatible.

On Thu, Oct 16, 2014 at 11:14 AM, Erik Hollensbe notifications@github.com
wrote:

Windows supports / as well, it's equivalent. Since NT or so.

On Oct 16, 2014, at 10:04 AM, Blake Geno notifications@github.com
wrote:

I had forgotten that windows uses "" not "/" . But if windows
containers will be supported as well, than that eliminates option 3, since
absolute path names don't start with a / in windows.

The ability to use quotes for pathnames now seems best to me, especially
since it would not invalidate anything, and would allow for nearly anything
(except ' " ', unless we want to allow escapes) in path names. And not
requiring quotes, except with ":" or any other non-standard characters
would not break any backwards compatibility.

Again, I may be missing something more obvious.

Reply to this email directly or view it on GitHub.

Reply to this email directly or view it on GitHub
#8604 (comment).

Contributor

thockin commented Oct 16, 2014

Maybe do it like sed - the first character of the argument becomes the
separator. Would need a new flag, since that is not backwards compatible.

On Thu, Oct 16, 2014 at 11:14 AM, Erik Hollensbe notifications@github.com
wrote:

Windows supports / as well, it's equivalent. Since NT or so.

On Oct 16, 2014, at 10:04 AM, Blake Geno notifications@github.com
wrote:

I had forgotten that windows uses "" not "/" . But if windows
containers will be supported as well, than that eliminates option 3, since
absolute path names don't start with a / in windows.

The ability to use quotes for pathnames now seems best to me, especially
since it would not invalidate anything, and would allow for nearly anything
(except ' " ', unless we want to allow escapes) in path names. And not
requiring quotes, except with ":" or any other non-standard characters
would not break any backwards compatibility.

Again, I may be missing something more obvious.

Reply to this email directly or view it on GitHub.

Reply to this email directly or view it on GitHub
#8604 (comment).

@blakery

This comment has been minimized.

Show comment
Hide comment
@blakery

blakery Oct 16, 2014

Contributor

The main thing I don't like about escape characters as a solution to this problem is that it seems like it makes windows users into second class citizens/users. It would require escapes for basically any filepath. I do like the idea of escape characters overall, but that has its own issues (such as which character, since "" would also be awkward with windows).

The problem with

  1. Try to split by :
  2. If more than two parts, try to split by =
  3. Error out if more than two parts

is that things like: -v C:/someplace:B:/tmp/foo:ro is perfectly valid, even though it would currently split into about 5 segments. Do you mean "=" as in C:/someplace=B:/tmp/foo=ro ?

Maybe do it like sed - the first character of the argument becomes the separator. Would need a new flag, since that is not backwards compatible.

That seems plausible.

Contributor

blakery commented Oct 16, 2014

The main thing I don't like about escape characters as a solution to this problem is that it seems like it makes windows users into second class citizens/users. It would require escapes for basically any filepath. I do like the idea of escape characters overall, but that has its own issues (such as which character, since "" would also be awkward with windows).

The problem with

  1. Try to split by :
  2. If more than two parts, try to split by =
  3. Error out if more than two parts

is that things like: -v C:/someplace:B:/tmp/foo:ro is perfectly valid, even though it would currently split into about 5 segments. Do you mean "=" as in C:/someplace=B:/tmp/foo=ro ?

Maybe do it like sed - the first character of the argument becomes the separator. Would need a new flag, since that is not backwards compatible.

That seems plausible.

@erikh

This comment has been minimized.

Show comment
Hide comment
@erikh

erikh Oct 16, 2014

Contributor

On Oct 16, 2014, at 11:28 AM, Blake Geno notifications@github.com wrote:

is that things like: -v C:/someplace:B:/tmp/foo:ro is perfectly valid, even though it would currently split into about 5 segments. Do you mean "=" as in C:/someplace=B:/tmp/foo=ro ?

Correct. It’s a fallback separator.

The sed idea works too. Let’s try to get more than us to chime in, though. Adding new flags is always problematic because they last forever. Let’s make sure that’s what people want.

-Erik

Contributor

erikh commented Oct 16, 2014

On Oct 16, 2014, at 11:28 AM, Blake Geno notifications@github.com wrote:

is that things like: -v C:/someplace:B:/tmp/foo:ro is perfectly valid, even though it would currently split into about 5 segments. Do you mean "=" as in C:/someplace=B:/tmp/foo=ro ?

Correct. It’s a fallback separator.

The sed idea works too. Let’s try to get more than us to chime in, though. Adding new flags is always problematic because they last forever. Let’s make sure that’s what people want.

-Erik

@blakery

This comment has been minimized.

Show comment
Hide comment
@blakery

blakery Oct 16, 2014

Contributor

Agreed. I can't say I'm enthusiastic about the idea of a new flag myself, but we'll see.

I don't see how a fallback separator is different than allowing quotes or brackets to guarantee the pathname, though.

Contributor

blakery commented Oct 16, 2014

Agreed. I can't say I'm enthusiastic about the idea of a new flag myself, but we'll see.

I don't see how a fallback separator is different than allowing quotes or brackets to guarantee the pathname, though.

@tianon

This comment has been minimized.

Show comment
Hide comment
@tianon

tianon Oct 16, 2014

Member

Or we punt on the whole discussion and use "docker volumes create" to
explicitly let Docker track our host paths.

Member

tianon commented Oct 16, 2014

Or we punt on the whole discussion and use "docker volumes create" to
explicitly let Docker track our host paths.

@cpuguy83

This comment has been minimized.

Show comment
Hide comment
@cpuguy83

cpuguy83 Oct 16, 2014

Contributor

+1

Contributor

cpuguy83 commented Oct 16, 2014

+1

@erikh

This comment has been minimized.

Show comment
Hide comment
@erikh

erikh Oct 16, 2014

Contributor

Those will get swallowed by the shell, making this even more complicated.

On Oct 16, 2014, at 11:43 AM, Blake Geno notifications@github.com wrote:

Agreed. I can't say I'm enthusiastic about the idea of a new flag myself, but we'll see.

I don't see how a fallback separator is different than allowing quotes or brackets to guarantee the pathname, though.


Reply to this email directly or view it on GitHub.

Contributor

erikh commented Oct 16, 2014

Those will get swallowed by the shell, making this even more complicated.

On Oct 16, 2014, at 11:43 AM, Blake Geno notifications@github.com wrote:

Agreed. I can't say I'm enthusiastic about the idea of a new flag myself, but we'll see.

I don't see how a fallback separator is different than allowing quotes or brackets to guarantee the pathname, though.


Reply to this email directly or view it on GitHub.

@solidsnack

This comment has been minimized.

Show comment
Hide comment
@solidsnack

solidsnack Oct 21, 2014

What about splitting on the strings of colons that are the longest?

-v C:/someplace::B:/tmp/foo::ro

solidsnack commented Oct 21, 2014

What about splitting on the strings of colons that are the longest?

-v C:/someplace::B:/tmp/foo::ro
@tnachen

This comment has been minimized.

Show comment
Hide comment
@tnachen

tnachen Oct 21, 2014

@solidsnack however multiple colons is a valid file name :(

tnachen commented Oct 21, 2014

@solidsnack however multiple colons is a valid file name :(

@solidsnack

This comment has been minimized.

Show comment
Hide comment
@solidsnack

solidsnack Oct 22, 2014

To programmatically make a valid <src>:<dest> pair, you would search for the longest string of colons in <src> and <dest> and then add one colon and use the resulting string as the separator.

solidsnack commented Oct 22, 2014

To programmatically make a valid <src>:<dest> pair, you would search for the longest string of colons in <src> and <dest> and then add one colon and use the resulting string as the separator.

@cpuguy83

This comment has been minimized.

Show comment
Hide comment
@cpuguy83

cpuguy83 Oct 22, 2014

Contributor

This will be handled by the upcoming docker volumes create --path /path/to/dir

Contributor

cpuguy83 commented Oct 22, 2014

This will be handled by the upcoming docker volumes create --path /path/to/dir

@tnachen

This comment has been minimized.

Show comment
Hide comment
@tnachen

tnachen Oct 22, 2014

@solidsnack I see, ideally this should be handled by docker cli itself not whatever is calling the docker cli though.
@cpuguy83 not sure what is going to be handled by that new call?

tnachen commented Oct 22, 2014

@solidsnack I see, ideally this should be handled by docker cli itself not whatever is calling the docker cli though.
@cpuguy83 not sure what is going to be handled by that new call?

@cpuguy83

This comment has been minimized.

Show comment
Hide comment
@cpuguy83

cpuguy83 Oct 22, 2014

Contributor

@tnachen You will be able to create a volume without the CLI having to parse colons since it's just the path that's being entered, no extra stuff.

Contributor

cpuguy83 commented Oct 22, 2014

@tnachen You will be able to create a volume without the CLI having to parse colons since it's just the path that's being entered, no extra stuff.

@tnachen

This comment has been minimized.

Show comment
Hide comment
@tnachen

tnachen Oct 22, 2014

@cpuguy83 I see, so for any path (including windows I suppose) that has a colon, you first create a volume to a path that cannot contain a colon (not sure how that works in windows), and then pass that to the docker run volume mapping option then right?

tnachen commented Oct 22, 2014

@cpuguy83 I see, so for any path (including windows I suppose) that has a colon, you first create a volume to a path that cannot contain a colon (not sure how that works in windows), and then pass that to the docker run volume mapping option then right?

@cpuguy83

This comment has been minimized.

Show comment
Hide comment
@cpuguy83

cpuguy83 Oct 22, 2014

Contributor

Well, the point is the path can contain a volume when using docker volumes create since there is no colon based separator.

Contributor

cpuguy83 commented Oct 22, 2014

Well, the point is the path can contain a volume when using docker volumes create since there is no colon based separator.

@tnachen

This comment has been minimized.

Show comment
Hide comment
@tnachen

tnachen Oct 22, 2014

@cpuguy83 I see, but then I still don't know how to get docker run -v to work

tnachen commented Oct 22, 2014

@cpuguy83 I see, but then I still don't know how to get docker run -v to work

@cpuguy83

This comment has been minimized.

Show comment
Hide comment
@cpuguy83
Contributor

cpuguy83 commented Oct 22, 2014

@ewindisch

This comment has been minimized.

Show comment
Hide comment
@ewindisch

ewindisch Nov 10, 2014

Contributor

This is still a problem for bind-mounts, so we also need support for escaping.

Contributor

ewindisch commented Nov 10, 2014

This is still a problem for bind-mounts, so we also need support for escaping.

@acornejo

This comment has been minimized.

Show comment
Hide comment
@acornejo

acornejo Dec 6, 2014

I just hit this bug, is there a known workaround?

acornejo commented Dec 6, 2014

I just hit this bug, is there a known workaround?

@ruffsl

This comment has been minimized.

Show comment
Hide comment
@ruffsl

ruffsl Jan 14, 2015

Just hit this bug as well, quite annoying.

ruffsl commented Jan 14, 2015

Just hit this bug as well, quite annoying.

@frol

This comment has been minimized.

Show comment
Hide comment
@frol

frol May 30, 2015

My worst day ever... hit 5 bugs only today :(
Is there any workaround?

frol commented May 30, 2015

My worst day ever... hit 5 bugs only today :(
Is there any workaround?

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah May 30, 2015

Member

ping @calavera are you working on this for the volumes rewrite?

Member

thaJeztah commented May 30, 2015

ping @calavera are you working on this for the volumes rewrite?

@calavera

This comment has been minimized.

Show comment
Hide comment
@calavera

calavera Jun 1, 2015

Contributor

Nope, it's the first time I see this issue.

Contributor

calavera commented Jun 1, 2015

Nope, it's the first time I see this issue.

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Jun 1, 2015

Member

@calavera this might be a problem, because (I think) the new Volume Driver feature will interpret C: as the (driver-)name? Might have my wires crossed on that, not sure

Member

thaJeztah commented Jun 1, 2015

@calavera this might be a problem, because (I think) the new Volume Driver feature will interpret C: as the (driver-)name? Might have my wires crossed on that, not sure

@calavera

This comment has been minimized.

Show comment
Hide comment
@calavera

calavera Jun 1, 2015

Contributor

I don't have a windows box to test it right now, but I'd expect go's filepath.IsAbs to know what an absolute path is in windows. Nothing has changed in that regard after the refactoring.

Contributor

calavera commented Jun 1, 2015

I don't have a windows box to test it right now, but I'd expect go's filepath.IsAbs to know what an absolute path is in windows. Nothing has changed in that regard after the refactoring.

@cpuguy83

This comment has been minimized.

Show comment
Hide comment
@cpuguy83

cpuguy83 Jun 1, 2015

Contributor

@calavera Correct, it does handle that.

Contributor

cpuguy83 commented Jun 1, 2015

@calavera Correct, it does handle that.

@murphyke

This comment has been minimized.

Show comment
Hide comment
@murphyke

murphyke Sep 24, 2015

+1 In one of my projects, directories on a Linux system are routinely created with colons in their names, and I hurl shellacked gourds around in a most shocking fashion when I have to mount one of these into a container.

murphyke commented Sep 24, 2015

+1 In one of my projects, directories on a Linux system are routinely created with colons in their names, and I hurl shellacked gourds around in a most shocking fashion when I have to mount one of these into a container.

@christoph-kluge

This comment has been minimized.

Show comment
Hide comment
@christoph-kluge

christoph-kluge Sep 30, 2015

+1 as I also hit this bug, just realised after 1 hour of investigating - mostly by accident I added :ro and :rw and then I saw the exception trace which told me it has an incorrect format.

christoph-kluge commented Sep 30, 2015

+1 as I also hit this bug, just realised after 1 hour of investigating - mostly by accident I added :ro and :rw and then I saw the exception trace which told me it has an incorrect format.

@Jaykah

This comment has been minimized.

Show comment
Hide comment
@Jaykah

Jaykah Jan 19, 2016

+1 Having the same problem, but with a CIFS share path via https://github.com/gondor/docker-volume-netshare.

I think that with this plethora of use-cases from Windows support to volumes with custom drivers - escaping is the only reasonable solution.

Jaykah commented Jan 19, 2016

+1 Having the same problem, but with a CIFS share path via https://github.com/gondor/docker-volume-netshare.

I think that with this plethora of use-cases from Windows support to volumes with custom drivers - escaping is the only reasonable solution.

@nbastin

This comment has been minimized.

Show comment
Hide comment
@nbastin

nbastin Mar 25, 2016

+1 they should be able to be escaped. Colons in linux paths are completely legal and it's unreasonable that there is no way to map these into containers.

nbastin commented Mar 25, 2016

+1 they should be able to be escaped. Colons in linux paths are completely legal and it's unreasonable that there is no way to map these into containers.

@RobertAtomic

This comment has been minimized.

Show comment
Hide comment
@RobertAtomic

RobertAtomic Oct 7, 2016

I notice that Docker for Windows has no problem with colons in the file path. Will Docker also gain this ability soon?

RobertAtomic commented Oct 7, 2016

I notice that Docker for Windows has no problem with colons in the file path. Will Docker also gain this ability soon?

@shnhrrsn

This comment has been minimized.

Show comment
Hide comment
@shnhrrsn

shnhrrsn Sep 27, 2017

For anyone else who ends up here via Google, this is now possible via --mount:

docker run --mount type=bind,source=/path/with:colon,destination=/mnt

shnhrrsn commented Sep 27, 2017

For anyone else who ends up here via Google, this is now possible via --mount:

docker run --mount type=bind,source=/path/with:colon,destination=/mnt
@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Sep 27, 2017

Member

Ah, yes, the --mount flag was added to address this issue (i.e., the shorthand -v <src>:<dest> format being ambiguous). This flag was added in this PR; #32251, and included in docker 17.05 and up. Also see the discussion on #28527

I'm closing this issue, because this problem is addressed, but feel free to comment after that

Member

thaJeztah commented Sep 27, 2017

Ah, yes, the --mount flag was added to address this issue (i.e., the shorthand -v <src>:<dest> format being ambiguous). This flag was added in this PR; #32251, and included in docker 17.05 and up. Also see the discussion on #28527

I'm closing this issue, because this problem is addressed, but feel free to comment after that

@thaJeztah thaJeztah closed this Sep 27, 2017

@thaJeztah thaJeztah added this to the 17.05.0 milestone Sep 27, 2017

kshpytsya added a commit to kshpytsya/tinyshar that referenced this issue Apr 25, 2018

Fix splitting of -f cli option for the sake of Windows
Recall Windows having colons in typical absolute paths.
Docker got hit by this as well:
moby/moby#8604
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment