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

Revert "fix: revert skip validation" #1721

Merged
merged 1 commit into from
Nov 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions src/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1272,7 +1272,7 @@ class File extends ServiceObject<File> {

const throughStream = streamEvents(new PassThrough());

let isCompressed = true;
let isServedCompressed = true;
let crc32c = true;
let md5 = false;

Expand Down Expand Up @@ -1379,7 +1379,7 @@ class File extends ServiceObject<File> {
rawResponseStream.on('error', onComplete);

const headers = rawResponseStream.toJSON().headers;
isCompressed = headers['content-encoding'] === 'gzip';
isServedCompressed = headers['content-encoding'] === 'gzip';
const throughStreams: Writable[] = [];

if (shouldRunValidation) {
Expand All @@ -1400,7 +1400,7 @@ class File extends ServiceObject<File> {
throughStreams.push(validateStream);
}

if (isCompressed && options.decompress) {
if (isServedCompressed && options.decompress) {
throughStreams.push(zlib.createGunzip());
}

Expand Down Expand Up @@ -1440,13 +1440,23 @@ class File extends ServiceObject<File> {
return;
}

if (!isCompressed) {
// TODO(https://github.com/googleapis/nodejs-storage/issues/709):
// Remove once the backend issue is fixed.
// If object is stored compressed (having
// metadata.contentEncoding === 'gzip') and was served decompressed,
// then skip checksum validation because the remote checksum is computed
// against the compressed version of the object.
if (!isServedCompressed) {
try {
await this.getMetadata({userProject: options.userProject});
} catch (e) {
throughStream.destroy(e);
return;
}
if (this.metadata.contentEncoding === 'gzip') {
throughStream.end();
return;
}
}

// If we're doing validation, assume the worst-- a data integrity
Expand Down
27 changes: 27 additions & 0 deletions system-test/storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2502,6 +2502,33 @@ describe('storage', () => {
});
});

it('should skip validation if file is served decompressed', async () => {
const filename = 'logo-gzipped.png';
await bucket.upload(FILES.logo.path, {destination: filename, gzip: true});

tmp.setGracefulCleanup();
const {name: tmpFilePath} = tmp.fileSync();

const file = bucket.file(filename);

await new Promise((resolve, reject) => {
file
.createReadStream()
.on('error', reject)
.on('response', raw => {
assert.strictEqual(
raw.toJSON().headers['content-encoding'],
undefined
);
})
.pipe(fs.createWriteStream(tmpFilePath))
.on('error', reject)
.on('finish', resolve);
});

await file.delete();
});

describe('simple write', () => {
it('should save arbitrary data', done => {
const file = bucket.file('TestFile');
Expand Down
18 changes: 18 additions & 0 deletions test/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1387,6 +1387,24 @@ describe('File', () => {
file.requestStream = getFakeSuccessfulRequest(data);
});

describe('server decompression', () => {
it('should skip validation if file was stored compressed', done => {
file.metadata.contentEncoding = 'gzip';

const validateStub = sinon.stub().returns(true);
fakeValidationStream.test = validateStub;

file
.createReadStream({validation: 'crc32c'})
.on('error', done)
.on('end', () => {
assert(validateStub.notCalled);
done();
})
.resume();
});
});

it('should emit errors from the validation stream', done => {
const error = new Error('Error.');

Expand Down