Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29806555: #12222 - Add redirects for ABP installation links (Closed)

Created:
June 13, 2018, 3:07 p.m. by f.lopez
Modified:
June 13, 2018, 7:17 p.m.
Reviewers:
mathias, Fred
Visibility:
Public.

Description

#12222 - Add redirects for ABP installation links

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -0 lines) Patch
M hiera/roles/web/redirect/eyeo.yaml View 1 chunk +5 lines, -0 lines 2 comments Download

Messages

Total messages: 3
f.lopez
June 13, 2018, 3:07 p.m. (2018-06-13 15:07:54 UTC) #1
mathias
https://codereview.adblockplus.org/29806555/diff/29806556/hiera/roles/web/redirect/eyeo.yaml File hiera/roles/web/redirect/eyeo.yaml (right): https://codereview.adblockplus.org/29806555/diff/29806556/hiera/roles/web/redirect/eyeo.yaml#newcode67 hiera/roles/web/redirect/eyeo.yaml:67: adblockplus/edge_install: "https://www.microsoft.com/store/p/adblock-plus/9nblggh4r9nz" Please add `?` signs to the end ...
June 13, 2018, 5:52 p.m. (2018-06-13 17:52:00 UTC) #2
f.lopez
June 13, 2018, 7:16 p.m. (2018-06-13 19:16:27 UTC) #3
https://codereview.adblockplus.org/29806555/diff/29806556/hiera/roles/web/red...
File hiera/roles/web/redirect/eyeo.yaml (right):

https://codereview.adblockplus.org/29806555/diff/29806556/hiera/roles/web/red...
hiera/roles/web/redirect/eyeo.yaml:67: adblockplus/edge_install:
"https://www.microsoft.com/store/p/adblock-plus/9nblggh4r9nz"
On 2018/06/13 17:52:00, mathias wrote:
> Please add `?` signs to the end of the target links, e.g. `foo:
> https://example.com/bar?%60, otherwise any incoming query parameters (GET)
will
> get included with the redirect.
> 
> This is something I just re-learned recently, though we've discovered and
worked
> around it the same way before. See i.e. `adblockplus/eyeo` above. Also please
> note that I do not know how to prevent this from happening when the target
link
> includes GET parameters itself (some of the new ones here do as well). IIRC
one
> can write something like `foo: bar?baz=1?` to achieve the sane, though you
would
> need to test that upfront.

Your recall is correct, after the last ? the parameters won't be included in the
redirect, I'll add a follow up codereview for this since I already pushed this
change

Powered by Google App Engine
This is Rietveld