Skip to content

Commit

Permalink
Limit non-English locales (#3661)
Browse files Browse the repository at this point in the history
* update/add locale constants

* add and test redirects of retired locales

* fix locale alias for pt

* fix for developing.test.js cookie test

* fix gather-git-history

* add integration tests for retired locale redirects

* move redirects into the fundamental redirects

* add lambda tests (feedbacked)

* use DEFAULT_LOCALE (feedbacked)
  • Loading branch information
escattone authored May 3, 2021
1 parent 7a4a06b commit 974f86a
Show file tree
Hide file tree
Showing 11 changed files with 343 additions and 52 deletions.
22 changes: 0 additions & 22 deletions client/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,37 +25,15 @@ export const AUTOCOMPLETE_SEARCH_WIDGET = JSON.parse(
// We could encode the list in the SSR rendering but that means the client side
// code needs to depend on having access to the `window` global first.
export const VALID_LOCALES = new Set([
"ar",
"bg",
"bn",
"ca",
"de",
"el",
"en-US",
"es",
"fa",
"fi",
"fr",
"he",
"hi-IN",
"hu",
"id",
"it",
"ja",
"kab",
"ko",
"ms",
"my",
"nl",
"pl",
"pt-BR",
"pt-PT",
"ru",
"sv-SE",
"th",
"tr",
"uk",
"vi",
"zh-CN",
"zh-TW",
]);
Expand Down
13 changes: 8 additions & 5 deletions deployer/aws-lambda/content-origin-request/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,14 @@ exports.handler = async (event) => {
return redirect(`/${request.uri.replace(/^\/+/g, "")}`);
}

const { url, status } = resolveFundamental(request.uri);
let { url, status } = resolveFundamental(request.uri);
if (url) {
// TODO: Do we want to add the query string to the redirect?
// If we decide we do, then we probably need to change
// the caching policy on the "*/docs/* behavior" to
// cache based on the query strings as well.
// NOTE: The query string is not forwarded for document requests,
// as directed by their origin request policy, so it's safe to
// assume "request.querystring" is empty for document requests.
if (request.querystring) {
url += (url.includes("?") ? "&" : "?") + request.querystring;
}
return redirect(url, {
status,
cacheControlSeconds: THIRTY_DAYS,
Expand All @@ -127,6 +129,7 @@ exports.handler = async (event) => {
const path = request.uri.endsWith("/")
? request.uri.slice(0, -1)
: request.uri;
// Note that "getLocale" only returns valid locales, never a retired locale.
const locale = getLocale(request);
// The only time we actually want a trailing slash is when the URL is just
// the locale. E.g. `/en-US/` (not `/en-US`)
Expand Down
31 changes: 31 additions & 0 deletions deployer/aws-lambda/content-origin-request/server.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,3 +207,34 @@ describe("redirect double-slash prefix URIs", () => {
expect(r.headers["location"]).toBe("/blablabla");
});
});

describe("retired locale redirects", () => {
it("should 302 redirect a retired locale (accept-language)", async () => {
const r = await get("/", {
"Accept-language": "sv-SE",
});
expect(r.statusCode).toBe(302);
expect(r.headers["location"]).toBe("/en-US/");
});
it("should 302 redirect a retired locale (preferredlocale cookie)", async () => {
const r = await get("/docs/Web/HTTP", {
Cookie: "preferredlocale=it",
});
expect(r.statusCode).toBe(302);
expect(r.headers["location"]).toBe("/en-US/docs/Web/HTTP");
});
it("should 302 redirect a retired locale (no query string)", async () => {
const r = await get("/sv-SE/docs/Web/HTML");
expect(r.statusCode).toBe(302);
expect(r.headers["location"]).toBe(
"/en-US/docs/Web/HTML?retiredLocale=sv-SE"
);
});
it("should 302 redirect a retired locale (query string, improper locale)", async () => {
const r = await get("/BN/search?q=video");
expect(r.statusCode).toBe(302);
expect(r.headers["location"]).toBe(
"/en-US/search?retiredLocale=bn&q=video"
);
});
});
30 changes: 18 additions & 12 deletions libs/constants/index.js
Original file line number Diff line number Diff line change
@@ -1,38 +1,43 @@
const VALID_LOCALES = new Map(
[
"de",
"en-US",
"es",
"fr",
"ja",
"ko",
"pl",
"pt-BR",
"ru",
"zh-CN",
"zh-TW",
].map((x) => [x.toLowerCase(), x])
);

const RETIRED_LOCALES = new Map(
[
"ar",
"bg",
"bn",
"ca",
"de",
"el",
"en-US",
"es",
"fa",
"fi",
"fr",
"he",
"hi-IN",
"hu",
"id",
"it",
"ja",
"kab",
"ko",
"ms",
"my",
"nl",
"pl",
"pt-BR",
"pt-PT",
"ru",
"sv-SE",
"th",
"tr",
"uk",
"vi",
"zh-CN",
"zh-TW",
].map((x) => [x.toLowerCase(), x])
);

Expand All @@ -41,7 +46,7 @@ const DEFAULT_LOCALE = "en-US";
const LOCALE_ALIASES = new Map([
// Case is not important on either the keys or the values.
["en", "en-us"],
["pt", "pt-PT"], // Note! Portugal Portugese is the default
["pt", "pt-br"],
["cn", "zh-cn"],
["zh", "zh-cn"],
["zh-hans", "zh-cn"],
Expand All @@ -64,6 +69,7 @@ const ACTIVE_LOCALES = new Set([
module.exports = {
ACTIVE_LOCALES,
VALID_LOCALES,
RETIRED_LOCALES,
DEFAULT_LOCALE,
LOCALE_ALIASES,
PREFERRED_LOCALE_COOKIE_NAME,
Expand Down
26 changes: 23 additions & 3 deletions libs/fundamental-redirects/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
const { VALID_LOCALES, LOCALE_ALIASES } = require("../constants");
const {
DEFAULT_LOCALE,
VALID_LOCALES,
LOCALE_ALIASES,
RETIRED_LOCALES,
} = require("../constants");

const startRe = /^\^?\/?/;
const startTemplate = /^\//;
Expand Down Expand Up @@ -65,12 +70,12 @@ for (const locale of VALID_LOCALES.keys()) {
}

for (const [alias, correct] of LOCALE_ALIASES) {
// E.g. things like `en` -> `en-us` or `pt` -> `pt-pt`
// E.g. things like `en` -> `en-us` or `pt` -> `pt-br`
fixableLocales.set(alias, correct);
}

// All things like `/en_Us/docs/...` -> `/en-US/docs/...`
const LOCALE_PATTERNS = [
// All things like `/en_Us/docs/...` -> `/en-US/docs/...`
redirect(
new RegExp(
`^(?<locale>${Array.from(fixableLocales.keys()).join(
Expand All @@ -92,6 +97,21 @@ const LOCALE_PATTERNS = [
},
{ permanent: true }
),
// Retired locales
redirect(
new RegExp(
`^(?<locale>${Array.from(RETIRED_LOCALES.keys()).join(
"|"
)})(/(?<suffix>.*)|$)`,
"i"
),
({ locale, suffix }) => {
const join = suffix && suffix.includes("?") ? "&" : "?";
return `/${DEFAULT_LOCALE}/${
(suffix || "") + join
}retiredLocale=${RETIRED_LOCALES.get(locale.toLowerCase())}`;
}
),
];

// Redirects/rewrites/aliases migrated from SCL3 httpd config
Expand Down
4 changes: 2 additions & 2 deletions testing/integration/headless/map_301.py
Original file line number Diff line number Diff line change
Expand Up @@ -1069,8 +1069,8 @@
url_test("/EN-US/?next=FOO", "/en-US/?next=FOO", status_code=302),
url_test("/eN-us/docs/Web", "/en-US/docs/Web", status_code=302),
url_test("/eN-us/docs/Web/", "/en-US/docs/Web", status_code=302),
url_test("/eN-us/docs/Web?next=FOO", "/en-US/docs/Web?next=FOO", status_code=302),
url_test("/eN-us/docs/Web/?next=FOO", "/en-US/docs/Web?next=FOO", status_code=302),
url_test("/eN-us/docs/Web?next=FOO", "/en-US/docs/Web", status_code=302),
url_test("/eN-us/docs/Web/?next=FOO", "/en-US/docs/Web", status_code=302),
url_test("/en-uS/search", "/en-US/search", status_code=302),
url_test("/en-uS/search/", "/en-US/search", status_code=302),
url_test("/en-Us/search?q=video", "/en-US/search?q=video", status_code=302),
Expand Down
57 changes: 57 additions & 0 deletions testing/integration/headless/test_redirects.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import pytest

from . import request
from utils.urls import assert_valid_url

from .map_301 import (
Expand Down Expand Up @@ -116,3 +117,59 @@ def test_firefox_source_docs_redirects(url, base_url):
def test_misc_redirects(url, base_url):
url["base_url"] = base_url
assert_valid_url(**url)


@pytest.mark.parametrize(
"retired_locale",
(
"ar",
"bg",
"bn",
"ca",
"el",
"fa",
"fi",
"he",
"hi-IN",
"hu",
"id",
"it",
"kab",
"ms",
"my",
"nl",
"pt-PT",
"sv-SE",
"th",
"tr",
"uk",
"vi",
),
)
@pytest.mark.parametrize(
"slug",
[
"",
"/",
"/docs/Web",
"/docs/Web/",
"/search",
"/search/",
"/search?q=video",
"/search/?q=video",
"/signup",
"/settings",
],
)
def test_retired_locale_redirects(base_url, slug, retired_locale):
"""Ensure that requests for retired locales properly redirect."""
resp = request("get", f"{base_url}/{retired_locale}{slug}")
assert resp.status_code == 302
slug_parts = slug.split("?")
expected_slug = slug_parts[0].lstrip("/")
expected_qs = f"?retiredLocale={retired_locale}"
if len(slug_parts) > 1:
expected_qs += f"&{slug_parts[1]}"
assert (
resp.headers["Location"] == f"/en-US/{expected_slug}{expected_qs}"
), f"{resp.headers['Location']} is not /en-US/{expected_slug}{expected_qs}"
4 changes: 2 additions & 2 deletions testing/tests/developing.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,12 +166,12 @@ describe("Testing the Express server", () => {
const response = await got(serverURL("/"), {
followRedirect: false,
headers: {
Cookie: "preferredlocale=SV-se",
Cookie: "preferredlocale=ja",
"Accept-language": "fr-CH, fr;q=0.9, en;q=0.8, de;q=0.7, *;q=0.5",
},
});
expect(response.statusCode).toBe(302);
expect(response.headers.location).toBe("/sv-SE/");
expect(response.headers.location).toBe("/ja/");
});
});

Expand Down
6 changes: 3 additions & 3 deletions testing/tests/headless.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,10 +218,10 @@ describe("Basic viewing of functional pages", () => {
});

it("should suggest the en-US equivalent on non-en-US pages not found", async () => {
await page.goto(testURL("/sv-SE/docs/Web/foo"));
await page.goto(testURL("/ja/docs/Web/foo"));
await expect(page).toMatch("Page not found");
await expect(page).toMatch("/sv-SE/docs/Web/foo could not be found");
// Simply by swapping the "sv-SE" for "en-US" it's able to find the index.json
await expect(page).toMatch("/ja/docs/Web/foo could not be found");
// Simply by swapping the "ja" for "en-US" it's able to find the index.json
// for that slug and present a link to it.
await expect(page).toMatch("Good news!");
await expect(page).toMatchElement("a", {
Expand Down
Loading

0 comments on commit 974f86a

Please sign in to comment.