diff --git a/peer/log.go b/peer/log.go index 6c7233b7..49cada81 100644 --- a/peer/log.go +++ b/peer/log.go @@ -7,7 +7,6 @@ package peer import ( "fmt" - "strings" "time" "github.com/decred/dcrd/chaincfg/chainhash" @@ -16,12 +15,6 @@ import ( "github.com/decred/slog" ) -const ( - // maxRejectReasonLen is the maximum length of a sanitized reject reason - // that will be logged. - maxRejectReasonLen = 250 -) - // log is a logger that is initialized with no output filters. This // means the package will not perform any logging by default until the caller // requests it. @@ -112,30 +105,6 @@ func locatorSummary(locator []*chainhash.Hash, stopHash *chainhash.Hash) string return fmt.Sprintf("no locator, stop %s", stopHash) } -// sanitizeString strips any characters which are even remotely dangerous, such -// as html control characters, from the passed string. It also limits it to -// the passed maximum size, which can be 0 for unlimited. When the string is -// limited, it will also add "..." to the string to indicate it was truncated. -func sanitizeString(str string, maxLength uint) string { - const safeChars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXY" + - "Z01234567890 .,;_/:?@" - - // Strip any characters not in the safeChars string removed. - str = strings.Map(func(r rune) rune { - if strings.ContainsRune(safeChars, r) { - return r - } - return -1 - }, str) - - // Limit the string to the max allowed length. - if maxLength > 0 && uint(len(str)) > maxLength { - str = str[:maxLength] - str = str + "..." - } - return str -} - // messageSummary returns a human-readable string which summarizes a message. // Not all messages have or need a summary. This is used for debug logging. func messageSummary(msg wire.Message) string { @@ -196,20 +165,6 @@ func messageSummary(msg wire.Message) string { } return summary - case *wire.MsgReject: - // Ensure the variable length strings don't contain any - // characters which are even remotely dangerous such as HTML - // control characters, etc. Also limit them to sane length for - // logging. - rejCommand := sanitizeString(msg.Cmd, wire.CommandSize) - rejReason := sanitizeString(msg.Reason, maxRejectReasonLen) - summary := fmt.Sprintf("cmd %v, code %v, reason %v", rejCommand, - msg.Code, rejReason) - if rejCommand == wire.CmdBlock || rejCommand == wire.CmdTx { - summary += fmt.Sprintf(", hash %v", msg.Hash) - } - return summary - case *wire.MsgGetInitState: return fmt.Sprintf("types %v", msg.Types) diff --git a/peer/peer.go b/peer/peer.go index 5caa65be..8cf08c48 100644 --- a/peer/peer.go +++ b/peer/peer.go @@ -178,14 +178,18 @@ type MessageListeners struct { OnFeeFilter func(p *Peer, msg *wire.MsgFeeFilter) // OnVersion is invoked when a peer receives a version wire message. - // The caller may return a reject message in which case the message will - // be sent to the peer and the peer will be disconnected. + // + // Deprecated: The return value is no longer used beyond logging it and + // causing the peer to be disconnected when it is a non-nil value. OnVersion func(p *Peer, msg *wire.MsgVersion) *wire.MsgReject // OnVerAck is invoked when a peer receives a verack wire message. OnVerAck func(p *Peer, msg *wire.MsgVerAck) // OnReject is invoked when a peer receives a reject wire message. + // + // Deprecated: This will be removed in a future release. Callers should + // avoid using it. OnReject func(p *Peer, msg *wire.MsgReject) // OnSendHeaders is invoked when a peer receives a sendheaders wire @@ -1294,22 +1298,10 @@ out: // needed. rmsg, buf, err := p.readMessage() if err != nil { - // Only log the error and send reject message if the - // local peer is not forcibly disconnecting and the - // remote peer has not disconnected. + // Only log the error if the local peer is not forcibly + // disconnecting and the remote peer has not disconnected. if p.shouldHandleReadError(err) { - errMsg := fmt.Sprintf("Can't read message from %s: %v", p, err) - log.Errorf(errMsg) - - // Push a reject message for the malformed message and wait for - // the message to be sent before disconnecting. - // - // NOTE: Ideally this would include the command in the header if - // at least that much of the message was valid, but that is not - // currently exposed by wire, so just used malformed for the - // command. - p.PushRejectMsg("malformed", wire.RejectMalformed, errMsg, nil, - true) + log.Errorf("Can't read message from %s: %v", p, err) } var nErr net.Error @@ -1328,16 +1320,15 @@ out: switch msg := rmsg.(type) { case *wire.MsgVersion: // Limit to one version message per peer. - p.PushRejectMsg(msg.Command(), wire.RejectDuplicate, - "duplicate version message", nil, true) + log.Debugf("Already received 'version' from peer %s -- "+ + "disconnecting", p) break out case *wire.MsgVerAck: - // No read lock is necessary because verAckReceived is not written // to in any other goroutine. if p.verAckReceived { - log.Infof("Already received 'verack' from peer %v -- "+ + log.Infof("Already received 'verack' from peer %s -- "+ "disconnecting", p) break out } @@ -1878,27 +1869,18 @@ func (p *Peer) readRemoteVersionMsg() error { p.flagsMtx.Unlock() // Invoke the callback if specified. In the case the callback returns a - // reject message, notify and disconnect the peer accordingly. + // reject message, disconnect the peer accordingly. if p.cfg.Listeners.OnVersion != nil { rejectMsg := p.cfg.Listeners.OnVersion(p, msg) if rejectMsg != nil { - _ = p.writeMessage(rejectMsg) return errors.New(rejectMsg.Reason) } } - // Notify and disconnect clients that have a protocol version that is - // too old. + // Disconnect clients that have a protocol version that is too old. if msg.ProtocolVersion < int32(wire.InitialProcotolVersion) { - // Send a reject message indicating the protocol version is - // obsolete and wait for the message to be sent before - // disconnecting. - reason := fmt.Sprintf("protocol version must be %d or greater", + return fmt.Errorf("protocol version must be %d or greater", wire.InitialProcotolVersion) - rejectMsg := wire.NewMsgReject(msg.Command(), wire.RejectObsolete, - reason) - _ = p.writeMessage(rejectMsg) - return errors.New(reason) } return nil