fix(settings): fixed two IPFS gateway issues (#19700)

* fix(settings): fixed two IPFS gateway issues

- adds back in two bugfixes that were originally in #19283
  - fixes #16871
  - fixes #18140

- achieves 100% code coverage for /ui/pages/settings/security-tab
- removes the npm package `valid-url`, which has not been updated in 10 years

* changes after #20172 was merged

* improved URL validation (specifically spaces)

* better Jest coverage

* response to legobeat review

* fixing lint and Jest

app/scripts/lib/rpc-method-middleware/handlers/add-ethereum-chain.js

@@ -1,16 +1,16 @@
-import { ethErrors, errorCodes } from 'eth-rpc-errors';
-import validUrl from 'valid-url';
-import { omit } from 'lodash';
 import { ApprovalType } from '@metamask/controller-utils';
+import { errorCodes, ethErrors } from 'eth-rpc-errors';
+import { omit } from 'lodash';
 import {
   MESSAGE_TYPE,
   UNKNOWN_TICKER_SYMBOL,
 } from '../../../../../shared/constants/app';
+import { MetaMetricsNetworkEventSource } from '../../../../../shared/constants/metametrics';
 import {
   isPrefixedFormattedHexString,
   isSafeChainId,
 } from '../../../../../shared/modules/network.utils';
-import { MetaMetricsNetworkEventSource } from '../../../../../shared/constants/metametrics';
+import { getValidUrl } from '../../util';
 
 const addEthereumChain = {
   methodNames: [MESSAGE_TYPE.ADD_ETHEREUM_CHAIN],
@@ -83,27 +83,25 @@ async function addEthereumChainHandler(
     );
   }
 
-  const isLocalhost = (strUrl) => {
-    try {
-      const url = new URL(strUrl);
-      return url.hostname === 'localhost' || url.hostname === '127.0.0.1';
-    } catch (error) {
-      return false;
-    }
-  };
+  function isLocalhostOrHttps(urlString) {
+    const url = getValidUrl(urlString);
+
+    return (
+      url !== null &&
+      (url.hostname === 'localhost' ||
+        url.hostname === '127.0.0.1' ||
+        url.protocol === 'https:')
+    );
+  }
 
   const firstValidRPCUrl = Array.isArray(rpcUrls)
-    ? rpcUrls.find(
-        (rpcUrl) => isLocalhost(rpcUrl) || validUrl.isHttpsUri(rpcUrl),
-      )
+    ? rpcUrls.find((rpcUrl) => isLocalhostOrHttps(rpcUrl))
     : null;
 
   const firstValidBlockExplorerUrl =
     blockExplorerUrls !== null && Array.isArray(blockExplorerUrls)
-      ? blockExplorerUrls.find(
-          (blockExplorerUrl) =>
-            isLocalhost(blockExplorerUrl) ||
-            validUrl.isHttpsUri(blockExplorerUrl),
+      ? blockExplorerUrls.find((blockExplorerUrl) =>
+          isLocalhostOrHttps(blockExplorerUrl),
         )
       : null;
 

app/scripts/lib/util.test.js

@@ -1,24 +1,27 @@
-import { isPrefixedFormattedHexString } from '../../../shared/modules/network.utils';
 import {
-  ENVIRONMENT_TYPE_POPUP,
-  ENVIRONMENT_TYPE_NOTIFICATION,
-  ENVIRONMENT_TYPE_FULLSCREEN,
   ENVIRONMENT_TYPE_BACKGROUND,
-  PLATFORM_FIREFOX,
-  PLATFORM_OPERA,
+  ENVIRONMENT_TYPE_FULLSCREEN,
+  ENVIRONMENT_TYPE_NOTIFICATION,
+  ENVIRONMENT_TYPE_POPUP,
   PLATFORM_CHROME,
   PLATFORM_EDGE,
+  PLATFORM_FIREFOX,
+  PLATFORM_OPERA,
 } from '../../../shared/constants/app';
 import {
+  TransactionEnvelopeType,
   TransactionStatus,
   TransactionType,
-  TransactionEnvelopeType,
 } from '../../../shared/constants/transaction';
+import { isPrefixedFormattedHexString } from '../../../shared/modules/network.utils';
 import {
+  addUrlProtocolPrefix,
   deferredPromise,
+  formatTxMetaForRpcResult,
   getEnvironmentType,
   getPlatform,
-  formatTxMetaForRpcResult,
+  getValidUrl,
+  isWebUrl,
 } from './util';
 
 describe('app utils', () => {
@@ -73,6 +76,39 @@ describe('app utils', () => {
     });
   });
 
+  describe('URL utils', () => {
+    it('should test addUrlProtocolPrefix', () => {
+      expect(addUrlProtocolPrefix('http://example.com')).toStrictEqual(
+        'http://example.com',
+      );
+      expect(addUrlProtocolPrefix('https://example.com')).toStrictEqual(
+        'https://example.com',
+      );
+      expect(addUrlProtocolPrefix('example.com')).toStrictEqual(
+        'https://example.com',
+      );
+      expect(addUrlProtocolPrefix('exa mple.com')).toStrictEqual(null);
+    });
+
+    it('should test isWebUrl', () => {
+      expect(isWebUrl('http://example.com')).toStrictEqual(true);
+      expect(isWebUrl('https://example.com')).toStrictEqual(true);
+      expect(isWebUrl('https://exa mple.com')).toStrictEqual(false);
+      expect(isWebUrl('')).toStrictEqual(false);
+    });
+
+    it('should test getValidUrl', () => {
+      expect(getValidUrl('http://example.com').toString()).toStrictEqual(
+        'http://example.com/',
+      );
+      expect(getValidUrl('https://example.com').toString()).toStrictEqual(
+        'https://example.com/',
+      );
+      expect(getValidUrl('https://exa%20mple.com')).toStrictEqual(null);
+      expect(getValidUrl('')).toStrictEqual(null);
+    });
+  });
+
   describe('isPrefixedFormattedHexString', () => {
     it('should return true for valid hex strings', () => {
       expect(isPrefixedFormattedHexString('0x1')).toStrictEqual(true);

app/scripts/lib/util.ts

@@ -1,24 +1,24 @@
+import urlLib from 'url';
+import { AccessList } from '@ethereumjs/tx';
 import BN from 'bn.js';
 import { memoize } from 'lodash';
-import { AccessList } from '@ethereumjs/tx';
-import { CHAIN_IDS, TEST_CHAINS } from '../../../shared/constants/network';
-
 import {
-  ENVIRONMENT_TYPE_POPUP,
-  ENVIRONMENT_TYPE_NOTIFICATION,
-  ENVIRONMENT_TYPE_FULLSCREEN,
   ENVIRONMENT_TYPE_BACKGROUND,
-  PLATFORM_FIREFOX,
-  PLATFORM_OPERA,
+  ENVIRONMENT_TYPE_FULLSCREEN,
+  ENVIRONMENT_TYPE_NOTIFICATION,
+  ENVIRONMENT_TYPE_POPUP,
+  PLATFORM_BRAVE,
   PLATFORM_CHROME,
   PLATFORM_EDGE,
-  PLATFORM_BRAVE,
+  PLATFORM_FIREFOX,
+  PLATFORM_OPERA,
 } from '../../../shared/constants/app';
-import { stripHexPrefix } from '../../../shared/modules/hexstring-utils';
+import { CHAIN_IDS, TEST_CHAINS } from '../../../shared/constants/network';
 import {
   TransactionEnvelopeType,
   TransactionMeta,
 } from '../../../shared/constants/transaction';
+import { stripHexPrefix } from '../../../shared/modules/hexstring-utils';
 
 /**
  * @see {@link getEnvironmentType}
@@ -143,13 +143,13 @@ function checkAlarmExists(alarmList: { name: string }[], alarmName: string) {
 }
 
 export {
-  getPlatform,
-  getEnvironmentType,
-  hexToBn,
   BnMultiplyByFraction,
   addHexPrefix,
-  getChainType,
   checkAlarmExists,
+  getChainType,
+  getEnvironmentType,
+  getPlatform,
+  hexToBn,
 };
 
 // Taken from https://stackoverflow.com/a/1349426/3696652
@@ -235,10 +235,43 @@ export function previousValueComparator<A>(
 }
 
 export function addUrlProtocolPrefix(urlString: string) {
-  if (!urlString.match(/(^http:\/\/)|(^https:\/\/)/u)) {
-    return `https://${urlString}`;
+  let trimmed = urlString.trim();
+
+  if (trimmed.length && !urlLib.parse(trimmed).protocol) {
+    trimmed = `https://${trimmed}`;
   }
-  return urlString;
+
+  if (getValidUrl(trimmed) !== null) {
+    return trimmed;
+  }
+
+  return null;
+}
+
+export function getValidUrl(urlString: string): URL | null {
+  try {
+    const url = new URL(urlString);
+
+    if (url.hostname.length === 0 || url.pathname.length === 0) {
+      return null;
+    }
+
+    if (url.hostname !== decodeURIComponent(url.hostname)) {
+      return null; // will happen if there's a %, a space, or other invalid character in the hostname
+    }
+
+    return url;
+  } catch (error) {
+    return null;
+  }
+}
+
+export function isWebUrl(urlString: string): boolean {
+  const url = getValidUrl(urlString);
+
+  return (
+    url !== null && (url.protocol === 'https:' || url.protocol === 'http:')
+  );
 }
 
 interface FormattedTransactionMeta {

jest.config.js

@@ -8,7 +8,7 @@ module.exports = {
     '<rootDir>/app/scripts/controllers/transactions/EtherscanRemoteTransactionSource.ts',
     '<rootDir>/app/scripts/controllers/transactions/IncomingTransactionHelper.ts',
     '<rootDir>/app/scripts/flask/**/*.js',
-    '<rootDir>/app/scripts/lib/**/*.js',
+    '<rootDir>/app/scripts/lib/**/*.(js|ts)',
     '<rootDir>/app/scripts/lib/createRPCMethodTrackingMiddleware.js',
     '<rootDir>/app/scripts/migrations/*.js',
     '<rootDir>/app/scripts/migrations/*.ts',
@@ -48,8 +48,7 @@ module.exports = {
     '<rootDir>/app/scripts/controllers/sign.test.ts',
     '<rootDir>/app/scripts/controllers/decrypt-message.test.ts',
     '<rootDir>/app/scripts/flask/**/*.test.js',
-    '<rootDir>/app/scripts/lib/**/*.test.js',
-    '<rootDir>/app/scripts/lib/**/*.test.ts',
+    '<rootDir>/app/scripts/lib/**/*.test.(js|ts)',
     '<rootDir>/app/scripts/lib/createRPCMethodTrackingMiddleware.test.js',
     '<rootDir>/app/scripts/migrations/*.test.(js|ts)',
     '<rootDir>/app/scripts/platforms/*.test.js',

package.json

@@ -362,7 +362,6 @@
     "single-call-balance-checker-abi": "^1.0.0",
     "unicode-confusables": "^0.1.1",
     "uuid": "^8.3.2",
-    "valid-url": "^1.0.9",
     "web3-stream-provider": "^4.0.0",
     "zxcvbn": "^4.4.2"
   },

test/data/mock-state.json

@@ -340,6 +340,8 @@
     "unapprovedTypedMessagesCount": 0,
     "useTokenDetection": true,
     "useCurrencyRateCheck": true,
+    "useNftDetection": true,
+    "openSeaEnabled": true,
     "advancedGasFee": {
       "maxBaseFee": "75",
       "priorityFee": "2"

ui/pages/keychains/reveal-seed.js

@@ -129,7 +129,7 @@ const RevealSeedPage = () => {
 
   const renderPasswordPromptContent = () => {
     return (
-      <form onSubmit={(event) => handleSubmit(event)}>
+      <form onSubmit={handleSubmit}>
         <Label htmlFor="password-box">{t('enterPasswordContinue')}</Label>
         <TextField
           inputProps={{