search

sintaxi / dbox

NodeJS SDK for Dropbox API (THIS LIBRARY IS OBSOLETE!!)
514 stars 91 forks source link

path.join #66

Open ghost opened 11 years ago

ghost commented 11 years ago

Currently helper.url uses path.join to build a uri. This doesn't work in windows as path.join uses the path.sep which is "\" in windows. Urls then get built like http:\google.com\anotherpath\

http://nodejs.org/api/path.html http://nodejs.org/api/path.html#path_path_sep

ghost commented 11 years ago

The general consensus is that path.join should be avoided for building uri's

https://github.com/joyent/node/issues/2216

ghost commented 11 years ago

I created the following pull request if you would like to take on this fix.

https://github.com/sintaxi/node-dbox/pull/67

paultanner commented 11 years ago

I was getting the same error with node v0.10.7 on a Mac (Snow Leopard) I applied your fix to see if it would help. It fixed the call to client.account :-) However, I get a status 400 and an empty array back from client.readdir (with parameter "Finse") (That folder has several files in it) Checking the fullpath I see this: api.dropbox.com/1/metadata/sandbox/undefinedFinse This tells me that the scopepath is undefined. Thought you might know how to find the cause of this? I also wondered if your fix might have a slash missing? Thx. Paul

sintaxi commented 11 years ago

Gents, sorry I have been inactive on this issue.

I'm not a fan of removing the path.join with string concatenation as it doesn't handle double slashes and things like that. I would like to have something more robust (ideally that supports both platforms).

@paultanner @DefinitelyCarter are you guys both running dbox on windows?

paultanner commented 11 years ago

I am using a mac osx (snow leopard) node v0.10.7. Planning to move to debian linux when this is working. the app instance I created at dbox is still in sandbox mode.

I appreciate that the suggested fix was intended for Windows folks. I thought it worth trying as I was getting the exact same error message.

\ on further investigation **

The version in the NPM repo worked without a change on debian linux. My problem (status 400s above) is unrelated to this issue. However, my experience seems to confirm that path.join problem exists on OS/X.

Paul