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

double leading slash when uploading a file to a bucket #1972

Closed
kilianc opened this issue Feb 7, 2017 · 5 comments
Closed

double leading slash when uploading a file to a bucket #1972

kilianc opened this issue Feb 7, 2017 · 5 comments
Assignees
Labels
api: storage Issues related to the Cloud Storage API. type: question Request for information or clarification. Not an issue.

Comments

@kilianc
Copy link

kilianc commented Feb 7, 2017

Environment details

  • OS: GKE
  • Node.js version: 7
  • npm version: 4.0.5
  • google-cloud-node version: "@google-cloud/storage": "^0.5.0"

Steps to reproduce

const bucket = Storage().bucket('gs://my-name')
const file = bucket.file('/foo/bar/lol.txt')
const readStream = fs.createReadStream('./lol.txt')
const writeStream = file.createWriteStream({ metadata: { contentType: 'text/plain' } })
readStream.pipe(writeStream)

Given the above code it uploads a file at gs://my-name//foo/bar/lol.txt

@stephenplusplus
Copy link
Contributor

The bucket name should simply be my-name, not its URL:

- const bucket = Storage().bucket('gs://my-name')
+ const bucket = Storage().bucket('my-name')

And same for the file method:

- const file = bucket.file('/foo/bar/lol.txt')
+ const file = bucket.file('foo/bar/lol.txt')

With these changes, the file uploaded should land at: gs://my-name/foo/bar/lol.txt.

I don't think it'd be a big change if our library auto-removed any leading slashes from the input to bucket.file(), but given the explanation above of the correct way to use these methods, do you think it's necessary?

@stephenplusplus stephenplusplus added the api: storage Issues related to the Cloud Storage API. label Feb 7, 2017
@kilianc
Copy link
Author

kilianc commented Feb 7, 2017

@stephenplusplus I was using only my-name indeed, I messed up while trying to extract the use case.

given the explanation above of the correct way to use these methods, do you think it's necessary?

Since I am not feeding the library anywhere a double leading slash it is very strange that this is the outcome. I mean my-name + /foo -> my-name/foo. I understand the other way around where in case of a missing leading slash it's assumed the user means root.

In my opinion this is straight up a bug, I can't think of anybody that needs to upload a file to something//file.txt IMHO.

🍻

@stephenplusplus
Copy link
Contributor

You're right, that is weird. I'll mark as help wanted, so if anybody wants to strip leading / from within the File constructor, please help!

@stephenplusplus
Copy link
Contributor

Oh, and 🍻

@kilianc
Copy link
Author

kilianc commented Feb 7, 2017

I am looking at the code, I'll def try to help

@danoscarmike danoscarmike added this to Cloud Storage in First 4 (GA) Feb 21, 2017
@danoscarmike danoscarmike added priority: p2 Moderately-important priority. Fix may not be included in next release. type: question Request for information or clarification. Not an issue. labels Feb 27, 2017
@lukesneeringer lukesneeringer removed the priority: p2 Moderately-important priority. Fix may not be included in next release. label May 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the Cloud Storage API. type: question Request for information or clarification. Not an issue.
Projects
No open projects
Development

No branches or pull requests

5 participants