-
Notifications
You must be signed in to change notification settings - Fork 447
调整 Token 过期判断条件,依据一些实践,expires=0时,token已经过期。 #276
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThe update adjusts the logic in the Changes
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
WebApiClientCore.Extensions.OAuths/TokenResult.cs (2)
76-79
: Consider using UTC time for more reliable token expiration checksWhile the current implementation works correctly, consider using
DateTime.UtcNow
instead ofDateTime.Now
for both the token creation time (line 15) and the expiration check. This would make the code more robust in scenarios involving different time zones or server time changes.- private readonly DateTime createTime = DateTime.Now; + private readonly DateTime createTime = DateTime.UtcNow; public virtual bool IsExpired() { - return DateTime.Now.Subtract(this.createTime) >= TimeSpan.FromSeconds(this.Expires_in); + return DateTime.UtcNow.Subtract(this.createTime) >= TimeSpan.FromSeconds(this.Expires_in); }
76-79
: Add validation for negative or zero expiration valuesConsider adding validation for the
Expires_in
property. A zero or negative value might lead to tokens that are always expired, which could be intentional (as per the PR description forexpires=0
) but might also be due to errors for negative values.public virtual bool IsExpired() { + // Handle explicitly the case where Expires_in is 0 or negative + if (this.Expires_in <= 0) + { + return true; + } return DateTime.Now.Subtract(this.createTime) >= TimeSpan.FromSeconds(this.Expires_in); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
WebApiClientCore.Extensions.OAuths/TokenResult.cs
(1 hunks)
🔇 Additional comments (1)
WebApiClientCore.Extensions.OAuths/TokenResult.cs (1)
78-78
: Approval for modified expiration check logicChanging from
>
to>=
is a good adjustment. This ensures that tokens are considered expired exactly at their expiration time rather than only after it has passed, which is the correct behavior according to the PR objective. This is especially important whenexpires=0
, as the token should be treated as already expired.
Summary by CodeRabbit