Skip to content

[Feature] Consider verifying checksum of Openstack Storage upload #236

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.

Sign up for GitHub

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

Closed
rossj wants to merge 1 commit into from
Closed

[Feature] Consider verifying checksum of Openstack Storage upload #236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions docs/providers/rackspace/storage.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ client.getFile(myContainer, 'my-file', function(err, file) { ... });

Returns a writeable stream. Upload a new file to a [`container`](#container-model). `result` will be `true` on success.

The MD5 checksum of the provided file or stream is calculated on the client side, and compared to the checksum calculated by Rackspace. This ensures round-trip data integrity.

To upload a file, you need to provide an `options` argument:

```Javascript
Expand Down
51 changes: 38 additions & 13 deletions lib/pkgcloud/openstack/storage/client/files.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ var fs = require('fs'),
base = require('../../../core/storage'),
pkgcloud = require('../../../../pkgcloud'),
_ = require('underscore'),
digestStream = require('digest-stream'),
storage = pkgcloud.providers.openstack.storage;

/**
Expand Down Expand Up @@ -65,18 +66,9 @@ exports.upload = function (options, callback) {
options = {};
}

//
// Optional helper function passed to `this.request`
// in the case when no callback is passed to `.upload(options)`.
//
function onUpload(err, body, res) {
return err
? callback(err)
: callback(null, true, res);
}
callback = callback || function () {};

var container = options.container,
success = callback ? onUpload : null,
self = this,
apiStream,
inputStream,
Expand Down Expand Up @@ -128,12 +120,45 @@ exports.upload = function (options, callback) {
self.serializeMetadata(self.OBJECT_META_PREFIX, options.metadata));
}

apiStream = this.request(uploadOptions, success);
//
// Before the upload finishes, digestCallback will get called and store
// the md5 hash calculated on this side.
//
var md5Hash;
var digestCallback = function(digest, length) {
md5Hash = digest;
};

apiStream = this.request(uploadOptions, function (err, body, res) {
if (err) {
return callback(err);
}

// Verify the md5 hash
if (!res.headers.etag || res.headers.etag === md5Hash) {
return callback(null, true, res);
}

// The returned checksum does not match what was calculated on client side, remove file
self.removeFile(container, options.remote, function (err) {
if (err)
Copy link
Member

@kenperkins kenperkins Feb 20, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels brittle to me. Obviously our service should be highly available, but the fact that we upload the file, then check and delete feels inverted.

I guess there's not much we can do if you want to not have to buffer the entire file locally to generate the hash as part of the inbound request.

// There was a checksum mismatch, but also error removing the file. This leaves us in a weird state.
err = new Error('Checksum mismatch during upload. File not removed.');
else
err = new Error('Checksum mismatch during upload. File removed.');

callback(err);
});
});

var md5Stream = digestStream('md5', 'hex', digestCallback);
md5Stream.pipe(apiStream);

if (inputStream) {
inputStream.pipe(apiStream);
inputStream.pipe(md5Stream);
}

return apiStream;
return md5Stream;
};

/**
Expand Down
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@
"request": "2.22.x",
"underscore": "1.4.x",
"url-join": "0.0.x",
"utile": "0.x.x"
"utile": "0.x.x",
"digest-stream": "0.2.x"
},
"devDependencies": {
"hock" : "0.2.x",
Expand Down
96 changes: 96 additions & 0 deletions test/rackspace/storage/storage-object-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,102 @@ describe('pkgcloud/rackspace/storage/storage-object', function () {
});
});

it('upload should complete successfully when returned checksum matches - local file upload', function (done) {
var filepath = __dirname + '/../../fixtures/fillerama.txt';

if (mock) {
server
.put('/v1/MossoCloudFS_00aa00aa-aa00-aa00-aa00-aa00aa00aa00/0.1.7-215/upload.txt', fs.readFileSync(filepath, 'utf8'))
.reply(201, '', {
ETag: '9b0d0a115145c13f1281818adc2bbcbe'
});
}

client.upload({
container: '0.1.7-215',
remote: 'upload.txt',
local: filepath
}, function (err, result) {
should.not.exist(err);
server && server.done();
done();
});
});

it('upload should complete successfully when returned checksum matches - stream upload', function (done) {
var filepath = __dirname + '/../../fixtures/fillerama.txt';

if (mock) {
server
.put('/v1/MossoCloudFS_00aa00aa-aa00-aa00-aa00-aa00aa00aa00/0.1.7-215/upload.txt', fs.readFileSync(filepath, 'utf8'))
.reply(201, '', {
ETag: '9b0d0a115145c13f1281818adc2bbcbe'
});
}

var stream = client.upload({
container: '0.1.7-215',
remote: 'upload.txt'
}, function (err, result) {
should.not.exist(err);
server && server.done();
done();
});

fs.createReadStream(filepath).pipe(stream);
});

it('upload should remove file and return error on checksum mismatch - local file upload', function (done) {
var filepath = __dirname + '/../../fixtures/fillerama.txt';

if (mock) {
server
.put('/v1/MossoCloudFS_00aa00aa-aa00-aa00-aa00-aa00aa00aa00/0.1.7-215/upload.txt', fs.readFileSync(filepath, 'utf8'))
.reply(201, '', {
ETag: '12bad12bad12bad12bad12bad12bad12'
})
.delete('/v1/MossoCloudFS_00aa00aa-aa00-aa00-aa00-aa00aa00aa00/0.1.7-215/upload.txt')
.reply(204);
}

client.upload({
container: '0.1.7-215',
remote: 'upload.txt',
local: filepath
}, function (err, result) {
should.exist(err);
err.should.be.an.instanceOf(Error);
server && server.done();
done();
});
});

it('upload should remove file and return error on checksum mismatch - stream upload', function (done) {
var filepath = __dirname + '/../../fixtures/fillerama.txt';

if (mock) {
server
.put('/v1/MossoCloudFS_00aa00aa-aa00-aa00-aa00-aa00aa00aa00/0.1.7-215/upload.txt', fs.readFileSync(filepath, 'utf8'))
.reply(201, '', {
ETag: '12bad12bad12bad12bad12bad12bad12'
})
.delete('/v1/MossoCloudFS_00aa00aa-aa00-aa00-aa00-aa00aa00aa00/0.1.7-215/upload.txt')
.reply(204);
}

var stream = client.upload({
container: '0.1.7-215',
remote: 'upload.txt'
}, function (err, result) {
should.exist(err);
err.should.be.an.instanceOf(Error);
server && server.done();
done();
});

fs.createReadStream(filepath).pipe(stream);
});

it('extract should ask server to extract the uploaded tar file', function(done) {

var data = "H4sIABub81EAA+3TzUrEMBAH8CiIeNKTXvMC1nxuVzx58CiC9uBNam1kQZt1N4X1XXwDX9IJXVi6UDxo6sH/D4akadJOmY7z/owlJkhubTdOulEo040dJpXMTS6tjuuSriTjNnViUbsM5YJztvCPs+atHdxH25wbI6FxOap/9hDqZcjCKqR5RyzwxJjB+iurN/WXiuqvpdGMizTp9P3z+rO94322y9h1WfGbO37P1+IaO6BQFO8U8fqzd/Jo6JGXRXG7nsYTHxSHW1t2NusnlX/Nyvn8pc6KehWumso/zZpnutkGdzq9kNrQv3E+Nb/yudAX+z9t93/f/0LIrf5XNEP/j0H+dQIAAAAAAAAAAAAAAAAAAADwY194ELb5ACgAAA==";
Expand Down