scality / cloudserver

Zenko CloudServer, an open-source Node.js implementation of the Amazon S3 protocol on the front-end and backend storage capabilities to multiple clouds, including Azure and Google.
https://www.zenko.io/cloudserver
Apache License 2.0
1.71k stars 241 forks source link

Multipart upload: Failure on non sequential part numbers #789

Closed mikeys closed 7 years ago

mikeys commented 7 years ago

Description

On multipart upload: the upload fails when using non sequential part numbers.

Based on AWS S3 specification: "Amazon S3 creates an object by concatenating the parts in ascending order based on the part number"

That means that potentially you can have part numbers which are non sequential (1, 2, 3, 7) and as long as the Complete Multipart Upload request lists those parts, everything should still work correctly (the concatenation of the parts is made based on the part number ascending order).

Specifically (in lib/api/completeMultipartUpload.js):

for (let i = 0; i < partLength; i++) {
    const part = jsonList.Part[i];
    const partNumber = Number.parseInt(part.PartNumber[0], 10);
    if (partNumber !== i + 1) {     
        // If not in order return InvalidPartOrder      
        if (partNumber <= partLength) {     
            return next(errors.InvalidPartOrder, destBucket);       
        }       
        // If missing part return InvalidPart       
        return next(errors.InvalidPart, destBucket);        
    }

The above seems incorrect since it requires the partNumber to go sequentially starting from 1.

Steps to reproduce the issue

  1. Simply initiate a multipart upload in one of your favorite AWS S3 SDKs
  2. When using upload.add_part, assign larger but non sequential part numbers to sequential file parts (1: file_part1, 3: file_part2, 4: file_part3).
  3. InvalidPart error is raised.

Expected result: Upload should be successful - concatenation of parts based on ascending part number order.

Additional information: (Node.js version, Docker version, etc)

Node.js v6.9.5 without Docker.

Possible fix

const _partNumberFromPart = (part) => Number.parseInt(part.PartNumber[0], 10);
const _partNumberComparer = (a, b) => _partNumberFromPart(a) - _partNumberFromPart(b);

function parsePartsList(destBucket, objMD, mpuBucket, next) {
    if (request.post) {
        return parseXml(request.post, (err, jsonList) => {
            if (err) {
                return next(err, destBucket);
            }

             const parts = jsonList.Part;

            // "Amazon S3 creates an object by concatenating the parts in ascending
            // order based on the part number"
            jsonList.Part = parts.sort(_partNumberComparer);

            return next(err, destBucket, objMD, mpuBucket, jsonList);
        });
    }
    return next(errors.MalformedXML, destBucket);
}

for (let i = 0; i < partLength; i++) {
    const part = jsonList.Part[i];
    // If the complete list of parts sent with
    // the complete multipart upload request is not
    // in numerical order
    // return an error
    const partNumber = Number.parseInt(part.PartNumber[0], 10);
    // if (partNumber !== i + 1) {       
        // If not in order return InvalidPartOrder        
        // if (partNumber <= partLength) {       
        //    return next(errors.InvalidPartOrder, destBucket);     
        // }     
        // If missing part return InvalidPart     
        //return next(errors.InvalidPart, destBucket);      
    // }
nicolas2bert commented 7 years ago

Hi @mikeys,

Thanks for opening this issue and writing a possible fix. We are currently working on reproducing and fixing this issue.

I will get back to you soon with any update on this.

Cheers

mikeys commented 7 years ago

@nicolas2bert 👍 Note that the above works with actual AWS S3 fine.

LaurenSpiegel commented 7 years ago

Fixed by #802