test,crypto: adjust tests for BoringSSL

[codebytere]

Ongoing effort to make crypto tests work with BoringSSL.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.65%. Comparing base (da5843b) to head (d27e019).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #61459      +/-   ##
==========================================
- Coverage   89.67%   89.65%   -0.02%     
==========================================
  Files         676      676              
  Lines      206469   206469              
  Branches    39537    39542       +5     
==========================================
- Hits       185157   185117      -40     
- Misses      13448    13491      +43     
+ Partials     7864     7861       -3     

see 32 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[addaleax]

^ Do you know what's causing these? This is a difference in the public API that I would not expect.

This seems like the kind of thing where we should consistently return an uppercase string from the API, because it would be reasonable for user code to rely on that particular behavior. This should do the trick:

diff --git a/src/crypto/crypto_x509.cc b/src/crypto/crypto_x509.cc
index 11622d823bbc..f40f6dd4f2bc 100644
--- a/src/crypto/crypto_x509.cc
+++ b/src/crypto/crypto_x509.cc
@@ -253,8 +253,8 @@ MaybeLocal<Value> GetSignatureAlgorithmOID(Environment* env,
 
 MaybeLocal<Value> GetSerialNumber(Environment* env, const X509View& view) {
   if (auto serial = view.getSerialNumber()) {
-    return OneByteString(env->isolate(),
-                         static_cast<unsigned char*>(serial.get()));
+    return ToV8Value(
+        env, ToUpper(std::string_view(static_cast<char*>(serial.get()))));
   }
   return Undefined(env->isolate());
 }

If there's more to this and we really, really cannot do that for some reason, these regexps should be anchored, i.e.

Suggested change

	/147D36C1C2F74206DE9FAB5F2226D78ADB00A426/i);
	/^147D36C1C2F74206DE9FAB5F2226D78ADB00A426$/i);

[addaleax]

(and actually, maybe this is something we should handle at all sites where we call BN_bn2hex()...)

[codebytere]

Given that'd be a slightly larger undertaking and not just a test change should i break that into a new PR?

[addaleax]

If you want to go with the diff above, I think that would be a smaller change overall 🙂

I personally don't have a strong preference for a new PR or keeping this one, I'm happy to review either way

[addaleax]

@codebytere Fyi, I've opened a separate PR with that suggestion: #61752

[codebytere]

@addaleax @panva is there anything else I need to do here?