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

Bad code at filestore.go #84

Closed
alexandrevicenzi opened this issue Dec 15, 2016 · 1 comment
Closed

Bad code at filestore.go #84

alexandrevicenzi opened this issue Dec 15, 2016 · 1 comment

Comments

@alexandrevicenzi
Copy link

In the lines above the path is concatenated with /.

https://github.com/tus/tusd/blob/master/filestore/filestore.go#L187
https://github.com/tus/tusd/blob/master/filestore/filestore.go#L192

I think this will fail on Windows. And if I set the Path with slash it will add two slash. Most Linux don't break because of this, but the correct way is to use path.Join function to concat paths.

If there's no specific reason for this I can send a PR to fix it.

@Acconut
Copy link
Member

Acconut commented Dec 16, 2016

This problem is not a big as you think. Windows is smart enough to also recognize / as a path delimiter and is capable of handling multiple slashes repeatedly in it. Later one also applies for Unix system, I believe. To prove this, we actually test tusd on Windows and the results are usually green, e.g. https://ci.appveyor.com/project/Acconut/tusd/build/1.0.293.

However, there is no specific reason and it would be great if you could send a PR for this. Please also remember to update the corresponding tests as they might break by this change.

@Acconut Acconut closed this as completed in 19c9576 Jan 4, 2017
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

No branches or pull requests

2 participants