fix: revert skip validation

[shaffeeullah]

Fixes #709

[frankyn]

One more comment.

[shaffeeullah]

@frankyn i think this system test is failing because it's not returning the 'x-goog-stored-content-encoding' header. is that supposed to happen automatically?

[danielbankhead]

I think we need some integration tests to associate with the new behavior

[frankyn]

One last nit, @shaffeeullah, this will be blocked until the header is no longer allowlist required.

Otherwise, this LGTM! Thank you!

[shaffeeullah]

I think we need some integration tests to associate with the new behavior

There are already integration tests associated with this new boolean.

False case:

nodejs-storage/system-test/storage.ts

Line 2181 in 0845525

it('should skip validation if file is served decompressed', async () => {

True case:

nodejs-storage/system-test/storage.ts

Line 2148 in 0845525

it('should upload a gzipped file and download it', async () => {

There are also a few unit tests verifying that validation happens correctly. (Let me know if you'd like me to link those. I know they exist because I originally broke them with this change; they're doing good work!)

[shaffeeullah]

One last nit, @shaffeeullah, this will be blocked until the header is no longer allowlist required.

@frankyn What is the timeline for that?

[frankyn]

One more comment; otherwise LGTM, we can't merge this until the feature is available in production to everyone unless y'all are planning making a release until then.

[frankyn]

I think i got confused with failed variable; what does it do?

[shaffeeullah]

failed defaults to true (if crc32c || md5) and then later gets changed to false if it passes. (see below lines.) It's not true that it failed if it wasnt safeToValidate to begin with, so we need to make sure that's true before assuming failed

[shaffeeullah]

@frankyn says this is launched, clear to merge