Use fetch helpers instead of fetch (#27026)

WIP because: - [x] Some calls set a `content-type` but send no body, can likely remove the header - [x] Need to check whether `charset=utf-8` has any significance on the webauthn calls, I assume not as it is the default for json content. - [x] Maybe `no-restricted-globals` is better for eslint, but will require a lot of duplication in the yaml or moving eslint config to a `.js` extension. - [x] Maybe export `request` as `fetch`, shadowing the global.

silverwind committed 3 years ago

Commit ae8e8f01 parent 809923817 files · +70 −98Browse files

Unified · Split

17 files

.eslintrc.yaml +4 −1
docs/content/contributing/guidelines-frontend.en-us.md +1 −1
web_src/js/components/DashboardRepoList.vue +3 −2
web_src/js/components/DiffCommitSelector.vue +2 −1
web_src/js/components/RepoBranchTagSelector.vue +2 −2
web_src/js/features/common-global.js +4 −3
web_src/js/features/common-issue-list.js +3 −2
web_src/js/features/comp/ImagePaste.js +2 −7
web_src/js/features/comp/ReactionSelector.js +3 −11
web_src/js/features/copycontent.js +2 −1
web_src/js/features/install.js +2 −1
web_src/js/features/pull-view-file.js +3 −6
web_src/js/features/repo-diff-commit.js +2 −1
web_src/js/features/repo-issue-list.js +3 −15
web_src/js/features/repo-migrate.js +4 −9
web_src/js/features/user-auth-webauthn.js +13 −25
web_src/js/modules/fetch.js +17 −10

.eslintrc.yaml+4−1

   - files: ["*.config.*"]
     rules:
       import/no-unused-modules: [0]
+  - files: ["web_src/js/modules/fetch.js", "web_src/js/standalone/**/*"]
+    rules:
+      no-restricted-syntax: [2, WithStatement, ForInStatement, LabeledStatement, SequenceExpression]
 rules:
   "@eslint-community/eslint-comments/disable-enable-pair": [2]
   no-restricted-exports: [0]
   no-restricted-globals: [2, addEventListener, blur, close, closed, confirm, defaultStatus, defaultstatus, error, event, external, find, focus, frameElement, frames, history, innerHeight, innerWidth, isFinite, isNaN, length, location, locationbar, menubar, moveBy, moveTo, name, onblur, onerror, onfocus, onload, onresize, onunload, open, opener, opera, outerHeight, outerWidth, pageXOffset, pageYOffset, parent, print, removeEventListener, resizeBy, resizeTo, screen, screenLeft, screenTop, screenX, screenY, scroll, scrollbars, scrollBy, scrollTo, scrollX, scrollY, self, status, statusbar, stop, toolbar, top, __dirname, __filename]
   no-restricted-imports: [0]
-  no-restricted-syntax: [2, WithStatement, ForInStatement, LabeledStatement, SequenceExpression]
+  no-restricted-syntax: [2, WithStatement, ForInStatement, LabeledStatement, SequenceExpression, {selector: "CallExpression[callee.name='fetch']", message: "use modules/fetch.js instead"}]
   no-return-assign: [0]
   no-script-url: [2]
   no-self-assign: [2, {props: true}]

docs/content/contributing/guidelines-frontend.en-us.md+1−1

@@ -95,7 +95,7 @@ Some lint rules and IDEs also have warnings if the returned Promise is not handl · 94 unmodified lines
95	### Fetching data	95	### Fetching data
96		96
97	To fetch data, use the wrapper functions `GET`, `POST` etc. from `modules/fetch.js`. They	97	To fetch data, use the wrapper functions `GET`, `POST` etc. from `modules/fetch.js`. They
98	accept a `data` option for the content, will automatically set CSFR token and return a	98	accept a `data` option for the content, will automatically set CSRF token and return a
99	Promise for a [Response](https://developer.mozilla.org/en-US/docs/Web/API/Response).	99	Promise for a [Response](https://developer.mozilla.org/en-US/docs/Web/API/Response).
100		100
101	### HTML Attributes and `dataset`	101	### HTML Attributes and `dataset`

web_src/js/components/DashboardRepoList.vue+3−2

 import {createApp, nextTick} from 'vue';
 import $ from 'jquery';
 import {SvgIcon} from '../svg.js';
+import {GET} from '../modules/fetch.js';
 const {appSubUrl, assetUrlPrefix, pageData} = window.config;
       try {
         if (!this.reposTotalCount) {
           const totalCountSearchURL = `${this.subUrl}/repo/search?count_only=1&uid=${this.uid}&team_id=${this.teamId}&q=&page=1&mode=`;
-          response = await fetch(totalCountSearchURL);
+          response = await GET(totalCountSearchURL);
           this.reposTotalCount = response.headers.get('X-Total-Count');
         }
-        response = await fetch(searchedURL);
+        response = await GET(searchedURL);
         json = await response.json();
       } catch {
         if (searchedURL === this.searchURL) {

web_src/js/components/DiffCommitSelector.vue+2−1

 <script>
 import {SvgIcon} from '../svg.js';
+import {GET} from '../modules/fetch.js';
 export default {
   components: {SvgIcon},
     },
     /** Load the commits to show in this dropdown */
     async fetchCommits() {
-      const resp = await fetch(`${this.issueLink}/commits/list`);
+      const resp = await GET(`${this.issueLink}/commits/list`);
       const results = await resp.json();
       this.commits.push(...results.commits.map((x) => {
         x.hovered = false;

web_src/js/components/RepoBranchTagSelector.vue+2−2

 import {SvgIcon} from '../svg.js';
 import {pathEscapeSegments} from '../utils/url.js';
 import {showErrorToast} from '../modules/toast.js';
+import {GET} from '../modules/fetch.js';
 const sfc = {
   components: {SvgIcon},
       }
       this.isLoading = true;
       try {
-        const reqUrl = `${this.repoLink}/${this.mode}/list`;
+        const resp = await GET(`${this.repoLink}/${this.mode}/list`);
-        const resp = await fetch(reqUrl);
         const {results} = await resp.json();
         for (const result of results) {
           let selected = false;

web_src/js/features/common-global.js+4−3

 import {showTemporaryTooltip} from '../modules/tippy.js';
 import {confirmModal} from './comp/ConfirmModal.js';
 import {showErrorToast} from '../modules/toast.js';
+import {request} from '../modules/fetch.js';
 const {appUrl, appSubUrl, csrfToken, i18n} = window.config;
 async function fetchActionDoRequest(actionElem, url, opt) {
   try {
-    const resp = await fetch(url, opt);
+    const resp = await request(url, opt);
     if (resp.status === 200) {
       let {redirect} = await resp.json();
       redirect = redirect || actionElem.getAttribute('data-redirect');
   }
   let reqUrl = formActionUrl;
-  const reqOpt = {method: formMethod.toUpperCase(), headers: {'X-Csrf-Token': csrfToken}};
+  const reqOpt = {method: formMethod.toUpperCase()};
   if (formMethod.toLowerCase() === 'get') {
     const params = new URLSearchParams();
     for (const [key, value] of formData) {
   const url = el.getAttribute('data-url');
   const doRequest = async () => {
     el.disabled = true;
-    await fetchActionDoRequest(el, url, {method: 'POST', headers: {'X-Csrf-Token': csrfToken}});
+    await fetchActionDoRequest(el, url, {method: 'POST'});
     el.disabled = false;
   };

web_src/js/features/common-issue-list.js+3−2

 import $ from 'jquery';
 import {isElemHidden, onInputDebounce, toggleElem} from '../utils/dom.js';
-const {appSubUrl} = window.config;
+import {GET} from '../modules/fetch.js';
+const {appSubUrl} = window.config;
 const reIssueIndex = /^(\d+)$/; // eg: "123"
 const reIssueSharpIndex = /^#(\d+)$/; // eg: "#123"
 const reIssueOwnerRepoIndex = /^([-.\w]+)\/([-.\w]+)#(\d+)$/;  // eg: "{owner}/{repo}#{index}"
     // try to check whether the parsed goto link is valid
     let targetUrl = parseIssueListQuickGotoLink(repoLink, searchText);
     if (targetUrl) {
-      const res = await fetch(`${targetUrl}/info`);
+      const res = await GET(`${targetUrl}/info`);
       if (res.status !== 200) targetUrl = '';
     }

web_src/js/features/comp/ImagePaste.js+2−7

@@ -1,16 +1,11 @@
1	import $ from 'jquery';	1	import $ from 'jquery';
2		2	import {POST} from '../../modules/fetch.js';
3	const {csrfToken} = window.config;
4		3
5	async function uploadFile(file, uploadUrl) {	4	async function uploadFile(file, uploadUrl) {
6	const formData = new FormData();	5	const formData = new FormData();
7	formData.append('file', file, file.name);	6	formData.append('file', file, file.name);
8		7
9	const res = await fetch(uploadUrl, {	8	const res = await POST(uploadUrl, {data: formData});
10	method: 'POST',
11	headers: {'X-Csrf-Token': csrfToken},
12	body: formData,
13	});
14	return await res.json();	9	return await res.json();
15	}	10	}
16		11

web_src/js/features/comp/ReactionSelector.js+3−11

@@ -1,6 +1,5 @@
1	import $ from 'jquery';	1	import $ from 'jquery';
2		2	import {POST} from '../../modules/fetch.js';
3	const {csrfToken} = window.config;
4		3
5	export function initCompReactionSelector($parent) {	4	export function initCompReactionSelector($parent) {
6	$parent.find(`.select-reaction .item.reaction, .comment-reaction-button`).on('click', async function (e) {	5	$parent.find(`.select-reaction .item.reaction, .comment-reaction-button`).on('click', async function (e) {
@@ -12,15 +11,8 @@ export function initCompReactionSelector($parent) { · 5 unmodified lines
12	const reactionContent = $(this).attr('data-reaction-content');	11	const reactionContent = $(this).attr('data-reaction-content');
13	const hasReacted = $(this).closest('.ui.segment.reactions').find(`a[data-reaction-content="${reactionContent}"]`).attr('data-has-reacted') === 'true';	12	const hasReacted = $(this).closest('.ui.segment.reactions').find(`a[data-reaction-content="${reactionContent}"]`).attr('data-has-reacted') === 'true';
14		13
15	const res = await fetch(`${actionUrl}/${hasReacted ? 'unreact' : 'react'}`, {	14	const res = await POST(`${actionUrl}/${hasReacted ? 'unreact' : 'react'}`, {
16	method: 'POST',	15	data: new URLSearchParams({content: reactionContent}),
17	headers: {
18	'content-type': 'application/x-www-form-urlencoded',
19	},
20	body: new URLSearchParams({
21	_csrf: csrfToken,
22	content: reactionContent,
23	}),
24	});	16	});
25		17
26	const data = await res.json();	18	const data = await res.json();

web_src/js/features/copycontent.js+2−1

 import {clippie} from 'clippie';
 import {showTemporaryTooltip} from '../modules/tippy.js';
 import {convertImage} from '../utils.js';
+import {GET} from '../modules/fetch.js';
 const {i18n} = window.config;
     if (link) {
       btn.classList.add('is-loading', 'small-loading-icon');
       try {
-        const res = await fetch(link, {credentials: 'include', redirect: 'follow'});
+        const res = await GET(link, {credentials: 'include', redirect: 'follow'});
         const contentType = res.headers.get('content-type');
         if (contentType.startsWith('image/') && !contentType.startsWith('image/svg')) {

web_src/js/features/install.js+2−1

 import $ from 'jquery';
 import {hideElem, showElem} from '../utils/dom.js';
+import {GET} from '../modules/fetch.js';
 export function initInstall() {
   const $page = $('.page-content.install');
   const targetUrl = el.getAttribute('href');
   let tid = setInterval(async () => {
     try {
-      const resp = await fetch(targetUrl);
+      const resp = await GET(targetUrl);
       if (tid && resp.status === 200) {
         clearInterval(tid);
         tid = null;

web_src/js/features/pull-view-file.js+3−6

 import {diffTreeStore} from '../modules/stores.js';
 import {setFileFolding} from './file-fold.js';
+import {POST} from '../modules/fetch.js';
-const {csrfToken, pageData} = window.config;
+const {pageData} = window.config;
 const prReview = pageData.prReview || {};
 const viewedStyleClass = 'viewed-file-checked-form';
 const viewedCheckboxSelector = '.viewed-file-form'; // Selector under which all "Viewed" checkbox forms can be found
       const data = {files};
       const headCommitSHA = form.getAttribute('data-headcommit');
       if (headCommitSHA) data.headCommitSHA = headCommitSHA;
-      fetch(form.getAttribute('data-link'), {
+      POST(form.getAttribute('data-link'), {data});
-        method: 'POST',
-        headers: {'X-Csrf-Token': csrfToken},
-        body: JSON.stringify(data),
-      });
       // Fold the file accordingly
       const parentBox = form.closest('.diff-file-header');

web_src/js/features/repo-diff-commit.js+2−1

 import {hideElem, showElem, toggleElem} from '../utils/dom.js';
+import {GET} from '../modules/fetch.js';
 async function loadBranchesAndTags(area, loadingButton) {
   loadingButton.classList.add('disabled');
   try {
-    const res = await fetch(loadingButton.getAttribute('data-fetch-url'));
+    const res = await GET(loadingButton.getAttribute('data-fetch-url'));
     const data = await res.json();
     hideElem(loadingButton);
     addTags(area, data.tags);

web_src/js/features/repo-issue-list.js+3−15

 import {confirmModal} from './comp/ConfirmModal.js';
 import {showErrorToast} from '../modules/toast.js';
 import {createSortable} from '../modules/sortable.js';
+import {DELETE, POST} from '../modules/fetch.js';
 function initRepoIssueListCheckboxes() {
   const $issueSelectAll = $('.issue-checkbox-all');
       const id = Number(el.getAttribute('data-issue-id'));
       // Send the unpin request
-      const response = await fetch(el.getAttribute('data-unpin-url'), {
+      const response = await DELETE(el.getAttribute('data-unpin-url'));
-        method: 'delete',
-        headers: {
-          'X-Csrf-Token': window.config.csrfToken,
-          'Content-Type': 'application/json',
-        },
-      });
       if (response.ok) {
         // Delete the tooltip
         el._tippy.destroy();
 async function pinMoveEnd(e) {
   const url = e.item.getAttribute('data-move-url');
   const id = Number(e.item.getAttribute('data-issue-id'));
-  await fetch(url, {
+  await POST(url, {data: {id, position: e.newIndex + 1}});
-    method: 'post',
-    body: JSON.stringify({id, position: e.newIndex + 1}),
-    headers: {
-      'X-Csrf-Token': window.config.csrfToken,
-      'Content-Type': 'application/json',
-    },
-  });
 }
 async function initIssuePinSort() {

web_src/js/features/repo-migrate.js+4−9

 import $ from 'jquery';
 import {hideElem, showElem} from '../utils/dom.js';
+import {GET, POST} from '../modules/fetch.js';
-const {appSubUrl, csrfToken} = window.config;
+const {appSubUrl} = window.config;
 export function initRepoMigrationStatusChecker() {
   const $repoMigrating = $('#repo_migrating');
   // returns true if the refresh still need to be called after a while
   const refresh = async () => {
-    const res = await fetch(`${appSubUrl}/user/task/${task}`);
+    const res = await GET(`${appSubUrl}/user/task/${task}`);
     if (res.status !== 200) return true; // continue to refresh if network error occurs
     const data = await res.json();
 }
 async function doMigrationRetry(e) {
-  await fetch($(e.target).attr('data-migrating-task-retry-url'), {
+  await POST($(e.target).attr('data-migrating-task-retry-url'));
-    method: 'post',
-    headers: {
-      'X-Csrf-Token': csrfToken,
-      'Content-Type': 'application/json',
-    },
-  });
   window.location.reload();
 }

web_src/js/features/user-auth-webauthn.js+13−25

 import {encodeURLEncodedBase64, decodeURLEncodedBase64} from '../utils.js';
 import {showElem} from '../utils/dom.js';
+import {GET, POST} from '../modules/fetch.js';
-const {appSubUrl, csrfToken} = window.config;
+const {appSubUrl} = window.config;
 export async function initUserAuthWebAuthn() {
   const elPrompt = document.querySelector('.user.signin.webauthn-prompt');
     return;
   }
-  const res = await fetch(`${appSubUrl}/user/webauthn/assertion`);
+  const res = await GET(`${appSubUrl}/user/webauthn/assertion`);
   if (res.status !== 200) {
     webAuthnError('unknown');
     return;
   const sig = new Uint8Array(assertedCredential.response.signature);
   const userHandle = new Uint8Array(assertedCredential.response.userHandle);
-  const res = await fetch(`${appSubUrl}/user/webauthn/assertion`, {
+  const res = await POST(`${appSubUrl}/user/webauthn/assertion`, {
-    method: 'POST',
+    data: {
-    headers: {
-      'Content-Type': 'application/json; charset=utf-8'
-    },
-    body: JSON.stringify({
       id: assertedCredential.id,
       rawId: encodeURLEncodedBase64(rawId),
       type: assertedCredential.type,
         signature: encodeURLEncodedBase64(sig),
         userHandle: encodeURLEncodedBase64(userHandle),
       },
-    }),
+    },
   });
   if (res.status === 500) {
     webAuthnError('unknown');
   const clientDataJSON = new Uint8Array(newCredential.response.clientDataJSON);
   const rawId = new Uint8Array(newCredential.rawId);
-  const res = await fetch(`${appSubUrl}/user/settings/security/webauthn/register`, {
+  const res = await POST(`${appSubUrl}/user/settings/security/webauthn/register`, {
-    method: 'POST',
+    data: {
-    headers: {
-      'X-Csrf-Token': csrfToken,
-      'Content-Type': 'application/json; charset=utf-8',
-    },
-    body: JSON.stringify({
       id: newCredential.id,
       rawId: encodeURLEncodedBase64(rawId),
       type: newCredential.type,
         attestationObject: encodeURLEncodedBase64(attestationObject),
         clientDataJSON: encodeURLEncodedBase64(clientDataJSON),
       },
-    }),
+    },
   });
   if (res.status === 409) {
 async function webAuthnRegisterRequest() {
   const elNickname = document.getElementById('nickname');
-  const body = new FormData();
+  const formData = new FormData();
-  body.append('name', elNickname.value);
+  formData.append('name', elNickname.value);
-  const res = await fetch(`${appSubUrl}/user/settings/security/webauthn/request_register`, {
+  const res = await POST(`${appSubUrl}/user/settings/security/webauthn/request_register`, {
-    method: 'POST',
+    data: formData,
-    headers: {
-      'X-Csrf-Token': csrfToken,
-    },
-    body,
   });
   if (res.status === 409) {

web_src/js/modules/fetch.js+17−10

 const {csrfToken} = window.config;
+// safe HTTP methods that don't need a csrf token
+const safeMethods = new Set(['GET', 'HEAD', 'OPTIONS', 'TRACE']);
 // fetch wrapper, use below method name functions and the `data` option to pass in data
-// which will automatically set an appropriate content-type header. For json content,
+// which will automatically set an appropriate headers. For json content, only object
-// only object and array types are currently supported.
+// and array types are currently supported.
-function request(url, {headers, data, body, ...other} = {}) {
+export function request(url, {method = 'GET', headers = {}, data, body, ...other} = {}) {
   let contentType;
   if (!body) {
     if (data instanceof FormData) {
-      contentType = 'multipart/form-data';
       body = data;
     } else if (data instanceof URLSearchParams) {
-      contentType = 'application/x-www-form-urlencoded';
       body = data;
     } else if (isObject(data) || Array.isArray(data)) {
       contentType = 'application/json';
     }
   }
+  const headersMerged = new Headers({
+    ...(!safeMethods.has(method.toUpperCase()) && {'x-csrf-token': csrfToken}),
+    ...(contentType && {'content-type': contentType}),
+  });
+  for (const [name, value] of Object.entries(headers)) {
+    headersMerged.set(name, value);
+  }
   return fetch(url, {
-    headers: {
+    method,
-      'x-csrf-token': csrfToken,
+    headers: headersMerged,
-      ...(contentType && {'content-type': contentType}),
-      ...headers,
-    },
     ...(body && {body}),
     ...other,
   });