Conversation
There was a problem hiding this comment.
Pull request overview
Adds exception handling to URL query-parameter parsing so stale/removed portlet window ids don’t throw during request parsing, aiming to reduce noisy failures in logs.
Changes:
- Wrap
portletWindowRegistry.getPortletWindowId(...)inparsePortletParameterName()with atry/catchforIllegalArgumentException - Log and fall back to treating the parameter as “unscoped” when window id parsing fails
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
uPortal-rendering/src/main/java/org/apereo/portal/url/UrlSyntaxProviderImpl.java
Outdated
Show resolved
Hide resolved
…xProviderImpl.java Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| this.logger.debug( | ||
| "Failed to parse portlet window id '{}' from parameter '{}', treating as unscoped parameter", | ||
| additionalPortletId, | ||
| name); |
There was a problem hiding this comment.
The caught IllegalArgumentException is ignored entirely (not logged/passed through), which makes diagnosing unexpected parsing issues harder and leaves the local e unused. Consider either logging the exception as a debug cause (third argument) or renaming the variable to ignored if intentionally discarding it.
| name); | |
| name, | |
| e); |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| final IPortletWindowId portletWindowId = | ||
| this.portletWindowRegistry.getPortletWindowId(request, additionalPortletId); | ||
| return new Tuple<String, IPortletWindowId>(paramName, portletWindowId); | ||
| } catch (IllegalArgumentException e) { |
| name.substring( | ||
| PORTLET_PARAM_PREFIX.length() | ||
| + additionalPortletId.length() | ||
| + SEPARATOR.length()); | ||
| final IPortletWindowId portletWindowId = | ||
| this.portletWindowRegistry.getPortletWindowId(request, additionalPortletId); | ||
| return new Tuple<String, IPortletWindowId>(paramName, portletWindowId); | ||
| try { |
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Checklist
security.propertiesportal.propertiesCHANGES.mdDescription of change
Exception is not caught around removed links. This should clean up the logs.