Niedzielski has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/392573 )
Change subject: Fix: gracefully handle redirects to external sites
......................................................................
Fix: gracefully handle redirects to external sites
When redirected to externals sites such as Commons by the Mobile Content
Service API, don't report the redirect status to the client. Marvin only
supports English Wikipedia. This is a hack and probably will break under
certain scenarios but at least will unbreak File pages on Commons.
Bug: T177681
Change-Id: Idbb9803b36acd503d97ac666deb47a19b29eed57
---
M package.json
M src/common/http/page-http-client.ts
M src/common/http/page-summary-http-client.ts
M src/common/pages/home.tsx
M src/common/pages/summary.tsx
M src/common/pages/wiki.tsx
6 files changed, 79 insertions(+), 49 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/marvin refs/changes/73/392573/1
diff --git a/package.json b/package.json
index 33ff1af..1c928f5 100644
--- a/package.json
+++ b/package.json
@@ -123,11 +123,11 @@
},
{
"path": "dist/public/pages/summary.*.js",
- "maxSize": "2.3KB"
+ "maxSize": "2.4KB"
},
{
"path": "dist/public/pages/wiki.*.js",
- "maxSize": "2.9KB"
+ "maxSize": "3KB"
}
]
}
diff --git a/src/common/http/page-http-client.ts
b/src/common/http/page-http-client.ts
index c7c015e..0326917 100644
--- a/src/common/http/page-http-client.ts
+++ b/src/common/http/page-http-client.ts
@@ -19,8 +19,9 @@
revision?: number;
redirect?: PageRedirect;
random?: undefined;
+ init?: RequestInit;
}
-export type Params = PageParams | { random: true };
+export type Params = PageParams | { random: true; init?: RequestInit };
function url(params: Params, endpoint: string) {
if (params.random) {
@@ -51,7 +52,7 @@
unmarshal: (params: UnmarshalParams) => Type
): Promise<HttpResponse<Type>> {
const headers = params.random ? RANDOM_HEADERS : PAGE_HEADERS;
- return fetch(url(params, endpoint), { headers })
+ return fetch(url(params, endpoint), { headers, ...params.init })
.then(response =>
response
.json()
diff --git a/src/common/http/page-summary-http-client.ts
b/src/common/http/page-summary-http-client.ts
index 140fdc4..7e35d3d 100644
--- a/src/common/http/page-summary-http-client.ts
+++ b/src/common/http/page-summary-http-client.ts
@@ -13,8 +13,9 @@
titlePath: PageTitlePath;
redirect?: PageRedirect;
random?: undefined;
+ init?: RequestInit;
}
-export type Params = PageParams | { random: true };
+export type Params = PageParams | { random: true; init?: RequestInit };
const url = (params: Params) => {
if (params.random) {
@@ -34,7 +35,10 @@
// todo: this can actually return an empty response when redirect is false. Do
// we want to support it? Same question for the other redirect usages.
export const request = (params: Params): Promise<HttpResponse<PageSummary>> =>
- fetch(url(params), { headers: params.random ? RANDOM_HEADERS : PAGE_HEADERS
})
+ fetch(url(params), {
+ headers: params.random ? RANDOM_HEADERS : PAGE_HEADERS,
+ ...params.init
+ })
.then(response =>
response
.json()
diff --git a/src/common/pages/home.tsx b/src/common/pages/home.tsx
index 20b1a64..d500b3b 100644
--- a/src/common/pages/home.tsx
+++ b/src/common/pages/home.tsx
@@ -61,6 +61,10 @@
title: "Ice_cream_cake",
revision: "24242119",
text: "An arbitrary revision"
+ },
+ {
+ title: "File:Vanilla_Ice_Cream_Cone_at_Camp_Manitoulin.jpg",
+ text: "Redirect (external) and File page"
}
];
diff --git a/src/common/pages/summary.tsx b/src/common/pages/summary.tsx
index cac2889..c6a5601 100644
--- a/src/common/pages/summary.tsx
+++ b/src/common/pages/summary.tsx
@@ -6,7 +6,7 @@
import Page from "../components/page/page";
import { RouteParams } from "../routers/route";
import { summary } from "../routers/api";
-import { request } from "../http/page-summary-http-client";
+import { request as requestSummary } from "../http/page-summary-http-client";
import ContentHeader from "../components/content-header/content-header";
import ContentFooter from "../components/content-footer/content-footer";
import HttpResponse from "../http/http-response";
@@ -30,26 +30,36 @@
summary: PageSummaryModel;
}
+function request(
+ params: Params = {},
+ init?: RequestInit
+): Promise<HttpResponse<PageSummaryModel>> {
+ return requestSummary(
+ params.title === undefined
+ ? { random: true, init }
+ : { titlePath: params.title, init }
+ ).catch(error => {
+ if (error instanceof RedirectError) {
+ const titleID = unmarshalPageTitleID(error.url);
+ if (params.title === encodeURIComponent(titleID)) {
+ // URL is external. Follow it and don't report redirect status.
+ return request(params, { redirect: "follow" });
+ }
+
+ const url = summary.toPath({ title: unmarshalPageTitleID(error.url) });
+ throw new RedirectError(error.status, url);
+ }
+
+ throw error;
+ });
+}
+
export default {
getInitialProps(params: Params = {}): Promise<HttpResponse<Props>> {
- return request(
- params.title === undefined
- ? { random: true }
- : { titlePath: params.title }
- )
- .then(({ status, data }) => ({
- status,
- data: { summary: data }
- }))
- .catch(error => {
- if (error instanceof RedirectError) {
- error = new RedirectError(
- error.status,
- summary.toPath({ title: unmarshalPageTitleID(error.url) })
- );
- }
- throw error;
- });
+ return request(params).then(({ status, data }) => ({
+ status,
+ data: { summary: data }
+ }));
},
Component({ summary }: Props): JSX.Element {
diff --git a/src/common/pages/wiki.tsx b/src/common/pages/wiki.tsx
index ef011b8..ec747d5 100644
--- a/src/common/pages/wiki.tsx
+++ b/src/common/pages/wiki.tsx
@@ -36,32 +36,43 @@
page: PageModel;
}
+function request(
+ params: Params = {},
+ init?: RequestInit
+): Promise<HttpResponse<PageModel>> {
+ return requestPage(
+ params.title === undefined
+ ? { random: true, init }
+ : {
+ titlePath: params.title,
+ revision:
+ params.revision === undefined
+ ? undefined
+ : parseInt(params.revision, 10),
+ init
+ }
+ ).catch(error => {
+ if (error instanceof RedirectError) {
+ const titleID = unmarshalPageTitleID(error.url);
+ if (params.title === encodeURIComponent(titleID)) {
+ // URL is external. Follow it and don't report redirect status.
+ return request(params, { redirect: "follow" });
+ }
+
+ const url = wiki.toPath({ title: titleID, revision: params.revision });
+ throw new RedirectError(error.status, url);
+ }
+
+ throw error;
+ });
+}
+
export default {
getInitialProps(params: Params = {}): Promise<HttpResponse<Props>> {
- return requestPage(
- params.title === undefined
- ? { random: true }
- : {
- titlePath: params.title,
- revision:
- params.revision === undefined
- ? undefined
- : parseInt(params.revision, 10)
- }
- )
- .then(({ status, data }) => ({ status, data: { page: data } }))
- .catch(error => {
- if (error instanceof RedirectError) {
- error = new RedirectError(
- error.status,
- wiki.toPath({
- title: unmarshalPageTitleID(error.url),
- revision: params.revision
- })
- );
- }
- throw error;
- });
+ return request(params).then(({ status, data }) => ({
+ status,
+ data: { page: data }
+ }));
},
Component({ page }: Props): JSX.Element {
--
To view, visit https://gerrit.wikimedia.org/r/392573
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idbb9803b36acd503d97ac666deb47a19b29eed57
Gerrit-PatchSet: 1
Gerrit-Project: marvin
Gerrit-Branch: master
Gerrit-Owner: Niedzielski <[email protected]>
Gerrit-Reviewer: Sniedzielski <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
