diff --git a/crypto/bio/bio_test.cc b/crypto/bio/bio_test.cc index 12daa755b7..e69105e728 100644 --- a/crypto/bio/bio_test.cc +++ b/crypto/bio/bio_test.cc @@ -867,14 +867,19 @@ TEST(BIOTest, FileMode) { ASSERT_TRUE(bio); ASSERT_TRUE(BIO_read_filename(bio.get(), temp.path().c_str())); expect_binary_mode(bio.get()); - // |BIO_new_file| should use the specified mode. + // |BIO_new_file| does not set |BIO_FP_TEXT|, so expect it to set the file's + // mode to binary in all cases. This odd behavior, but it's what OpenSSL does. bio.reset(BIO_new_file(temp.path().c_str(), "rb")); ASSERT_TRUE(bio); expect_binary_mode(bio.get()); bio.reset(BIO_new_file(temp.path().c_str(), "r")); ASSERT_TRUE(bio); + // NOTE: Our behavior here aligns with OpenSSL. BoringSSL would + // |expect_text_mode| below because they don't call |_setmode| unless + // |BIO_FP_TEXT| is set. expect_text_mode(bio.get()); - // |BIO_new_fp| inherits the file's existing mode by default. + // |BIO_new_fp| will always set |_O_BINARY| if |BIO_FP_TEXT| is not set in the + // call to |BIO_new_fp|. ScopedFILE file = temp.Open("rb"); ASSERT_TRUE(file); bio.reset(BIO_new_fp(file.get(), BIO_NOCLOSE)); @@ -884,7 +889,10 @@ TEST(BIOTest, FileMode) { ASSERT_TRUE(file); bio.reset(BIO_new_fp(file.get(), BIO_NOCLOSE)); ASSERT_TRUE(bio); - expect_text_mode(bio.get()); + // NOTE: Our behavior here aligns with OpenSSL. BoringSSL would + // |expect_text_mode| below because they don't call |_setmode| unless + // |BIO_FP_TEXT| is set. + expect_binary_mode(bio.get()); // However, |BIO_FP_TEXT| changes the file to be text mode, no matter how it // was opened. file = temp.Open("rb"); diff --git a/crypto/bio/file.c b/crypto/bio/file.c index 91bdf1b81f..9a520f2a5e 100644 --- a/crypto/bio/file.c +++ b/crypto/bio/file.c @@ -206,9 +206,13 @@ static long file_ctrl(BIO *b, int cmd, long num, void *ptr) { b->init = 1; #if defined(OPENSSL_WINDOWS) // Windows differentiates between "text" and "binary" file modes, so set - // the file to text mode if caller specifies BIO_FP_TEXT flag. + // the file to text mode if caller specifies BIO_FP_TEXT flag. If + // |BIO_FP_TEXT| is not set, we still call |_setmode| with |_O_BINARY| to + // match OpenSSL's behavior. BoringSSL, on the other hand, does nothing in + // this case. // // https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/setmode?view=msvc-170#remarks + // https://github.com/google/boringssl/commit/5ee4e9512e9a99f97c4a3fad397034028b3457c2 _setmode(_fileno(b->ptr), num & BIO_FP_TEXT ? _O_TEXT : _O_BINARY); #endif break; @@ -299,8 +303,8 @@ int BIO_get_fp(BIO *bio, FILE **out_file) { return (int)BIO_ctrl(bio, BIO_C_GET_FILE_PTR, 0, (char *)out_file); } -int BIO_set_fp(BIO *bio, FILE *file, int close_flag) { - return (int)BIO_ctrl(bio, BIO_C_SET_FILE_PTR, close_flag, (char *)file); +int BIO_set_fp(BIO *bio, FILE *file, int flags) { + return (int)BIO_ctrl(bio, BIO_C_SET_FILE_PTR, flags, (char *)file); } int BIO_read_filename(BIO *bio, const char *filename) {