Răsfoiți Sursa

Remove panic recover

We now believe that it's safer to allow panics to terminate the process and
rely on panic monitoring to address the underlying issue.

In the case of bugs in the Go runtime, random memory corruption is possible.
In past cases, this lead to "impossible" panics, such as an index out of
bounds immediately after a bounds check. As the program state is no longer
sound, it's better to terminate rather than continue on and risk arbitrary
execution; e.g., security checks may no longer be correctly enforced.

Another risk with panic recovery is failure to clean up resources. For
example, code below the recovery may have acquired a mutex lock and panicked
before releasing the lock.

In this recover case, the code below the recover used to be simpler, but now
makes calls back into TunnelServer and it's more difficult/risky to reason
about resource cleanup.
Rod Hynes 4 ani în urmă
părinte
comite
6c16db7381
1 a modificat fișierele cu 0 adăugiri și 16 ștergeri
  1. 0 16
      psiphon/server/api.go

+ 0 - 16
psiphon/server/api.go

@@ -26,7 +26,6 @@ import (
 	std_errors "errors"
 	"net"
 	"regexp"
-	"runtime/debug"
 	"strconv"
 	"strings"
 	"time"
@@ -104,21 +103,6 @@ func dispatchAPIRequestHandler(
 	name string,
 	params common.APIParameters) (response []byte, reterr error) {
 
-	// Recover from and log any unexpected panics caused by user input
-	// handling bugs. User inputs should be properly validated; this
-	// mechanism is only a last resort to prevent the process from
-	// terminating in the case of a bug.
-	defer func() {
-		if e := recover(); e != nil {
-			if intentionalPanic, ok := e.(IntentionalPanicError); ok {
-				panic(intentionalPanic)
-			} else {
-				log.LogPanicRecover(e, debug.Stack())
-				reterr = errors.TraceNew("request handler panic")
-			}
-		}
-	}()
-
 	// Before invoking the handlers, enforce some preconditions:
 	//
 	// - A handshake request must precede any other requests.