Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Macros to periodically warn #482

Merged
merged 2 commits into from
Mar 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 6 additions & 18 deletions src/workerd/api/crypto-impl-asymmetric.c++
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,8 @@ public:

JSG_REQUIRE(EVP_MD_size(type) + paddingOverhead <= RSA_size(&rsa), DOMOperationError,
"key too small for signing with given digest");
if (RSA_size(&rsa) < EVP_MD_size(type) + 32) {
static bool logOnce KJ_UNUSED = ([rsa] {
KJ_LOG(WARNING, "Signing with peculiar key size of ", RSA_size(&rsa), " bytes");
return true;
})();
}
JSG_WARN_ONCE_IF(RSA_size(&rsa) < EVP_MD_size(type) + 32,
"Signing with peculiar key size of ", RSA_size(&rsa), " bytes");

// JSG_REQUIRE(EVP_MD_size(type) + 32 <= RSA_size(&rsa), DOMOperationError,
// "key too small for signing with given digest");
Expand Down Expand Up @@ -733,19 +729,11 @@ void validateRsaParams(int modulusLength, kj::ArrayPtr<kj::byte> publicExponent,
// TODO(soon): We should also enforce these limitations on imported keys. To see if this breaks
// existing scripts only provide a warning in sentry for now.
if (warnImport) {
if (modulusLength % 8 || modulusLength < 256 || modulusLength > 16384) {
static bool logOnce KJ_UNUSED = ([modulusLength] {
KJ_LOG(WARNING, "Imported RSA key has invalid modulus length ", modulusLength, ".");
return true;
})();
}
JSG_WARN_ONCE_IF(modulusLength % 8 || modulusLength < 256 || modulusLength > 16384,
"Imported RSA key has invalid modulus length ", modulusLength, ".");
KJ_IF_MAYBE(v, fromBignum<unsigned>(publicExponent)) {
if (*v != 3 && *v != 65537) {
static bool logOnce KJ_UNUSED = ([v] {
KJ_LOG(WARNING, "Imported RSA key has invalid publicExponent ", *v,".");
return true;
})();
}
JSG_WARN_ONCE_IF(*v != 3 && *v != 65537,
"Imported RSA key has invalid publicExponent ", *v, ".");
} else {
JSG_FAIL_REQUIRE(DOMOperationError, "The \"publicExponent\" must be either 3 or 65537, but "
"got a number larger than 2^32.");
Expand Down
7 changes: 2 additions & 5 deletions src/workerd/io/worker.c++
Original file line number Diff line number Diff line change
Expand Up @@ -374,11 +374,8 @@ void reportStartupError(
// TODO(soon): We add logging here to see if this hack is still necessary or if it can be
// removed. Adding this additional logging should be temporary! If we hit this log in
// sentry even once, then we'll keep the hack, otherwise we can likely safely remove it.
static bool logOnce KJ_UNUSED = ([] {
KJ_LOG(WARNING, "reportStartupError() customer-specific SyntaxError hack "
"is still relevant.");
return true;
})();
JSG_WARN_ONCE("reportStartupError() customer-specific SyntaxError hack "
"is still relevant.");
} else {
KJ_LOG(ERROR, "script startup threw exception", id, description, trace);
KJ_FAIL_REQUIRE("script startup threw exception");
Expand Down
13 changes: 8 additions & 5 deletions src/workerd/jsg/jsg.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,17 @@
KJ_FAIL_REQUIRE(kj::str(JSG_EXCEPTION(jsErrorType) ": ", ##__VA_ARGS__))
// JSG_REQUIRE + KJ_FAIL_REQUIRE

#define JSG_WARN_ONCE(msg, ...) \
static bool logOnce KJ_UNUSED = ([&] { \
KJ_LOG(WARNING, msg, ##__VA_ARGS__); \
return true; \
})() \

// Conditionally log a warning, at most once. Useful for determining if code changes would break
// any existing scripts.
#define JSG_WARN_ONCE_IF(cond, msg) \
#define JSG_WARN_ONCE_IF(cond, msg, ...) \
if (cond) { \
static bool logOnce KJ_UNUSED = ([] { \
KJ_LOG(WARNING, msg); \
return true; \
})(); \
JSG_WARN_ONCE(msg, ##__VA_ARGS__); \
}

// These are passthrough functions to KJ. We expect the error string to be
Expand Down
41 changes: 40 additions & 1 deletion src/workerd/util/sentry.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <kj/common.h>
#include <kj/string.h>
#include <kj/exception.h>
#include <kj/time.h>
#include <stdint.h>

namespace workerd {
Expand Down Expand Up @@ -47,4 +48,42 @@ inline kj::StringPtr maybeOmitColoFromSentry(uint32_t coloId) {
KJ_LOG(severity, __VA_ARGS__); \
}

} // namespace workerd
// Log this to Sentry once ever per process. Typically will be better to use LOG_ERROR_PERIODICALLY.
#define LOG_ERROR_ONCE(msg, ...) \
do { \
static bool logOnce KJ_UNUSED = [&]() { \
KJ_LOG(ERROR, msg, ##__VA_ARGS__); \
return true; \
}(); \
} while (0)

#define LOG_ERROR_ONCE_IF(cond, msg, ...) \
do { \
if (cond) { \
vlovich marked this conversation as resolved.
Show resolved Hide resolved
LOG_ERROR_ONCE(msg, ##__VA_ARGS__); \
} \
} while (0)

// Slightly more expensive than LOG_ERROR_ONCE. Avoid putting into a hot path (e.g. within a loop)
// where an overhead of ~hundreds of nanoseconds per evaluation to retrieve the current time would
// be prohibitive.
#define LOG_ERROR_PERIODICALLY(msg, ...) \
do { \
static kj::TimePoint KJ_UNIQUE_NAME(lastLogged) = \
vlovich marked this conversation as resolved.
Show resolved Hide resolved
kj::origin<kj::TimePoint>(); \
const auto now = kj::systemCoarseMonotonicClock().now(); \
const auto elapsed = now - KJ_UNIQUE_NAME(lastLogged); \
if (KJ_UNLIKELY(elapsed >= 1 * kj::HOURS)) { \
KJ_UNIQUE_NAME(lastLogged) = now; \
KJ_LOG(ERROR, msg, ##__VA_ARGS__); \
} \
} while (0)

#define LOG_ERROR_PERIODICALLY_IF(cond, msg, ...) \
do { \
if (cond) { \
LOG_ERROR_PERIODICALLY(msg, ##__VA_ARGS__); \
} \
} while (0)

} // namespace workerd