Skip to content

Comments

Fix #227: 修复可用性监控 15 分钟时间范围的 Invalid Date 错误#231

Merged
ding113 merged 2 commits intodevfrom
fix/issue-227-availability-invalid-date
Nov 28, 2025
Merged

Fix #227: 修复可用性监控 15 分钟时间范围的 Invalid Date 错误#231
ding113 merged 2 commits intodevfrom
fix/issue-227-availability-invalid-date

Conversation

@ding113
Copy link
Owner

@ding113 ding113 commented Nov 28, 2025

Close #227

Summary

  • 修复选择"最近 15 分钟"时可用性监控页面渲染失败的问题
  • 错误原因:bucketSizeMinutes 参数被 parseInt() 错误解析,导致除零错误

Root Cause

前端在选择 15 分钟时间范围时,计算出的 bucketSizeMinutes = 0.25(15 秒),但后端 API 使用 parseInt("0.25", 10) 解析,得到 0

availability-service.ts 中:

const bucketSizeMs = bucketSizeMinutes * 60 * 1000; // = 0
const bucketStart = new Date(Math.floor(req.createdAt.getTime() / bucketSizeMs) * bucketSizeMs);
// 除以 0 得到 Infinity,Math.floor(Infinity) * 0 = NaN
// new Date(NaN).toISOString() 抛出 RangeError: Invalid time value

Changes

  1. src/app/api/availability/route.ts: 使用 parseFloat() 代替 parseInt() 解析 bucketSizeMinutes,并增加最小值保护
  2. src/lib/availability/availability-service.ts: 增加最小值保护(0.25 分钟 = 15 秒),作为防御性编程的双重保险

🤖 Generated with Claude Code

根本原因:前端传递小数形式的 bucketSizeMinutes(如 0.25 表示 15 秒),
但后端使用 parseInt() 解析,导致 0.25 被转换为 0,
在除法运算中产生 Infinity,最终导致 Invalid Date 错误。

修复:
- API 路由:使用 parseFloat 代替 parseInt 解析 bucketSizeMinutes
- 可用性服务:增加最小值保护(0.25 分钟 = 15 秒)防止除零错误

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @ding113, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

此拉取请求旨在解决可用性监控页面在特定时间范围选择下出现的渲染故障。通过修正 bucketSizeMinutes 参数的解析逻辑并引入防御性的最小值保护,确保了系统能够正确处理时间桶大小,从而避免了因除零操作导致的 Invalid Date 错误,提升了监控功能的稳定性和用户体验。

Highlights

  • 修复可用性监控页面渲染问题: 解决了当选择“最近 15 分钟”时间范围时,可用性监控页面因 Invalid Date 错误而无法正确渲染的问题。
  • 参数解析修正: 将 bucketSizeMinutes 参数的解析方式从 parseInt() 更改为 parseFloat(),以正确处理小数分钟值(例如 0.25 分钟)。
  • 增加最小值保护: 在 API 路由和核心服务逻辑中都增加了 bucketSizeMinutes 的最小值保护,确保其不小于 0.25 分钟(15 秒),从而避免除零错误。
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@ding113 ding113 added the size/XS Extra Small PR (< 50 lines) label Nov 28, 2025
@ding113
Copy link
Owner Author

ding113 commented Nov 28, 2025

🔒 Security Scan Results

No security vulnerabilities detected

This PR has been scanned against OWASP Top 10, CWE Top 25, and common security anti-patterns. No security issues were identified in the code changes.

Changes Analyzed

File Change Security Impact
src/app/api/availability/route.ts parseInt()parseFloat() with Math.max(0.25, parsed) ✅ Neutral - No injection risk (numeric parsing for arithmetic)
src/lib/availability/availability-service.ts Added Math.max(0.25, rawBucketSize) protection ✅ Positive - Prevents division-by-zero edge cases

Scanned Categories

  • ✅ Injection attacks (SQL, NoSQL, Command, LDAP, etc.)
  • ✅ Authentication and session management
  • ✅ Sensitive data exposure
  • ✅ Access control and authorization
  • ✅ Security misconfiguration
  • ✅ Cross-site scripting (XSS)
  • ✅ Insecure deserialization
  • ✅ SSRF and path traversal
  • ✅ Cryptographic weaknesses
  • ✅ DoS/Resource exhaustion

