diff --git a/quic/api/QuicTransportBase.cpp b/quic/api/QuicTransportBase.cpp index 387b05ba4..6bcc00e67 100644 --- a/quic/api/QuicTransportBase.cpp +++ b/quic/api/QuicTransportBase.cpp @@ -213,6 +213,8 @@ void QuicTransportBase::closeGracefully() { } } +// TODO: t64691045 change the closeImpl API to include both the sanitized and +// unsanited error message, remove exceptionCloseWhat_. void QuicTransportBase::closeImpl( folly::Optional> errorCode, bool drainConnection, @@ -279,6 +281,13 @@ void QuicTransportBase::closeImpl( } else if (errorCode) { cancelCode = *errorCode; } + // cancelCode is used for communicating error message to local app layer. + // errorCode will be used for localConnectionError, and sent in close frames. + // It's safe to include the unsanitized error message in cancelCode + if (exceptionCloseWhat_) { + cancelCode.second = exceptionCloseWhat_.value(); + } + bool isReset = false; bool isAbandon = false; LocalErrorCode* localError = cancelCode.first.asLocalErrorCode(); @@ -390,11 +399,8 @@ void QuicTransportBase::closeImpl( if (noError) { connCallback_->onConnectionEnd(); } else { - std::string closeStr = exceptionCloseWhat_ - ? std::move(exceptionCloseWhat_.value()) - : cancelCode.second.str(); connCallback_->onConnectionError( - std::make_pair(cancelCode.first, closeStr)); + std::make_pair(cancelCode.first, cancelCode.second.str())); } } diff --git a/quic/api/test/QuicTransportBaseTest.cpp b/quic/api/test/QuicTransportBaseTest.cpp index c74c3fd75..314f60b81 100644 --- a/quic/api/test/QuicTransportBaseTest.cpp +++ b/quic/api/test/QuicTransportBaseTest.cpp @@ -1030,6 +1030,36 @@ TEST_F(QuicTransportImplTest, ConnectionErrorOnWrite) { QuicErrorCode(LocalErrorCode::CONNECTION_ABANDONED)); } +TEST_F(QuicTransportImplTest, ReadErrorUnsanitizedErrorMsg) { + transport->setServerConnectionId(); + transport->transportConn->oneRttWriteCipher = test::createNoOpAead(); + auto stream = transport->createBidirectionalStream().value(); + MockReadCallback rcb; + transport->setReadCallback(stream, &rcb); + EXPECT_CALL(rcb, readError(stream, _)) + .Times(1) + .WillOnce(Invoke( + [](StreamId, + std::pair> + error) { + EXPECT_EQ("You need to calm down.", *error.second); + })); + + EXPECT_CALL(*socketPtr, write(_, _)).WillOnce(Invoke([](auto&, auto&) { + throw std::runtime_error("You need to calm down."); + return 0; + })); + QuicSocket::WriteResult result = transport->writeChain( + stream, + folly::IOBuf::copyBuffer("You are being too loud."), + true, + false, + nullptr); + evb->loopOnce(); + + EXPECT_TRUE(transport->isClosed()); +} + TEST_F(QuicTransportImplTest, ConnectionErrorUnhandledException) { transport->transportConn->oneRttWriteCipher = test::createNoOpAead(); auto stream = transport->createBidirectionalStream().value();