Skip to content

Commit

Permalink
Adjust test to expect our and OpenSSL's behavior
Browse files Browse the repository at this point in the history
  • Loading branch information
WillChilds-Klein committed Dec 10, 2024
1 parent fc1cec3 commit 780fd7b
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 6 deletions.
14 changes: 11 additions & 3 deletions crypto/bio/bio_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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");
Expand Down
10 changes: 7 additions & 3 deletions crypto/bio/file.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 780fd7b

Please sign in to comment.