-
Notifications
You must be signed in to change notification settings - Fork 812
优化下载SSL异常处理 #5168
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
优化下载SSL异常处理 #5168
Conversation
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.
Pull request overview
This PR enhances SSL exception handling for downloads, specifically addressing DNS pollution issues. The changes fix a critical operator precedence bug that could cause NullPointerExceptions and add specific error messaging for DNS-related SSL failures.
Changes:
- Fixed operator precedence bug in SSL exception message checks across three Java files
- Added new localized error message
exception.dns.pollutionin 9 language files - Implemented DNS pollution detection in download error handling
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| HMCL/src/main/resources/assets/lang/I18N_zh_CN.properties | Added Chinese (Simplified) error message for DNS pollution SSL errors |
| HMCL/src/main/resources/assets/lang/I18N_zh.properties | Added Chinese (Traditional) error message for DNS pollution SSL errors |
| HMCL/src/main/resources/assets/lang/I18N_uk.properties | Added Ukrainian error message for DNS pollution SSL errors |
| HMCL/src/main/resources/assets/lang/I18N_ru.properties | Added Russian error message for DNS pollution SSL errors |
| HMCL/src/main/resources/assets/lang/I18N_lzh.properties | Added Classical Chinese error message for DNS pollution SSL errors |
| HMCL/src/main/resources/assets/lang/I18N_ja.properties | Added Japanese error message for DNS pollution SSL errors |
| HMCL/src/main/resources/assets/lang/I18N_es.properties | Added Spanish error message for DNS pollution SSL errors |
| HMCL/src/main/resources/assets/lang/I18N_ar.properties | Added Arabic error message for DNS pollution SSL errors |
| HMCL/src/main/resources/assets/lang/I18N.properties | Added English error message for DNS pollution SSL errors |
| HMCL/src/main/java/org/jackhuang/hmcl/ui/account/AddAuthlibInjectorServerPane.java | Fixed operator precedence bug in SSL exception message null check |
| HMCL/src/main/java/org/jackhuang/hmcl/setting/DownloadProviders.java | Added DNS pollution detection and fixed exception handling logic |
| HMCL/src/main/java/org/jackhuang/hmcl/setting/Accounts.java | Fixed operator precedence bug in SSL exception message null check |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } else if (exception.getCause() instanceof SSLHandshakeException && !(exception.getCause().getMessage() != null && exception.getCause().getMessage().contains("Remote host terminated"))) { | ||
| if (exception.getCause().getMessage() != null && (exception.getCause().getMessage().contains("No name matching") || exception.getCause().getMessage().contains("No subject alternative DNS name matching"))) { | ||
| return i18n("install.failed.downloading.detail", uri) + "\n" + i18n("exception.dns.pollution"); | ||
| } | ||
| return i18n("install.failed.downloading.detail", uri) + "\n" + i18n("exception.ssl_handshake"); | ||
| } else { | ||
| return i18n("install.failed.downloading.detail", uri) + "\n" + StringUtils.getStackTrace(exception.getCause()); |
Copilot
AI
Jan 11, 2026
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.
The condition on this line is overly complex and difficult to read. The nested null check combined with the negation makes the logic hard to follow. Consider extracting the "Remote host terminated" check into a separate boolean variable or method for better readability and maintainability.
| } else if (exception.getCause() instanceof SSLHandshakeException && !(exception.getCause().getMessage() != null && exception.getCause().getMessage().contains("Remote host terminated"))) { | |
| if (exception.getCause().getMessage() != null && (exception.getCause().getMessage().contains("No name matching") || exception.getCause().getMessage().contains("No subject alternative DNS name matching"))) { | |
| return i18n("install.failed.downloading.detail", uri) + "\n" + i18n("exception.dns.pollution"); | |
| } | |
| return i18n("install.failed.downloading.detail", uri) + "\n" + i18n("exception.ssl_handshake"); | |
| } else { | |
| return i18n("install.failed.downloading.detail", uri) + "\n" + StringUtils.getStackTrace(exception.getCause()); | |
| } else { | |
| Throwable cause = exception.getCause(); | |
| String causeMessage = cause == null ? null : cause.getMessage(); | |
| boolean isRemoteHostTerminated = causeMessage != null && causeMessage.contains("Remote host terminated"); | |
| if (cause instanceof SSLHandshakeException && !isRemoteHostTerminated) { | |
| if (causeMessage != null && (causeMessage.contains("No name matching") || causeMessage.contains("No subject alternative DNS name matching"))) { | |
| return i18n("install.failed.downloading.detail", uri) + "\n" + i18n("exception.dns.pollution"); | |
| } | |
| return i18n("install.failed.downloading.detail", uri) + "\n" + i18n("exception.ssl_handshake"); | |
| } else { | |
| return i18n("install.failed.downloading.detail", uri) + "\n" + StringUtils.getStackTrace(exception.getCause()); | |
| } |
| } else if (exception.getCause() instanceof SSLHandshakeException && !(exception.getCause().getMessage() != null && exception.getCause().getMessage().contains("Remote host terminated"))) { | ||
| if (exception.getCause().getMessage() != null && (exception.getCause().getMessage().contains("No name matching") || exception.getCause().getMessage().contains("No subject alternative DNS name matching"))) { |
Copilot
AI
Jan 11, 2026
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.
The method call exception.getCause().getMessage() is repeated multiple times on lines 137-138. Consider caching this value in a local variable to improve readability and avoid redundant method calls.
| } else if (exception.getCause() instanceof SSLHandshakeException && !(exception.getCause().getMessage() != null && exception.getCause().getMessage().contains("Remote host terminated"))) { | |
| if (exception.getCause().getMessage() != null && (exception.getCause().getMessage().contains("No name matching") || exception.getCause().getMessage().contains("No subject alternative DNS name matching"))) { | |
| } else if (exception.getCause() instanceof SSLHandshakeException) { | |
| String causeMessage = exception.getCause().getMessage(); | |
| if (causeMessage != null && causeMessage.contains("Remote host terminated")) { | |
| return i18n("install.failed.downloading.detail", uri) + "\n" + StringUtils.getStackTrace(exception.getCause()); | |
| } | |
| if (causeMessage != null && (causeMessage.contains("No name matching") || causeMessage.contains("No subject alternative DNS name matching"))) { |
fix #5115
针对DNS污染SSL问题进行处理
(修了上一个pr的null check)