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

Escape backward slashes in synced folder id #296

Merged
merged 1 commit into from
Apr 10, 2017

Conversation

legal90
Copy link
Collaborator

@legal90 legal90 commented Apr 2, 2017

That fixes invalid share names in Windows guests, where backward slash
character is used as a path separator.

Details:
Currently, if we define the synced folder this way (for Windows guest):

config.vm.synced_folder "../", 'C:\Users\vagrant\my_share'

then the share id will be this string: 'C_\Users\vagrant\my_share'
In the guest it will be exposed as \\psf\C_Usersvagrantmy_share - Parallels Desktop tries to replace backslashes with some special character by itself. Meanwhile the Vagrant expects that the
share is available at \\psf\C_Usersvagrantmy_share, so it creates a broken symlink.

This Pull-Request adds \ to the regex, so this char will be replaced with _.
The result share name will be 'C__Users_vagrant_my_share' and it will be available at \\psf\C__Users_vagrant_my_share without any issues.

The similar PR has been sent to VirtualBox provider: hashicorp/vagrant#8433

@romankulikov
Copy link
Collaborator

romankulikov commented Apr 4, 2017

Two points from my side:

  • Why does Vagrant expects path \\psf\C_Usersvagrantmy_share (without special Unicode symbols)? May be this the issue in Vagrant?
  • Parallels Desktop also remaps other symbols: ", *, :, <, >, ?, | – may they should be handled as well?

@legal90
Copy link
Collaborator Author

legal90 commented Apr 8, 2017

  • Parallels Desktop also remaps other symbols: ", *, :, <, >, ?, | – may they should be handled as well?

Yes, good point! Currently we handle only : and / here. I'll add other characters too.

  • Why does Vagrant expects path \\psf\C_Usersvagrantmy_share (without special Unicode symbols)? May be this the issue in Vagrant?

At the early stage, before booting the VM, our provider configures the VM by setting the list of shared folders. It calls driver method #share_folders, which actually doesprlctl set <vm_uuid> --shf-host-add <share_name> --path <host_path>. For <share_name> it uses the value of :name param if it is set explicitly in Vagrantfile. Otherwise the guest's path will be used.
Before putting the share name to the CLI call, our provider replaces special characters there:

After the VM boot Then right before mounting shared folders in the guest, provider determines the share name from the output of drivers method #read_shared_folder, e.q. from the output of prlctl list -i <vm_uuid> --json:

id = shf_config.key(data[:hostpath])

In our example prlctl returns the following:

...
			"C_\Users\vagrant\my_share": {
				"enabled": true,
				"path": "\/Users\/legal\/Documents",
				"mode": "rw"
			},
...

Since backslashes are not escaped in the output (which looks like a bug of prlctl, BTW), the key is decoded exactly as a string 'C_Usersvagrantmy_share'.
So, the expected share name is already "broken" at this point.
But on the other hand, there is an internal remapping made by Parallels Desktop in Windows guest (it becomes 'C_Usersvagrantmy_share') and Vagrant knows nothing about that.
So here we should just avoid all cases when the actual share name is different from the expected one.

That fixes invalid share names in Windows guests, where backward slash
character is used as a path separator.
@legal90
Copy link
Collaborator Author

legal90 commented Apr 9, 2017

I've updated PR by adding more characters to the escaping regex.
Also I've updated my previous comment: shared folders configuring and mounting happens subsequently, after the VM boot.

@romankulikov
Copy link
Collaborator

Ok. Thanks for detailed explanation!

@romankulikov romankulikov merged commit 68e455f into Parallels:master Apr 10, 2017
@legal90 legal90 deleted the fix-synced-folder-id branch April 10, 2017 12:22
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.

2 participants