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

I can't see any warning when downloading a file with a bad MD5 hash #1212

Closed
JohnRusk opened this issue Mar 5, 2019 · 9 comments

Comments

@JohnRusk
Copy link

@JohnRusk JohnRusk commented Mar 5, 2019

Storage Explorer Version: 1.6.2
Platform/OS: Windows 10
Architecture: x64

Bug description
I downloaded a file for which I know the MD5 hash is wrong. (I used a different tool to uploaded with, deliberately, the wrong hash). I don't see any warning in the Storage Explorer UI. In "activities" at the bottom of the screen, it says "Downloaded 'badhash.dat' to "C:\users\…\Downloads\…"

Steps to Reproduce
Covered above. Email me if you'd like a SAS pointing to the file in question.

Expected Experience
A visible warning that the file is, or may be, corrupt.

Actual Experience
As above

Additional Context

@MRayermannMSFT MRayermannMSFT added this to the 1.8.0 milestone Mar 6, 2019
@MRayermannMSFT MRayermannMSFT added this to Committed in Storage Explorer via automation Mar 6, 2019
@MRayermannMSFT

This comment has been minimized.

Copy link
Member

@MRayermannMSFT MRayermannMSFT commented Mar 15, 2019

@JohnRusk are you working with blobs, files, or adlsgen2 blobs?

@MRayermannMSFT MRayermannMSFT self-assigned this Mar 15, 2019
@JohnRusk

This comment has been minimized.

Copy link
Author

@JohnRusk JohnRusk commented Mar 15, 2019

@MRayermannMSFT

This comment has been minimized.

Copy link
Member

@MRayermannMSFT MRayermannMSFT commented Mar 15, 2019

Hmmmm interesting. So quick summary of my findings today:

  1. If not using AzCopy preview feature, then the download never finished. This is due to a bug in the Storage SDK. So maybe yours finishing despite the bad hash is related to that issue? Hard to tell since I can't repro what you're seeing. If you don't mind, I'll let you know once I have a version of the SDK that contains a fix -> I'll produce a private build of Storage Explorer with it -> you can see if you get the MD5 error.
  2. If using AzCopy preview feature, there will never be an error. This is because we didn't enable MD5 checking yet for the AzCopy preview. We'll go ahead and make sure that is enabled for 1.8.0. 👍
@JohnRusk

This comment has been minimized.

Copy link
Author

@JohnRusk JohnRusk commented Mar 15, 2019

@MRayermannMSFT MRayermannMSFT moved this from Committed to Blocked in Storage Explorer Mar 15, 2019
@MRayermannMSFT

This comment has been minimized.

Copy link
Member

@MRayermannMSFT MRayermannMSFT commented Apr 11, 2019

Still waiting on the SDK team to release the fix. In the meantime, in 1.8.0, AzCopy will give errors if the MD5s don't match. Moving to 1.9.0 as I don't think the SDK team will make our release. =/

@MRayermannMSFT MRayermannMSFT modified the milestones: 1.8.0, 1.9.0 Apr 11, 2019
@JohnRusk

This comment has been minimized.

Copy link
Author

@JohnRusk JohnRusk commented Apr 11, 2019

Thanks for the update @MRayermannMSFT.
Two quick points, for your info:

  1. By the way, the approach we have settled on for AzCopy v10, when downloading, is that it should validate the MD5 if Content-MD5 is present on the source blob. We have an option to turn that validation off if desired - e.g. if a page blob has been mounted as a (unmanaged?) disk, it will never have a valid MD5 because its content gets changed after upload.

  2. Maybe you should also be aware that, as of the current RC, which is 10.0.9, AzCopy defaults to NOT calculating the MD5 on upload. That's the opposite of previous versions of AzCopy and (maybe, I guess) the opposite of what Storage Explorer is currently doing.

@MRayermannMSFT

This comment has been minimized.

Copy link
Member

@MRayermannMSFT MRayermannMSFT commented Apr 16, 2019

@JohnRusk , when you say we, are you talking about yourself? Also, unfortunately we don't have the option/don't expose an option to disable MD5 (either on a file by file basis, or as an option present in say the download dialog). But it is something we will add when we get to doing settings (which should be soon?). And yep! I am aware of that behavior in 10.0.9. We've reacted accordingly and will always be adding the failIfDifferent option when running AzCopy. Hmmm maybe we should allow people to retry with that option off, similar to overwrite....I'll think about that!

@JohnRusk

This comment has been minimized.

Copy link
Author

@JohnRusk JohnRusk commented Apr 16, 2019

@MRayermannMSFT, sorry, it's not clear from my signature. (Maybe I should fix that). I'm at Microsoft, working on AzCopy v10, and personally wrote the MD5 support.

@MRayermannMSFT MRayermannMSFT modified the milestones: 1.9.0, 1.10.0 May 20, 2019
@MRayermannMSFT MRayermannMSFT moved this from Blocked to Committed in Storage Explorer May 22, 2019
@jinglouMSFT

This comment has been minimized.

Copy link

@jinglouMSFT jinglouMSFT commented Jul 26, 2019

We eventually will use AzCopy to replace our JS implementation for stability and performance. If we don't get the SDK fix before we use AzCopy by default, there is not much value to fix this bug. Our current AzCopy behavior is correct already, so won't fix this bug in our JS implementation.

Storage Explorer automation moved this from Committed to Done Jul 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
3 participants
You can’t perform that action at this time.