Skip to content
This repository has been archived by the owner on Sep 11, 2020. It is now read-only.

packfile: Avoid panics patching corrupted deltas. #492

Merged
merged 1 commit into from
Jul 19, 2017

Conversation

ajnavarro
Copy link
Contributor

No description provided.

@@ -40,15 +45,18 @@ func ApplyDelta(target, base plumbing.EncodedObject, delta []byte) error {
return nil
}

var ErrDeltaLen = errors.New("wrong delta length")
Copy link
Contributor

Choose a reason for hiding this comment

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

can you group the errors?

Copy link
Collaborator

Choose a reason for hiding this comment

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

ErrInvalidDelta?

@@ -40,15 +45,18 @@ func ApplyDelta(target, base plumbing.EncodedObject, delta []byte) error {
return nil
}

var ErrDeltaLen = errors.New("wrong delta length")
var ErrDeltaCmd = errors.New("wrong delta command")

// PatchDelta returns the result of applying the modification deltas in delta to src.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain why an error can be returned?

@@ -59,6 +67,12 @@ func PatchDelta(src, delta []byte) []byte {
for {
cmd = delta[0]
delta = delta[1:]

if len(delta) < 2 {
// wrong delta
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need for the comment? It's returning an error explaining that the delta is not valid, so there is no need to comment that.

@@ -76,17 +90,23 @@ func PatchDelta(src, delta []byte) []byte {
}
dest = append(dest, delta[0:sz]...)
remainingTargetSz -= sz

if len(delta) < int(sz) {
// wrong delta
Copy link
Collaborator

Choose a reason for hiding this comment

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

superfluous comment.

@ajnavarro
Copy link
Contributor Author

@smola @mcuadros I address all your changes and cover more possible index out of bounds exceptions. Could you have a look?

@mcuadros mcuadros merged commit 595dfe6 into src-d:master Jul 19, 2017
@ajnavarro ajnavarro deleted the fix/panic-in-invalid-delta branch July 20, 2017 06:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants