Skip to content

Commit 52b719d

Browse files
Copilotbbockelm
andcommitted
Address remaining code review issues
Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>
1 parent 354b4e7 commit 52b719d

File tree

3 files changed

+10
-13
lines changed

3 files changed

+10
-13
lines changed

src/scitokens_internal.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,6 @@ class MonitoringStats {
134134
public:
135135
static MonitoringStats &instance();
136136

137-
void record_validation_start(const std::string &issuer);
138137
void record_validation_success(const std::string &issuer,
139138
uint64_t duration_ns);
140139
void record_validation_failure(const std::string &issuer,
@@ -156,7 +155,7 @@ class MonitoringStats {
156155

157156
mutable std::mutex m_mutex;
158157
std::unordered_map<std::string, IssuerStats> m_issuer_stats;
159-
std::unordered_map<std::string, std::atomic<uint64_t>> m_failed_issuer_lookups;
158+
std::unordered_map<std::string, uint64_t> m_failed_issuer_lookups;
160159

161160
std::string sanitize_issuer_for_json(const std::string &issuer) const;
162161
void prune_failed_issuers();
@@ -472,7 +471,7 @@ class Validator {
472471
}
473472

474473
void verify(const SciToken &scitoken, time_t expiry_time) {
475-
std::string issuer;
474+
std::string issuer = "";
476475
auto start_time = std::chrono::steady_clock::now();
477476
bool has_issuer = false;
478477

@@ -514,7 +513,7 @@ class Validator {
514513
}
515514

516515
void verify(const jwt::decoded_jwt<jwt::traits::kazuho_picojson> &jwt) {
517-
std::string issuer;
516+
std::string issuer = "";
518517
auto start_time = std::chrono::steady_clock::now();
519518
bool has_issuer = false;
520519

src/scitokens_monitoring.cpp

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,6 @@ MonitoringStats &MonitoringStats::instance() {
1616
return instance;
1717
}
1818

19-
void MonitoringStats::record_validation_start(const std::string &issuer) {
20-
// This is a no-op for now, but could be used for tracking in-progress
21-
// validations
22-
}
23-
2419
void MonitoringStats::record_validation_success(const std::string &issuer,
2520
uint64_t duration_ns) {
2621
std::lock_guard<std::mutex> lock(m_mutex);
@@ -80,7 +75,7 @@ void MonitoringStats::prune_failed_issuers() {
8075
// Find the minimum count
8176
uint64_t min_count = UINT64_MAX;
8277
for (const auto &entry : m_failed_issuer_lookups) {
83-
uint64_t count = entry.second.load();
78+
uint64_t count = entry.second;
8479
if (count < min_count) {
8580
min_count = count;
8681
}
@@ -89,7 +84,7 @@ void MonitoringStats::prune_failed_issuers() {
8984
// Remove all entries with the minimum count
9085
for (auto it = m_failed_issuer_lookups.begin();
9186
it != m_failed_issuer_lookups.end();) {
92-
if (it->second.load() == min_count) {
87+
if (it->second == min_count) {
9388
it = m_failed_issuer_lookups.erase(it);
9489
} else {
9590
++it;
@@ -142,7 +137,7 @@ std::string MonitoringStats::get_json() const {
142137
std::string sanitized_issuer =
143138
sanitize_issuer_for_json(entry.first);
144139
failed_obj[sanitized_issuer] =
145-
picojson::value(static_cast<double>(entry.second.load()));
140+
picojson::value(static_cast<double>(entry.second));
146141
}
147142
root["failed_issuer_lookups"] = picojson::value(failed_obj);
148143
}

src/test_monitoring_comprehensive.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ void print_monitoring_stats(const std::string &label) {
2525
int main() {
2626
char *err_msg = nullptr;
2727

28+
// Test constants
29+
const int DDOS_TEST_COUNT = 150; // Test beyond MAX_FAILED_ISSUERS limit
30+
2831
// Reset monitoring stats at start
2932
scitoken_reset_monitoring_stats(&err_msg);
3033

@@ -71,7 +74,7 @@ int main() {
7174

7275
// Try to create many tokens with different invalid issuers
7376
// The monitoring system should limit tracking to MAX_FAILED_ISSUERS (100)
74-
for (int i = 0; i < 150; i++) {
77+
for (int i = 0; i < DDOS_TEST_COUNT; i++) {
7578
// These are malformed tokens that will fail early
7679
std::string fake_token = "invalid.token." + std::to_string(i);
7780
SciToken temp_token = nullptr;

0 commit comments

Comments
 (0)