Fix BIO_eof for BIO pairs
See uptream's 3105d695358d86c0f2a404b2b74a1870b941ce5e. Fleshed out
BIO pair tests to cover all these cases.
Change-Id: I5fd7f9c08c50b1462459c19a34230212b81cb674
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/78829
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
diff --git a/crypto/bio/bio_test.cc b/crypto/bio/bio_test.cc
index faeccd8..1935b6d 100644
--- a/crypto/bio/bio_test.cc
+++ b/crypto/bio/bio_test.cc
@@ -791,97 +791,134 @@
class BIOPairTest : public testing::TestWithParam<bool> {};
TEST_P(BIOPairTest, TestPair) {
- BIO *bio1, *bio2;
- ASSERT_TRUE(BIO_new_bio_pair(&bio1, 10, &bio2, 10));
- bssl::UniquePtr<BIO> free_bio1(bio1), free_bio2(bio2);
+ BIO *bio1_raw, *bio2_raw;
+ ASSERT_TRUE(BIO_new_bio_pair(&bio1_raw, 10, &bio2_raw, 10));
+ bssl::UniquePtr<BIO> bio1(bio1_raw), bio2(bio2_raw);
if (GetParam()) {
std::swap(bio1, bio2);
}
// Check initial states.
- EXPECT_EQ(10u, BIO_ctrl_get_write_guarantee(bio1));
- EXPECT_EQ(0u, BIO_ctrl_get_read_request(bio1));
+ EXPECT_EQ(10u, BIO_ctrl_get_write_guarantee(bio1.get()));
+ EXPECT_EQ(0u, BIO_ctrl_get_read_request(bio1.get()));
+ EXPECT_FALSE(BIO_eof(bio1.get()));
+ EXPECT_EQ(0u, BIO_pending(bio1.get()));
+ EXPECT_EQ(0u, BIO_wpending(bio1.get()));
// Data written in one end may be read out the other.
uint8_t buf[20];
- EXPECT_EQ(5, BIO_write(bio1, "12345", 5));
- EXPECT_EQ(5u, BIO_ctrl_get_write_guarantee(bio1));
- ASSERT_EQ(5, BIO_read(bio2, buf, sizeof(buf)));
+ EXPECT_EQ(5, BIO_write(bio1.get(), "12345", 5));
+ EXPECT_EQ(5u, BIO_ctrl_get_write_guarantee(bio1.get()));
+ EXPECT_EQ(5u, BIO_pending(bio2.get()));
+ EXPECT_EQ(5u, BIO_wpending(bio1.get()));
+ ASSERT_EQ(5, BIO_read(bio2.get(), buf, sizeof(buf)));
EXPECT_EQ(Bytes("12345"), Bytes(buf, 5));
- EXPECT_EQ(10u, BIO_ctrl_get_write_guarantee(bio1));
+ EXPECT_EQ(10u, BIO_ctrl_get_write_guarantee(bio1.get()));
+ EXPECT_EQ(0u, BIO_pending(bio2.get()));
+ EXPECT_EQ(0u, BIO_wpending(bio1.get()));
// Attempting to write more than 10 bytes will write partially.
- EXPECT_EQ(10, BIO_write(bio1, "1234567890___", 13));
- EXPECT_EQ(0u, BIO_ctrl_get_write_guarantee(bio1));
- EXPECT_EQ(-1, BIO_write(bio1, "z", 1));
- EXPECT_TRUE(BIO_should_write(bio1));
- ASSERT_EQ(10, BIO_read(bio2, buf, sizeof(buf)));
+ EXPECT_EQ(10, BIO_write(bio1.get(), "1234567890___", 13));
+ EXPECT_EQ(0u, BIO_ctrl_get_write_guarantee(bio1.get()));
+ EXPECT_EQ(-1, BIO_write(bio1.get(), "z", 1));
+ EXPECT_TRUE(BIO_should_write(bio1.get()));
+ ASSERT_EQ(10, BIO_read(bio2.get(), buf, sizeof(buf)));
EXPECT_EQ(Bytes("1234567890"), Bytes(buf, 10));
- EXPECT_EQ(10u, BIO_ctrl_get_write_guarantee(bio1));
+ EXPECT_EQ(10u, BIO_ctrl_get_write_guarantee(bio1.get()));
// Unsuccessful reads update the read request.
- EXPECT_EQ(-1, BIO_read(bio2, buf, 5));
- EXPECT_TRUE(BIO_should_read(bio2));
- EXPECT_EQ(5u, BIO_ctrl_get_read_request(bio1));
+ EXPECT_EQ(-1, BIO_read(bio2.get(), buf, 5));
+ EXPECT_TRUE(BIO_should_read(bio2.get()));
+ EXPECT_EQ(5u, BIO_ctrl_get_read_request(bio1.get()));
// The read request is clamped to the size of the buffer.
- EXPECT_EQ(-1, BIO_read(bio2, buf, 20));
- EXPECT_TRUE(BIO_should_read(bio2));
- EXPECT_EQ(10u, BIO_ctrl_get_read_request(bio1));
+ EXPECT_EQ(-1, BIO_read(bio2.get(), buf, 20));
+ EXPECT_TRUE(BIO_should_read(bio2.get()));
+ EXPECT_EQ(10u, BIO_ctrl_get_read_request(bio1.get()));
// Data may be written and read in chunks.
- EXPECT_EQ(5, BIO_write(bio1, "12345", 5));
- EXPECT_EQ(5u, BIO_ctrl_get_write_guarantee(bio1));
- EXPECT_EQ(5, BIO_write(bio1, "67890___", 8));
- EXPECT_EQ(0u, BIO_ctrl_get_write_guarantee(bio1));
- ASSERT_EQ(3, BIO_read(bio2, buf, 3));
+ EXPECT_EQ(5, BIO_write(bio1.get(), "12345", 5));
+ EXPECT_EQ(5u, BIO_ctrl_get_write_guarantee(bio1.get()));
+ EXPECT_EQ(5, BIO_write(bio1.get(), "67890___", 8));
+ EXPECT_EQ(0u, BIO_ctrl_get_write_guarantee(bio1.get()));
+ ASSERT_EQ(3, BIO_read(bio2.get(), buf, 3));
EXPECT_EQ(Bytes("123"), Bytes(buf, 3));
- EXPECT_EQ(3u, BIO_ctrl_get_write_guarantee(bio1));
- ASSERT_EQ(7, BIO_read(bio2, buf, sizeof(buf)));
+ EXPECT_EQ(3u, BIO_ctrl_get_write_guarantee(bio1.get()));
+ ASSERT_EQ(7, BIO_read(bio2.get(), buf, sizeof(buf)));
EXPECT_EQ(Bytes("4567890"), Bytes(buf, 7));
- EXPECT_EQ(10u, BIO_ctrl_get_write_guarantee(bio1));
+ EXPECT_EQ(10u, BIO_ctrl_get_write_guarantee(bio1.get()));
// Successful reads reset the read request.
- EXPECT_EQ(0u, BIO_ctrl_get_read_request(bio1));
+ EXPECT_EQ(0u, BIO_ctrl_get_read_request(bio1.get()));
// Test writes and reads starting in the middle of the ring buffer and
// wrapping to front.
- EXPECT_EQ(8, BIO_write(bio1, "abcdefgh", 8));
- EXPECT_EQ(2u, BIO_ctrl_get_write_guarantee(bio1));
- ASSERT_EQ(3, BIO_read(bio2, buf, 3));
+ EXPECT_EQ(8, BIO_write(bio1.get(), "abcdefgh", 8));
+ EXPECT_EQ(2u, BIO_ctrl_get_write_guarantee(bio1.get()));
+ ASSERT_EQ(3, BIO_read(bio2.get(), buf, 3));
EXPECT_EQ(Bytes("abc"), Bytes(buf, 3));
- EXPECT_EQ(5u, BIO_ctrl_get_write_guarantee(bio1));
- EXPECT_EQ(5, BIO_write(bio1, "ijklm___", 8));
- EXPECT_EQ(0u, BIO_ctrl_get_write_guarantee(bio1));
- ASSERT_EQ(10, BIO_read(bio2, buf, sizeof(buf)));
+ EXPECT_EQ(5u, BIO_ctrl_get_write_guarantee(bio1.get()));
+ EXPECT_EQ(5, BIO_write(bio1.get(), "ijklm___", 8));
+ EXPECT_EQ(0u, BIO_ctrl_get_write_guarantee(bio1.get()));
+ ASSERT_EQ(10, BIO_read(bio2.get(), buf, sizeof(buf)));
EXPECT_EQ(Bytes("defghijklm"), Bytes(buf, 10));
- EXPECT_EQ(10u, BIO_ctrl_get_write_guarantee(bio1));
+ EXPECT_EQ(10u, BIO_ctrl_get_write_guarantee(bio1.get()));
// Data may flow from both ends in parallel.
- EXPECT_EQ(5, BIO_write(bio1, "12345", 5));
- EXPECT_EQ(5, BIO_write(bio2, "67890", 5));
- ASSERT_EQ(5, BIO_read(bio2, buf, sizeof(buf)));
+ EXPECT_EQ(5, BIO_write(bio1.get(), "12345", 5));
+ EXPECT_EQ(5, BIO_write(bio2.get(), "67890", 5));
+ ASSERT_EQ(5, BIO_read(bio2.get(), buf, sizeof(buf)));
EXPECT_EQ(Bytes("12345"), Bytes(buf, 5));
- ASSERT_EQ(5, BIO_read(bio1, buf, sizeof(buf)));
+ ASSERT_EQ(5, BIO_read(bio1.get(), buf, sizeof(buf)));
EXPECT_EQ(Bytes("67890"), Bytes(buf, 5));
// Closing the write end causes an EOF on the read half, after draining.
- EXPECT_EQ(5, BIO_write(bio1, "12345", 5));
- EXPECT_TRUE(BIO_shutdown_wr(bio1));
- ASSERT_EQ(5, BIO_read(bio2, buf, sizeof(buf)));
+ EXPECT_EQ(5, BIO_write(bio1.get(), "12345", 5));
+ EXPECT_TRUE(BIO_shutdown_wr(bio1.get()));
+ EXPECT_FALSE(BIO_eof(bio2.get()));
+ EXPECT_EQ(5u, BIO_pending(bio2.get()));
+ EXPECT_EQ(5u, BIO_wpending(bio1.get()));
+ ASSERT_EQ(5, BIO_read(bio2.get(), buf, sizeof(buf)));
EXPECT_EQ(Bytes("12345"), Bytes(buf, 5));
- EXPECT_EQ(0, BIO_read(bio2, buf, sizeof(buf)));
+ EXPECT_TRUE(BIO_eof(bio2.get()));
+ EXPECT_EQ(0u, BIO_pending(bio2.get()));
+ EXPECT_EQ(0u, BIO_wpending(bio1.get()));
+ EXPECT_EQ(0, BIO_read(bio2.get(), buf, sizeof(buf)));
+ EXPECT_TRUE(BIO_eof(bio2.get()));
+ EXPECT_EQ(0u, BIO_pending(bio2.get()));
+ EXPECT_EQ(0u, BIO_wpending(bio1.get()));
// A closed write end may not be written to.
- EXPECT_EQ(0u, BIO_ctrl_get_write_guarantee(bio1));
- EXPECT_EQ(-1, BIO_write(bio1, "_____", 5));
+ EXPECT_EQ(0u, BIO_ctrl_get_write_guarantee(bio1.get()));
+ EXPECT_EQ(-1, BIO_write(bio1.get(), "_____", 5));
EXPECT_TRUE(ErrorEquals(ERR_get_error(), ERR_LIB_BIO, BIO_R_BROKEN_PIPE));
// The other end is still functional.
- EXPECT_EQ(5, BIO_write(bio2, "12345", 5));
- ASSERT_EQ(5, BIO_read(bio1, buf, sizeof(buf)));
+ EXPECT_EQ(5, BIO_write(bio2.get(), "12345", 5));
+ ASSERT_EQ(5, BIO_read(bio1.get(), buf, sizeof(buf)));
EXPECT_EQ(Bytes("12345"), Bytes(buf, 5));
+ EXPECT_FALSE(BIO_eof(bio1.get()));
+
+ // Destroying |bio2| implicitly closes it, and discards unread data.
+ EXPECT_EQ(5, BIO_write(bio2.get(), "12345", 5));
+ bio2 = nullptr;
+
+ // |bio1| no longer has the "init" flag set, so reads and writes will fail at
+ // the BIO framework.
+ EXPECT_FALSE(BIO_get_init(bio1.get()));
+ EXPECT_EQ(-2, BIO_write(bio1.get(), "12345", 5));
+ EXPECT_TRUE(ErrorEquals(ERR_get_error(), ERR_LIB_BIO, BIO_R_UNINITIALIZED));
+ EXPECT_EQ(-2, BIO_read(bio1.get(), buf, sizeof(buf)));
+ EXPECT_TRUE(ErrorEquals(ERR_get_error(), ERR_LIB_BIO, BIO_R_UNINITIALIZED));
+
+ // Although in this disconnected state, |BIO_ctrl| works. |bio1| should
+ // report EOF when it has no peer.
+ EXPECT_TRUE(BIO_eof(bio1.get()));
+
+ // BIO_ctrl_get_write_guarantee should return 0 because there is no one to
+ // write to.
+ EXPECT_EQ(0u, BIO_ctrl_get_write_guarantee(bio1.get()));
}
INSTANTIATE_TEST_SUITE_P(All, BIOPairTest, testing::Values(false, true));
diff --git a/crypto/bio/pair.cc b/crypto/bio/pair.cc
index ac5a9c5..c290332 100644
--- a/crypto/bio/pair.cc
+++ b/crypto/bio/pair.cc
@@ -381,12 +381,10 @@
return 1;
case BIO_CTRL_EOF: {
- BIO *other_bio = reinterpret_cast<BIO *>(ptr);
- if (other_bio) {
- struct bio_bio_st *other_b =
- reinterpret_cast<bio_bio_st *>(other_bio->ptr);
- assert(other_b != nullptr);
- return other_b->len == 0 && other_b->closed;
+ if (b->peer) {
+ auto *peer_b = reinterpret_cast<bio_bio_st *>(b->peer->ptr);
+ assert(peer_b != nullptr);
+ return peer_b->len == 0 && peer_b->closed;
}
return 1;
}
