actions / upload-release-asset

An Action to upload a release asset via the GitHub Release API
MIT License
683 stars 187 forks source link

Corrupting archive on upload #1

Closed EHadoux closed 4 years ago

EHadoux commented 4 years ago

Hello,

whenever I use the action for uploading an archive, the resulting archive upload is a 10 bytes corrupted one.

My process and checks are as follows:

  1. building
  2. tar cvzf dist.tgz my_dir and indeed the output shows the files being added
  3. Create release: strict cut and paste but with @ master instead of @v1.0.0
  4. run: ls -l which gives me -rw-r--r-- 1 runner docker 2043145 Oct 10 15:51 dist.tgz so the archive has indeed files in it
  5. Upload release asset, likewise with @ master and a few changes:
    asset_path: ./dist.tgz
    asset_name: dist.tgz
    asset_content_type: application/gzip

    When I go on the release that has been created, I see the two source code archives and the dist.tgz archive that is 10 bytes according to the interface. If I download it and try to tar x I get that it's corrupted.

I've tried with a zip archive and application/zip to no avail.

Do you have any idea where this might come from? Looking at this repo code, I couldn't find an issue but only guess that it might come from where the content-length is calculated.

Thanks!

IAmHughes commented 4 years ago

Hi @EHadoux, thank you for the report. I'll have to take a look at this when I have some time and see if I can find better logic around the content length, potentially.

This is the line in which we calculate the content length, in bytes, for reference.

@EHadoux do you know what the size of the .tgz is locally if you ran tar cvzf dist.tgz my_dir on your machine? I may need to debug the way the size field on the fs node module's Stat object works. It's a synchronous function, so there's not an issue with await being needed that I can tell.

EHadoux commented 4 years ago

Hey @IAmHughes,

thanks for getting back so quickly.

Interestingly, the size on my machine is a tiny bit bigger. It's a Vue app getting compiled so I can see the size of each file after compilation that is going into the archive and they are identical to 2 decimals in KB (it's rounded or truncated there). In the Actions env the archive is 2043145 B and on my machine (Mac host, compiled in a node:alpine container) it's 2049937 B. I don't know if it makes much of a difference w.r.t. this issue though.

Please let me know if you need more. Thanks again!

IAmHughes commented 4 years ago

Thanks @EHadoux, I may need to do some more research on my own when I get a chance to figure out what's causing the issue. If you do happen to do the research and want to contribute, PRs are VERY much welcome! ❤️

Our CONTRIBUTING guide would detail specifics, but you can fork the repo and make changes, then submit the PR if you'd like. 🎉

jhelwig commented 4 years ago

I'm getting very similar behavior with this action. The resulting release files are plain text files that contain the string that is passed in for asset_path, instead of the content of the file at the asset_path.

jhelwig commented 4 years ago

Looking at how assetPath is used, this behavior is making sense. Looking at the integration tests for octokit/rest.js, it looks like uploadReleaseAsset expects the content of the file, instead of the name of the file on disk to read from.

IAmHughes commented 4 years ago

Thank you both for the information and then a quick PR too @EHadoux! I'll be reviewing it today, thank you!