OWASP Top 10 Coverage

  • A01: Injection - Clean (numeric parsing, no string interpolation in queries)
  • A02: Broken Authentication - N/A (no auth changes)
  • A03: Sensitive Data Exposure - N/A (no sensitive data handling)
  • A04: XML External Entities - N/A
  • A05: Broken Access Control - N/A (existing admin check unchanged)
  • A06: Security Misconfiguration - Clean
  • A07: XSS - N/A (API endpoint, no HTML rendering)
  • A08: Insecure Deserialization - N/A
  • A09: Known Vulnerabilities - N/A (no new dependencies)
  • A10: Logging & Monitoring - N/A

🛡️ Security Posture

Acceptable - This PR is a bug fix that adds input validation protection. The Math.max(0.25, ...) pattern prevents potential edge cases with very small bucket sizes that could cause performance issues.


🤖 Automated security scan by Claude AI - OWASP Top 10 & CWE coverage

Copy link
Owner Author

@ding113 ding113 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📋 Code Review Summary

This PR fixes a division-by-zero bug in the availability monitoring feature by using parseFloat() instead of parseInt() and adding minimum value protection. The fix addresses the root cause correctly. However, there is one issue that should be addressed.

🔍 Issues Found

  • Critical (🔴): 0 issues
  • High (🟠): 1 issue
  • Medium (🟡): 0 issues
  • Low (🟢): 0 issues

🎯 Priority Actions

  1. 🟠 High - src/app/api/availability/route.ts:55: parseFloat() returns NaN for invalid input (e.g., bucketSizeMinutes=abc), which bypasses the minimum value protection since Math.max(0.25, NaN) returns NaN. This would still cause the original division-by-zero bug with malformed input.

    Suggested fix:

    // Use parseFloat to support sub-minute bucket sizes (e.g., 0.25 for 15 seconds)
    const parsed = parseFloat(bucketSizeMinutes);
    // Ensure bucket size is valid and at least 0.25 minutes (15 seconds) to prevent division by zero
    options.bucketSizeMinutes = Number.isNaN(parsed) ? 0.25 : Math.max(0.25, parsed);

💡 General Observations

The defensive programming approach of adding the minimum value check in both the API route and the service layer is sound. The service layer's protection at availability-service.ts:186 would catch the NaN case if the route is fixed, but it's better to handle invalid input at the API boundary. Consider adding the Number.isNaN() check to both locations for complete defense-in-depth.


🤖 Automated review by Claude AI - focused on identifying issues for improvement

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

本次 PR 修复了可用性监控页面在选择15分钟时间范围时因 parseInt 错误解析小数而导致的 “Invalid Date” 错误。修改的核心是将 parseInt 替换为 parseFloat,并增加了最小值保护,确保 bucketSizeMinutes 不会为零。整体来看,这是一个正确的修复。我补充了一个建议,用于处理当 bucketSizeMinutes 参数为无效字符串(如 'abc')时 parseFloat 返回 NaN 的边缘情况,以增强代码的健壮性。

Comment on lines 55 to 57
const parsed = parseFloat(bucketSizeMinutes);
// Ensure bucket size is at least 0.25 minutes (15 seconds) to prevent division by zero
options.bucketSizeMinutes = Math.max(0.25, parsed);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

这里使用 parseFloat 转换查询参数是正确的,但没有处理 parseFloat 可能返回 NaN 的情况(例如,当 bucketSizeMinutes 是一个无效的数字字符串如 'abc' 时)。如果 parsedNaNMath.max(0.25, NaN) 的结果也是 NaN。这会导致 options.bucketSizeMinutes 被设置为 NaN,并最终在 availability-service.ts 中引发同样的 Invalid time value 错误。

建议在赋值前增加一个 isNaN 检查,以确保只有在解析出有效数字时才设置 bucketSizeMinutes

      const parsed = parseFloat(bucketSizeMinutes);
      if (!isNaN(parsed)) {
        // Ensure bucket size is at least 0.25 minutes (15 seconds) to prevent division by zero
        options.bucketSizeMinutes = Math.max(0.25, parsed);
      }

- Add Number.isNaN check in route.ts to handle invalid parseFloat input
- Add Number.isNaN check in availability-service.ts as defense-in-depth
- Fixes edge case where Math.max(0.25, NaN) returns NaN, causing Invalid Date error
@ding113 ding113 merged commit 356fef9 into dev Nov 28, 2025
7 checks passed
@ding113 ding113 deleted the fix/issue-227-availability-invalid-date branch November 28, 2025 16:48
@ding113
Copy link
Owner Author

ding113 commented Nov 28, 2025

🔒 Security Scan Results

No security vulnerabilities detected

This PR has been scanned against OWASP Top 10, CWE Top 25, and common security anti-patterns. No security issues were identified in the code changes.

