service/s3/s3manager: Fix uploader to check for empty part before max parts check

[michaelf-stratoscale]

…multiparts

[michaelf-stratoscale]

reference to an issue will be added shortly

[michaelf-stratoscale]

#2557

[jasdel]

Thanks for taking the time to create this PR @michaelf-stratoscale. The logic makes sense and looks to fix the issue with the max parts corresponds with the reader's EOF exactly. I agree this is a bug that needs to be fixed. I think there are two potentially negative behavior that could result:

• If the input reader's is larger than MaxUploadParts*PartSize the uploader will read an additional up to PartSize before deciding to throw the read part chunk away since number of parts has reached MaxUploadParts.

• If the input reader is an unbounded stream (i.e. no length and not seekable) the uploader would block reading the input reader until EOF or PartSize bytes were available. Since the uploader doesn't have a ContentLength parameter the only ways to control how many bites are uploaded is with a bounded reader, or manipulating MaxUploadParts.

Both of these situations require the user to be using MaxUploadParts to control the total size of content uploaded, which is a fragile method since objects would need to be PartSize*N size. Using an io.LimitReader would provide a much safer method of accomplishing this goal.

[jasdel]

Made a minor update clarifying the role of MaxUploadParts, and how it must not be used as a tool to limit the total size of the object uploaded.

[michaelf-stratoscale]

Hey @jasdel in the example that i wrote in the issue #2557 i set MaxUploadParts in my production code i don't really set it, i just use the default, my only edge case is when the data that i get is in exactly 10K parts and because i use io.MultiReader i get 10K+1 parts.

In any case the code looks good. Do you know when it's going to be pushed?

[jasdel]

Hi @michaelf-stratoscale i pushed a small change to this PR that adds unit tests. I'll merge the change in once the Travis tests pass.