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

feat: harden encoding/decoding functions against panics #243

Merged
merged 2 commits into from
Apr 18, 2022

Conversation

Stebalien
Copy link
Member

Part of libp2p/go-libp2p#1389

These kinds of functions:

  1. Handle user input.
  2. Often have out-of-bounds, null pointer, etc bugs.
  3. Have completely isolated logic where local panics are unlikely to cause memory corruption elsewhere.

Copy link
Contributor

@marten-seemann marten-seemann left a comment

Choose a reason for hiding this comment

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

As discussed:

  • let's keep this to crypto operations
  • add stderr output

@marten-seemann
Copy link
Contributor

@Stebalien Can you update this PR to log the stack trace to stderr?

"runtime/debug"
)

func CatchAndLog(err *error, where string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the idea, but I don't think recover can be moved to a separate function. I added a random panic to one of the functions and it didn't work.

Copy link
Member Author

Choose a reason for hiding this comment

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

This works:

package main

import (
	"fmt"
)

func myRecover() {
	if err := recover(); err != nil {
		fmt.Println("reccovered: ", err)
	}
}

func foo() {
	defer myRecover()

	panic("foo")
}

func main() {
	foo()
}

I'll see if I can figure out what I did wrong...

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added a test case.

Part of libp2p/go-libp2p#1389

These kinds of functions:

1. Handle user input.
2. Often have out-of-bounds, null pointer, etc bugs.
3. Have completely isolated logic where local panics are unlikely to
   cause memory corruption elsewhere.
@marten-seemann marten-seemann merged commit 648dc3f into master Apr 18, 2022
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.

2 participants