Scanned Categories

  • ✅ Injection attacks (SQL, NoSQL, Command, LDAP, etc.)
  • ✅ Authentication and session management
  • ✅ Sensitive data exposure
  • ✅ Access control and authorization
  • ✅ Security misconfiguration
  • ✅ Cross-site scripting (XSS)
  • ✅ Insecure deserialization
  • ✅ SSRF and path traversal
  • ✅ Cryptographic weaknesses
  • ✅ DoS / Resource exhaustion

Security Positive Findings

This PR actually improves the security posture by:

  1. Input Validation Enhancement - The addition of Number.isNaN() checks prevents potential issues from malformed input being passed through the system.

  2. DoS Prevention - The minimum bucket size of 0.25 minutes (15 seconds) prevents edge cases where division by zero could occur, which could potentially cause:

    • Infinity values in calculations
    • Invalid Date errors
    • Excessive memory/CPU usage from malformed data

Code Quality Notes

The changes are well-implemented:

  • Defense-in-depth approach with validation at both API route level and service level
  • Proper NaN handling (nullish coalescing ?? doesn't catch NaN)
  • Reasonable default fallback values

🤖 Automated security scan by Claude AI - OWASP Top 10 & CWE coverage

@ding113 ding113 added the bug Something isn't working label Nov 28, 2025
ding113 pushed a commit that referenced this pull request Nov 28, 2025
options.bucketSizeMinutes = parseInt(bucketSizeMinutes, 10);
// Use parseFloat to support sub-minute bucket sizes (e.g., 0.25 for 15 seconds)
const parsed = parseFloat(bucketSizeMinutes);
// Ensure bucket size is valid and at least 0.25 minutes (15 seconds) to prevent division by zero
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High: Missing validation for Infinity input causes the same "Invalid Date" error this PR aims to fix.

Why this is a problem: When a user provides bucketSizeMinutes=Infinity, parseFloat("Infinity") returns Infinity, which passes both the Number.isNaN() check and Math.max(0.25, Infinity). This results in bucketSizeMs = Infinity, and subsequently new Date(Math.floor(reqTime / Infinity) * Infinity) produces an Invalid Date, throwing the same RangeError this PR is intended to fix.

Suggested fix:

      // Use parseFloat to support sub-minute bucket sizes (e.g., 0.25 for 15 seconds)
      const parsed = parseFloat(bucketSizeMinutes);
      // Ensure bucket size is valid, finite, and at least 0.25 minutes (15 seconds) to prevent division by zero
      options.bucketSizeMinutes = Number.isFinite(parsed) ? Math.max(0.25, parsed) : 0.25;

Using Number.isFinite() instead of !Number.isNaN() will also reject Infinity and -Infinity, providing more robust validation.

// Handle NaN case (nullish coalescing doesn't catch NaN from invalid parseFloat input)
const rawBucketSize =
explicitBucketSize ?? determineOptimalBucketSize(requests.length, timeRangeMinutes);
const bucketSizeMinutes = Number.isNaN(rawBucketSize)
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High: Same Infinity vulnerability exists in the service layer.

Why this is a problem: If explicitBucketSize is Infinity (passed from the route), this code path will accept it since Number.isNaN(Infinity) is false and Math.max(0.25, Infinity) returns Infinity. This leads to the same Invalid Date error.

Suggested fix:

  const bucketSizeMinutes = !Number.isFinite(rawBucketSize)
    ? determineOptimalBucketSize(requests.length, timeRangeMinutes)
    : Math.max(0.25, rawBucketSize);

Using !Number.isFinite() covers NaN, Infinity, and -Infinity in a single check.

Copy link
Owner Author

@ding113 ding113 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📋 Code Review Summary

This PR correctly identifies and addresses the root cause of the Invalid Date error when using 15-minute time ranges in the availability monitor. However, the fix is incomplete as it doesn't handle Infinity input values, which can cause the same error.

🔍 Issues Found

  • Critical (🔴): 0 issues
  • High (🟠): 2 issues
  • Medium (🟡): 0 issues
  • Low (🟢): 0 issues

🎯 Priority Actions

  1. Replace Number.isNaN() with Number.isFinite() in both files - parseFloat("Infinity") returns Infinity, which passes the Number.isNaN() check but still causes Invalid Date errors when used in the bucket calculation.

💡 General Observations

The defensive programming approach with validation in both the API route and service layer is good practice. However, both layers need the same fix: use Number.isFinite() instead of !Number.isNaN() to catch both NaN and Infinity values in a single check.


🤖 Automated review by Claude AI - focused on identifying issues for improvement

@ding113 ding113 mentioned this pull request Nov 28, 2025
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working size/XS Extra Small PR (< 50 lines)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant