update main-with-bazel from master branch

4 files changed, 138 insertions, 28 deletions

diff --git a/src/crypto/pool/internal.h b/src/crypto/pool/internal.h
index ed91de6..b39ee42 100644
--- a/src/crypto/pool/internal.h
+++ b/src/crypto/pool/internal.h
@@ -18,18 +18,22 @@
 #include <openssl/lhash.h>
 #include <openssl/thread.h>
 
+#include "../lhash/internal.h"
+
+
 #if defined(__cplusplus)
 extern "C" {
 #endif
 
 
-DECLARE_LHASH_OF(CRYPTO_BUFFER)
+DEFINE_LHASH_OF(CRYPTO_BUFFER)
 
 struct crypto_buffer_st {
   CRYPTO_BUFFER_POOL *pool;
   uint8_t *data;
   size_t len;
   CRYPTO_refcount_t references;
+  int data_is_static;
 };
 
 struct crypto_buffer_pool_st {
diff --git a/src/crypto/pool/pool.c b/src/crypto/pool/pool.c
index 88bf8af..89bf4c2 100644
--- a/src/crypto/pool/pool.c
+++ b/src/crypto/pool/pool.c
@@ -22,12 +22,9 @@
 #include <openssl/thread.h>
 
 #include "../internal.h"
-#include "../lhash/internal.h"
 #include "internal.h"
 
 
-DEFINE_LHASH_OF(CRYPTO_BUFFER)
-
 static uint32_t CRYPTO_BUFFER_hash(const CRYPTO_BUFFER *buf) {
   return OPENSSL_hash32(buf->data, buf->len);
 }
@@ -73,16 +70,28 @@ void CRYPTO_BUFFER_POOL_free(CRYPTO_BUFFER_POOL *pool) {
   OPENSSL_free(pool);
 }
 
-CRYPTO_BUFFER *CRYPTO_BUFFER_new(const uint8_t *data, size_t len,
-                                 CRYPTO_BUFFER_POOL *pool) {
+static void crypto_buffer_free_object(CRYPTO_BUFFER *buf) {
+  if (!buf->data_is_static) {
+    OPENSSL_free(buf->data);
+  }
+  OPENSSL_free(buf);
+}
+
+static CRYPTO_BUFFER *crypto_buffer_new(const uint8_t *data, size_t len,
+                                        int data_is_static,
+                                        CRYPTO_BUFFER_POOL *pool) {
   if (pool != NULL) {
     CRYPTO_BUFFER tmp;
     tmp.data = (uint8_t *) data;
     tmp.len = len;
 
     CRYPTO_MUTEX_lock_read(&pool->lock);
-    CRYPTO_BUFFER *const duplicate =
-        lh_CRYPTO_BUFFER_retrieve(pool->bufs, &tmp);
+    CRYPTO_BUFFER *duplicate = lh_CRYPTO_BUFFER_retrieve(pool->bufs, &tmp);
+    if (data_is_static && duplicate != NULL && !duplicate->data_is_static) {
+      // If the new |CRYPTO_BUFFER| would have static data, but the duplicate
+      // does not, we replace the old one with the new static version.
+      duplicate = NULL;
+    }
     if (duplicate != NULL) {
       CRYPTO_refcount_inc(&duplicate->references);
     }
@@ -99,10 +108,15 @@ CRYPTO_BUFFER *CRYPTO_BUFFER_new(const uint8_t *data, size_t len,
   }
   OPENSSL_memset(buf, 0, sizeof(CRYPTO_BUFFER));
 
-  buf->data = OPENSSL_memdup(data, len);
-  if (len != 0 && buf->data == NULL) {
-    OPENSSL_free(buf);
-    return NULL;
+  if (data_is_static) {
+    buf->data = (uint8_t *)data;
+    buf->data_is_static = 1;
+  } else {
+    buf->data = OPENSSL_memdup(data, len);
+    if (len != 0 && buf->data == NULL) {
+      OPENSSL_free(buf);
+      return NULL;
+    }
   }
 
   buf->len = len;
@@ -116,11 +130,18 @@ CRYPTO_BUFFER *CRYPTO_BUFFER_new(const uint8_t *data, size_t len,
 
   CRYPTO_MUTEX_lock_write(&pool->lock);
   CRYPTO_BUFFER *duplicate = lh_CRYPTO_BUFFER_retrieve(pool->bufs, buf);
+  if (data_is_static && duplicate != NULL && !duplicate->data_is_static) {
+    // If the new |CRYPTO_BUFFER| would have static data, but the duplicate does
+    // not, we replace the old one with the new static version.
+    duplicate = NULL;
+  }
   int inserted = 0;
   if (duplicate == NULL) {
     CRYPTO_BUFFER *old = NULL;
     inserted = lh_CRYPTO_BUFFER_insert(pool->bufs, &old, buf);
-    assert(old == NULL);
+    // |old| may be non-NULL if a match was found but ignored. |pool->bufs| does
+    // not increment refcounts, so there is no need to clean up after the
+    // replacement.
   } else {
     CRYPTO_refcount_inc(&duplicate->references);
   }
@@ -129,14 +150,18 @@ CRYPTO_BUFFER *CRYPTO_BUFFER_new(const uint8_t *data, size_t len,
   if (!inserted) {
     // We raced to insert |buf| into the pool and lost, or else there was an
     // error inserting.
-    OPENSSL_free(buf->data);
-    OPENSSL_free(buf);
+    crypto_buffer_free_object(buf);
     return duplicate;
   }
 
   return buf;
 }
 
+CRYPTO_BUFFER *CRYPTO_BUFFER_new(const uint8_t *data, size_t len,
+                                 CRYPTO_BUFFER_POOL *pool) {
+  return crypto_buffer_new(data, len, /*data_is_static=*/0, pool);
+}
+
 CRYPTO_BUFFER *CRYPTO_BUFFER_alloc(uint8_t **out_data, size_t len) {
   CRYPTO_BUFFER *const buf = OPENSSL_malloc(sizeof(CRYPTO_BUFFER));
   if (buf == NULL) {
@@ -156,10 +181,16 @@ CRYPTO_BUFFER *CRYPTO_BUFFER_alloc(uint8_t **out_data, size_t len) {
   return buf;
 }
 
-CRYPTO_BUFFER* CRYPTO_BUFFER_new_from_CBS(CBS *cbs, CRYPTO_BUFFER_POOL *pool) {
+CRYPTO_BUFFER *CRYPTO_BUFFER_new_from_CBS(const CBS *cbs,
+                                          CRYPTO_BUFFER_POOL *pool) {
   return CRYPTO_BUFFER_new(CBS_data(cbs), CBS_len(cbs), pool);
 }
 
+CRYPTO_BUFFER *CRYPTO_BUFFER_new_from_static_data_unsafe(
+    const uint8_t *data, size_t len, CRYPTO_BUFFER_POOL *pool) {
+  return crypto_buffer_new(data, len, /*data_is_static=*/1, pool);
+}
+
 void CRYPTO_BUFFER_free(CRYPTO_BUFFER *buf) {
   if (buf == NULL) {
     return;
@@ -171,8 +202,7 @@ void CRYPTO_BUFFER_free(CRYPTO_BUFFER *buf) {
       // If a reference count of zero is observed, there cannot be a reference
       // from any pool to this buffer and thus we are able to free this
       // buffer.
-      OPENSSL_free(buf->data);
-      OPENSSL_free(buf);
+      crypto_buffer_free_object(buf);
     }
 
     return;
@@ -188,13 +218,19 @@ void CRYPTO_BUFFER_free(CRYPTO_BUFFER *buf) {
   // find this buffer and increment the reference count. Thus, if the count is
   // zero there are and can never be any more references and thus we can free
   // this buffer.
-  void *found = lh_CRYPTO_BUFFER_delete(pool->bufs, buf);
-  assert(found != NULL);
-  assert(found == buf);
-  (void)found;
+  //
+  // Note it is possible |buf| is no longer in the pool, if it was replaced by a
+  // static version. If that static version was since removed, it is even
+  // possible for |found| to be NULL.
+  CRYPTO_BUFFER *found = lh_CRYPTO_BUFFER_retrieve(pool->bufs, buf);
+  if (found == buf) {
+    found = lh_CRYPTO_BUFFER_delete(pool->bufs, buf);
+    assert(found == buf);
+    (void)found;
+  }
+
   CRYPTO_MUTEX_unlock_write(&buf->pool->lock);
-  OPENSSL_free(buf->data);
-  OPENSSL_free(buf);
+  crypto_buffer_free_object(buf);
 }
 
 int CRYPTO_BUFFER_up_ref(CRYPTO_BUFFER *buf) {
diff --git a/src/crypto/pool/pool_test.cc b/src/crypto/pool/pool_test.cc
index 8f32fb6..a50f50f 100644
--- a/src/crypto/pool/pool_test.cc
+++ b/src/crypto/pool/pool_test.cc
@@ -16,6 +16,7 @@
 
 #include <openssl/pool.h>
 
+#include "internal.h"
 #include "../test/test_util.h"
 
 #if defined(OPENSSL_THREADS)
@@ -35,6 +36,15 @@ TEST(PoolTest, Unpooled) {
 
   // Test that reference-counting works properly.
   bssl::UniquePtr<CRYPTO_BUFFER> buf2 = bssl::UpRef(buf);
+
+  bssl::UniquePtr<CRYPTO_BUFFER> buf_static(
+    CRYPTO_BUFFER_new_from_static_data_unsafe(kData, sizeof(kData), nullptr));
+  ASSERT_TRUE(buf_static);
+  EXPECT_EQ(kData, CRYPTO_BUFFER_data(buf_static.get()));
+  EXPECT_EQ(sizeof(kData), CRYPTO_BUFFER_len(buf_static.get()));
+
+  // Test that reference-counting works properly.
+  bssl::UniquePtr<CRYPTO_BUFFER> buf_static2 = bssl::UpRef(buf_static);
 }
 
 TEST(PoolTest, Empty) {
@@ -43,22 +53,76 @@ TEST(PoolTest, Empty) {
 
   EXPECT_EQ(Bytes(""),
             Bytes(CRYPTO_BUFFER_data(buf.get()), CRYPTO_BUFFER_len(buf.get())));
+
+  bssl::UniquePtr<CRYPTO_BUFFER> buf_static(
+      CRYPTO_BUFFER_new_from_static_data_unsafe(nullptr, 0, nullptr));
+  ASSERT_TRUE(buf_static);
+
+  EXPECT_EQ(nullptr, CRYPTO_BUFFER_data(buf_static.get()));
+  EXPECT_EQ(0u, CRYPTO_BUFFER_len(buf_static.get()));
 }
 
 TEST(PoolTest, Pooled) {
   bssl::UniquePtr<CRYPTO_BUFFER_POOL> pool(CRYPTO_BUFFER_POOL_new());
   ASSERT_TRUE(pool);
 
-  static const uint8_t kData[4] = {1, 2, 3, 4};
+  static const uint8_t kData1[4] = {1, 2, 3, 4};
   bssl::UniquePtr<CRYPTO_BUFFER> buf(
-      CRYPTO_BUFFER_new(kData, sizeof(kData), pool.get()));
+      CRYPTO_BUFFER_new(kData1, sizeof(kData1), pool.get()));
   ASSERT_TRUE(buf);
+  EXPECT_EQ(Bytes(kData1),
+            Bytes(CRYPTO_BUFFER_data(buf.get()), CRYPTO_BUFFER_len(buf.get())));
 
   bssl::UniquePtr<CRYPTO_BUFFER> buf2(
-      CRYPTO_BUFFER_new(kData, sizeof(kData), pool.get()));
+      CRYPTO_BUFFER_new(kData1, sizeof(kData1), pool.get()));
   ASSERT_TRUE(buf2);
+  EXPECT_EQ(Bytes(kData1), Bytes(CRYPTO_BUFFER_data(buf2.get()),
+                                 CRYPTO_BUFFER_len(buf2.get())));
 
   EXPECT_EQ(buf.get(), buf2.get()) << "CRYPTO_BUFFER_POOL did not dedup data.";
+
+  // Different inputs do not get deduped.
+  static const uint8_t kData2[4] = {5, 6, 7, 8};
+  bssl::UniquePtr<CRYPTO_BUFFER> buf3(
+      CRYPTO_BUFFER_new(kData2, sizeof(kData2), pool.get()));
+  ASSERT_TRUE(buf3);
+  EXPECT_EQ(Bytes(kData2), Bytes(CRYPTO_BUFFER_data(buf3.get()),
+                                 CRYPTO_BUFFER_len(buf3.get())));
+  EXPECT_NE(buf.get(), buf3.get());
+
+  // When the last refcount on |buf3| is dropped, it is removed from the pool.
+  buf3 = nullptr;
+  EXPECT_EQ(1u, lh_CRYPTO_BUFFER_num_items(pool->bufs));
+
+  // Static buffers participate in pooling.
+  buf3.reset(CRYPTO_BUFFER_new_from_static_data_unsafe(kData2, sizeof(kData2),
+                                                       pool.get()));
+  ASSERT_TRUE(buf3);
+  EXPECT_EQ(kData2, CRYPTO_BUFFER_data(buf3.get()));
+  EXPECT_EQ(sizeof(kData2), CRYPTO_BUFFER_len(buf3.get()));
+  EXPECT_NE(buf.get(), buf3.get());
+
+  bssl::UniquePtr<CRYPTO_BUFFER> buf4(
+      CRYPTO_BUFFER_new(kData2, sizeof(kData2), pool.get()));
+  EXPECT_EQ(buf4.get(), buf3.get());
+
+  bssl::UniquePtr<CRYPTO_BUFFER> buf5(CRYPTO_BUFFER_new_from_static_data_unsafe(
+      kData2, sizeof(kData2), pool.get()));
+  EXPECT_EQ(buf5.get(), buf3.get());
+
+  // When creating a static buffer, if there is already a non-static buffer, it
+  // replaces the old buffer.
+  bssl::UniquePtr<CRYPTO_BUFFER> buf6(CRYPTO_BUFFER_new_from_static_data_unsafe(
+      kData1, sizeof(kData1), pool.get()));
+  ASSERT_TRUE(buf6);
+  EXPECT_EQ(kData1, CRYPTO_BUFFER_data(buf6.get()));
+  EXPECT_EQ(sizeof(kData1), CRYPTO_BUFFER_len(buf6.get()));
+  EXPECT_NE(buf.get(), buf6.get());
+
+  // Subsequent lookups of |kData1| should return |buf6|.
+  bssl::UniquePtr<CRYPTO_BUFFER> buf7(
+      CRYPTO_BUFFER_new(kData1, sizeof(kData1), pool.get()));
+  EXPECT_EQ(buf7.get(), buf6.get());
 }
 
 #if defined(OPENSSL_THREADS)
diff --git a/src/include/openssl/pool.h b/src/include/openssl/pool.h
index 0e4bdd5..c61a4ba 100644
--- a/src/include/openssl/pool.h
+++ b/src/include/openssl/pool.h
@@ -60,7 +60,13 @@ OPENSSL_EXPORT CRYPTO_BUFFER *CRYPTO_BUFFER_alloc(uint8_t **out_data,
 
 // CRYPTO_BUFFER_new_from_CBS acts the same as |CRYPTO_BUFFER_new|.
 OPENSSL_EXPORT CRYPTO_BUFFER *CRYPTO_BUFFER_new_from_CBS(
-    CBS *cbs, CRYPTO_BUFFER_POOL *pool);
+    const CBS *cbs, CRYPTO_BUFFER_POOL *pool);
+
+// CRYPTO_BUFFER_new_from_static_data_unsafe behaves like |CRYPTO_BUFFER_new|
+// but does not copy |data|. |data| must be immutable and last for the lifetime
+// of the address space.
+OPENSSL_EXPORT CRYPTO_BUFFER *CRYPTO_BUFFER_new_from_static_data_unsafe(
+    const uint8_t *data, size_t len, CRYPTO_BUFFER_POOL *pool);
 
 // CRYPTO_BUFFER_free decrements the reference count of |buf|. If there are no
 // other references, or if the only remaining reference is from a pool, then