PR comments; return success on EOF for BIO_read_ex

Author: samuel40791765

crypto/bio/bio.c

@@ -198,7 +198,7 @@ int BIO_read_ex(BIO *bio, void *data, size_t data_len, size_t *read_bytes) {
   }
 
   int ret = BIO_read(bio, data, read_len);
-  if (ret > 0) {
+  if (ret >= 0) {
     *read_bytes = ret;
     return 1;
   } else {

crypto/bio/bio_test.cc

@@ -1068,18 +1068,23 @@ TEST(BIOTest, ReadWriteEx) {
   bssl::UniquePtr<BIO> bio(BIO_new(BIO_s_mem()));
   ASSERT_TRUE(bio);
 
-  size_t written = 0;
+  // Reading from an initially empty bio should default to returning a error.
+  // Check that both |BIO_read| and |BIO_read_ex| fail.
+  char buf[32];
+  size_t read = 1;
+  EXPECT_EQ(BIO_read(bio.get(), buf, sizeof(buf)), -1);
+  EXPECT_FALSE(BIO_read_ex(bio.get(), buf, sizeof(buf), &read));
+  EXPECT_EQ(read, (size_t)0);
+
+  // Write and read normally from buffer.
+  size_t written = 1;
   ASSERT_TRUE(BIO_write_ex(bio.get(), "abcdef", 6, &written));
   EXPECT_EQ(written, (size_t)6);
-
-  char buf[32];
-  size_t read = 0;
   ASSERT_TRUE(BIO_read_ex(bio.get(), buf, sizeof(buf), &read));
   EXPECT_EQ(read, (size_t)6);
   EXPECT_EQ(Bytes(buf, read), Bytes("abcdef"));
 
   // Test NULL |written_bytes| behavior works.
-  read = 0;
   ASSERT_TRUE(BIO_write_ex(bio.get(), "ghilmnop", 8, nullptr));
   ASSERT_TRUE(BIO_read_ex(bio.get(), buf, sizeof(buf), &read));
   EXPECT_EQ(read, (size_t)8);
@@ -1090,13 +1095,12 @@ TEST(BIOTest, ReadWriteEx) {
   ASSERT_FALSE(BIO_read_ex(bio.get(), buf, sizeof(buf), nullptr));
 
   // Test that |BIO_write/read_ex| align with their non-ex counterparts, when
-  // encountering NULL data.
-  written = 1;
+  // encountering NULL data. EOF in |BIO_read| is indicated by returning 0.
+  // In AWS-LC's |BIO_read_ex|, this should return success and set |read| to 0.
   EXPECT_FALSE(BIO_write(bio.get(), nullptr, 0));
   EXPECT_FALSE(BIO_write_ex(bio.get(), nullptr, 0, &written));
   EXPECT_EQ(written, (size_t)0);
-  read = 1;
-  EXPECT_FALSE(BIO_read(bio.get(), nullptr, 0));
-  EXPECT_FALSE(BIO_read_ex(bio.get(), nullptr, 0, &read));
+  EXPECT_EQ(BIO_read(bio.get(), nullptr, 0), 0);
+  ASSERT_TRUE(BIO_read_ex(bio.get(), nullptr, 0, &read));
   EXPECT_EQ(read, (size_t)0);
 }

include/openssl/bio.h

@@ -112,6 +112,12 @@ OPENSSL_EXPORT int BIO_read(BIO *bio, void *data, int len);
 
 // BIO_read_ex calls |BIO_read| and stores the number of bytes read in
 // |read_bytes|. It returns one on success and zero otherwise.
+//
+// Note: OpenSSL's |BIO_read_ex| returns zero on EOF. This disallows any
+// mechanism to notify the user that an EOF has occurred and renders this API
+// unusable. See openssl/openssl#8208.
+// |BIO_read_ex| will return one for success and set |read_bytes| to 0 on EOF in
+// AWS-LC.
 OPENSSL_EXPORT int BIO_read_ex(BIO *bio, void *data, size_t data_len,
                                size_t *read_bytes);