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

Allow special characters (like #) #71

Closed
wants to merge 1 commit into from
Closed

Allow special characters (like #) #71

wants to merge 1 commit into from

Conversation

nickmisasi
Copy link

Currently the module does not support uploading files with a name like Data #1.csv. I've added functionality to parse the link and encode any special characters before it makes any requests to the Webav server.

@nickmisasi
Copy link
Author

Fixes #66

return url;
}

function request(fullUrl, options) {
Copy link
Owner

Choose a reason for hiding this comment

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

I wouldn't recommend changing this file to perform this work. For one, splitting the URL like this is a bit redundant and unsafe. Ultimately only the path needs encoding (here), which could be done in the interface files like putFile.js.

Ideally there'd be some helper method to perform the encoding like encodeRequestPath(path) that you could use on all get/put/mkdir requests.

Copy link
Author

@nickmisasi nickmisasi Feb 9, 2018

Choose a reason for hiding this comment

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

I tried a bunch of different splitting methods, particularly url = new URL(...) however since something like
https://webdav-server.com/webdav/file #1.json has the # character, the path is only parsed up to that point (#1.json is removed). How would you go about parsing?

I'll look into moving the encoding around. I went this way to keep the code more DRY.

@perry-mitchell
Copy link
Owner

Thanks for submitting @nickmisasi13 - I've commented regarding where the work has been done. I would prefer it to be performed in the actual request methods themselves rather than the request getter. Also the tests are failing, which must pass for a review to go through.

@perry-mitchell
Copy link
Owner

Thanks for the work, but unfortunately I'm going to close this one in favour of #76. The new PR addresses the encoding for only the paths.

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.

3 participants