Repository: envoyproxy/envoy
File: source/common/tls/ssl_socket.cc (C++)

Code before:
    }

    absl::StrAppend(&failure_reason_, "|", err, ":",
                    absl::NullSafeStringView(ERR_lib_error_string(err)), ":",
                    absl::NullSafeStringView(ERR_func_error_string(err)), ":",
                    absl::NullSafeStringView(ERR_reason_error_string(err)));
  }

  if (!saw_error) {
    return;
  }

  // Append detailed error info for certificate-related failures.
  if (saw_cert_verify_failed || saw_no_client_cert) {
    auto* extended_socket_info = reinterpret_cast<Envoy::Ssl::SslExtendedSocketInfo*>(
        SSL_get_ex_data(rawSsl(), ContextImpl::sslExtendedSocketInfoIndex()));
    if (extended_socket_info != nullptr) {
      absl::string_view cert_validation_error = extended_socket_info->certificateValidationError();
      if (!cert_validation_error.empty()) {
        absl::StrAppend(&failure_reason_, ":", cert_validation_error);
      } else if (saw_no_client_cert) {
        // Provide a default message if no detailed error was captured
        absl::StrAppend(&failure_reason_, ":peer did not provide required client certificate");
      }
    } else if (saw_no_client_cert) {
      absl::StrAppend(&failure_reason_, ":peer did not provide required client certificate");
    }
  }

  if (!failure_reason_.empty()) {
    absl::StrAppend(&failure_reason_, ":TLS_error_end");
    ENVOY_CONN_LOG(debug, "remote address:{},{}", callbacks_->connection(),
                   callbacks_->connection().connectionInfoProvider().remoteAddress()->asString(),
                   failure_reason_);
  }

  if (!saw_counted_error) {
    ctx_->stats().connection_error_.inc();
  }
}

Network::IoResult SslSocket::doWrite(Buffer::Instance& write_buffer, bool end_stream) {
  ASSERT(info_->state() != Ssl::SocketState::ShutdownSent || write_buffer.length() == 0);
  if (info_->state() != Ssl::SocketState::HandshakeComplete &&
      info_->state() != Ssl::SocketState::ShutdownSent) {
    PostIoAction action = doHandshake();
    if (action == PostIoAction::Close || info_->state() != Ssl::SocketState::HandshakeComplete) {
      return {action, 0, false};
    }
  }


Review comment (bug):
Please refactor the logic so there's only one case that appends this error. Maybe something roughly like this:

```
bool added_reason = false;
if (saw_cert_verify_failed) {
  // checks for null
  StrAppend(failure_reason_, "the text");
  added_reason = true;
}
if (!added_reason && saw_no_client_cert) {
  StrAppend(......);
}
```

Code after:
    }

    absl::StrAppend(&failure_reason_, "|", err, ":",
                    absl::NullSafeStringView(ERR_lib_error_string(err)), ":",
                    absl::NullSafeStringView(ERR_func_error_string(err)), ":",
                    absl::NullSafeStringView(ERR_reason_error_string(err)));
  }

  if (!saw_error) {
    return;
  }

  // Append detailed error info for certificate-related failures.
  bool added_detail = false;
  if (saw_cert_verify_failed) {
    auto* extended_socket_info = reinterpret_cast<Envoy::Ssl::SslExtendedSocketInfo*>(
        SSL_get_ex_data(rawSsl(), ContextImpl::sslExtendedSocketInfoIndex()));
    if (extended_socket_info != nullptr) {
      absl::string_view cert_validation_error = extended_socket_info->certificateValidationError();
      if (!cert_validation_error.empty()) {
        absl::StrAppend(&failure_reason_, ":", cert_validation_error);
        added_detail = true;
      }
    }
  }
  if (!added_detail && saw_no_client_cert) {
    absl::StrAppend(&failure_reason_, ":peer did not provide required client certificate");
  }

  if (!failure_reason_.empty()) {
    absl::StrAppend(&failure_reason_, ":TLS_error_end");
    ENVOY_CONN_LOG(debug, "remote address:{},{}", callbacks_->connection(),
                   callbacks_->connection().connectionInfoProvider().remoteAddress()->asString(),
                   failure_reason_);
  }

  if (!saw_counted_error) {
    ctx_->stats().connection_error_.inc();
  }
}

Network::IoResult SslSocket::doWrite(Buffer::Instance& write_buffer, bool end_stream) {
  ASSERT(info_->state() != Ssl::SocketState::ShutdownSent || write_buffer.length() == 0);
  if (info_->state() != Ssl::SocketState::HandshakeComplete &&
      info_->state() != Ssl::SocketState::ShutdownSent) {
    PostIoAction action = doHandshake();
    if (action == PostIoAction::Close || info_->state() != Ssl::SocketState::HandshakeComplete) {
      return {action, 0, false};
    }
  }