Skip to content

Fix overwriting path params by query params

Piotr Skorupa requested to merge psk-fix-overwriting-query-params into master

What does this MR do and why?

Related to https://gitlab.com/gitlab-org/gitlab/-/issues/342078.

This fixes an issue caught by Sentry on gitlab-com staging after rolling out mask_page_urls: event 1, event 2.

Query params were passed to url_for helper merged together with the path params as keyword arguments to the method, due to the helper forwarding unrecognized params as query params. This caused some URLs to not be pseudonymized properly due to query params overwriting path params with the same name, for example action and controller.

The helper would then fail to build the path for this malformed route.

Instead of passing query params directly, they are now passed in a nested params keyword as a Hash, as Rails docs actually recommend: https://api.rubyonrails.org/v6.1.4/classes/ActionDispatch/Routing/UrlFor.html#method-i-url_for

Screenshots or screen recordings

These are strongly recommended to assist reviewers and reduce the time to merge your change.

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Merge request reports