Handle unaligned uploadStart#1031
Conversation
465e197 to
fccee32
Compare
| } | ||
| // ...and then write it out. | ||
|
|
||
| // Now write the bundle out out. |
There was a problem hiding this comment.
I guess you only want one out.
There was a problem hiding this comment.
Weeeell, we do really really want to be writing it out. Like, out out. :)
| } | ||
|
|
||
| // bundleIterator returns an interator which yields MirrorPackages after verifying their subtree consistency with the provided pending checkpoint. | ||
| // bundleIterator returns an interator which yields entry bundles after verifying their subtree consistency with the provided pending checkpoint. |
There was a problem hiding this comment.
| // bundleIterator returns an interator which yields entry bundles after verifying their subtree consistency with the provided pending checkpoint. | |
| // bundleIterator returns an iterator which yields entry bundles after verifying their subtree consistency with the provided pending checkpoint. |
| yield(nil, fmt.Errorf("failed to unmarshal bundle containing uploadStart (%d): %v", start, err)) | ||
| return | ||
| } | ||
| padEntries = b.Entries[:p] |
There was a problem hiding this comment.
Can the client side trigger the mirror server runtime panic (i.e. slice bounds out of range) by providing entries length less than p?
There was a problem hiding this comment.
I don't think so, not unless there's been something gone quite wrong before - we read a partial bundle which matches the offset of uploadStart into that bundle. If there aren't enough entries to trim to p from that bundle, then the log is probably corrupt. OTOH, with garbage collection, we could (at least theoretically) end up being returned a full tile with more than p entries so need to trim it down.
I've added a defensive check to avoid the panic in this situation, though, as unlikely as it is, it would mean that one corrupt mirror tree would potentially make all the others served by the same process unavailable too.
There was a problem hiding this comment.
Gotcha, the partial bundle comes from the mirror's log storage. It's all good if the client couldn't trigger panic in the mirror server.
7a5d461 to
196df01
Compare
| yield(nil, fmt.Errorf("failed to unmarshal bundle containing uploadStart (%d): %v", start, err)) | ||
| return | ||
| } | ||
| padEntries = b.Entries[:p] |
There was a problem hiding this comment.
Gotcha, the partial bundle comes from the mirror's log storage. It's all good if the client couldn't trigger panic in the mirror server.
2 participants
This PR adds support for mirror uploads which don't start on an entrybundle boundary.
Towards #945