volojs / volo

Create front end projects from templates, add dependencies, and automate the resulting projects
https://volojs.github.io/
Other
1.41k stars 100 forks source link

Adding a specific file from a zipball doesn't work for GitHub build artifacts #194

Closed lambdabaa closed 10 years ago

lambdabaa commented 10 years ago

I am trying to do

volo add https://github.com/gaye/dav/releases/download/v1.0.3/dav.zip#dav.js

which fails with

Downloading: https://s3.amazonaws.com/github-cloud/releases/20107688/bd83b39a-f8d8-11e3-9f07-c5afa11724a9.zip?response-content-disposition=attachment%3B%20filename%3Ddav.zip&AWSAccessKeyId=AKIAISTNZFOVBIJMK3TQ&Expires=1403312939&Signature=E4eeWhBREYflaZdxaefBRnBXZQo%3D
Error: ENOENT, no such file or directory '/tmp/temp-dav-1403312878895-1/download/dav.js'

Interestingly, volo's archive lib does a HEAD request on https://github.com/gaye/dav/releases/download/v1.0.3/dav.zip then gets redirected to the s3 url. Then it does a HEAD on the s3 url and s3 responds (wrongly) that the content-type for the zipball is application/xml. Then volo gets confused here

https://github.com/volojs/volo/blob/db7d0f65a2592ddc3b4f7e31c41c7d147b0edd8d/lib/archive.js#L125

and thinks that what I'm trying to download is not an archive. I think that there's an issue with s3's HEAD implementation, but I wonder if it would be reasonable to patch volo to handle cases in which things that really are zipballs get served up by web servers which don't set application/zip correctly.

Thoughts @jrburke? @millermedeiros?

jrburke commented 10 years ago

On first look, using an manual HTTP send/receive tool, a GET for the Location provided in the 302, Amazon returns an error, which is why the content type is XML:

<?xml version="1.0" encoding="UTF-8"?>
<Error><Code>SignatureDoesNotMatch</Code><Message>The request signature we calculated does not match the signature you provided. Check your key and signing method.</Message>
[more XML goop here]

So I am trying to figure out why the URL generated from GitHub is not good on its own, or if I need to send something else or re-encode something. However, my understanding is that Location URLs should just be sent as-is. The location is on a different domain so I do not think it is about sending a cookie with the request either.

jrburke commented 10 years ago

Looks like the HTTP tool I was using was double URL-encoding the query string, so the bug is likely similar in volo. Should have a fix later today or tomorrow.

lambdabaa commented 10 years ago

Thanks @jrburke!

jrburke commented 10 years ago

Looks like it was just the HEAD instead of GET request that the signed AWS URLs do not like, so I just went a GET path for those URLs. Tagged and released on npm as version 0.3.3.

lambdabaa commented 10 years ago

Thanks so much! I appreciate the speedy help :racehorse: