Skip to content

Update redirect() and eliminate HttpRedirectView - #6862

Merged
labkey-adam merged 1 commit into
developfrom
fb_redirect
Jul 22, 2025
Merged

Update redirect() and eliminate HttpRedirectView#6862
labkey-adam merged 1 commit into
developfrom
fb_redirect

Conversation

@labkey-adam

@labkey-adamlabkey-adam commented Jul 19, 2025

Copy link
Copy Markdown
Contributor

Rationale

Eliminate HttpView.redirect(URLHelper) method in favor of other variants that take an ActionURL or a URLHelper plus a boolean indicating whether redirecting to an external URL is allowed. This tightens up security and reduces surprises.

Also eliminate HttpRedirectView in favor of throwing RedirectException.

Tasks 📍

  • Manual Testing / Verify Fix - not needed?
  • Needs Automation - rely on existing automation?

@labkey-jeckelslabkey-jeckels left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I didn't manually test this. I haven't looked at the callers to inform if/how to test. Any recommendations?

}

if (!isPrint() && !(reportView instanceof HttpRedirectView) && DiscussionService.get() != null)
if (!isPrint() && DiscussionService.get() != null)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This makes me even happier to eliminate HttpRedirectView.

@labkey-adam

Copy link
Copy Markdown
ContributorAuthor

I didn't manually test this. I haven't looked at the callers to inform if/how to test. Any recommendations?

No recommendations... I'm hoping automated tests would have shown issues. The redirect() changes are effectively no-ops: they are local redirects both before and after. And throwing vs. return a redirect view should be equivalent.

@labkey-adam
labkey-adam merged commit 5bcbd84 into developJul 22, 2025
11 checks passed
@labkey-adam
labkey-adam deleted the fb_redirect branch July 22, 2025 22:14
@labkey-adamlabkey-adam self-assigned this Jul 23, 2025
@labkey-adamlabkey-adam mentioned this pull request Jul 24, 2025
1 task
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@labkey-adam@labkey-jeckels