Skip to content

Fix 500 error when handling unsafe redirects#6735

Merged
mitchellhenke merged 1 commit intomainfrom
mitchellhenke/fix-unsafe-redirect-500-part-2
Aug 15, 2022
Merged

Fix 500 error when handling unsafe redirects#6735
mitchellhenke merged 1 commit intomainfrom
mitchellhenke/fix-unsafe-redirect-500-part-2

Conversation

@mitchellhenke
Copy link
Contributor

@mitchellhenke mitchellhenke commented Aug 12, 2022

New Relic

Similar strategy as in #6682, and since this controller does not inherit from ApplicationController, we can make similar changes.

@mitchellhenke mitchellhenke requested review from a team, jmhooper and zachmargolis August 12, 2022 17:19
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM! bonus points for a spec

@mitchellhenke
Copy link
Contributor Author

LGTM! bonus points for a spec

I can add one for this route, but we do have the ApplicationController behavior covered here

@zachmargolis
Copy link
Contributor

LGTM! bonus points for a spec

I can add one for this route, but we do have the ApplicationController behavior covered here

If it's easy, yes! It makes for a good regression case if we ever change/remove that behavior at the base class

@mitchellhenke mitchellhenke force-pushed the mitchellhenke/fix-unsafe-redirect-500-part-2 branch from 55596cd to df3c623 Compare August 12, 2022 19:10
changelog: Bug Fixes, Security, Fix 500 error when handling unsafe redirects
@mitchellhenke mitchellhenke force-pushed the mitchellhenke/fix-unsafe-redirect-500-part-2 branch from df3c623 to eb7cb7b Compare August 15, 2022 14:18
@mitchellhenke mitchellhenke merged commit e776de7 into main Aug 15, 2022
@mitchellhenke mitchellhenke deleted the mitchellhenke/fix-unsafe-redirect-500-part-2 branch August 15, 2022 14:47
mitchellhenke pushed a commit that referenced this pull request Aug 15, 2022
changelog: Bug Fixes, Security, Fix 500 error when handling unsafe redirects
Sign up for free to 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.

3 participants