From ee2764b86c4111821dbe52e6027f7b3f396aa9e6 Mon Sep 17 00:00:00 2001
From: Chris Bobbe <cbobbe@zulip.com>
Date: Wed, 19 Aug 2020 15:16:10 -0700
Subject: [PATCH] url: Make `getFullUrl` use URL constructor.

This function relied on some hacky heuristics to give its output,
and if something was wrong with its input, the effect wasn't very
clear. Now,

- no more hacky heuristics -- the URL constructor (as polyfilled by
  `react-native-url-polyfill`) will apply appropriate rules to give
  the correct output, given an acceptable input, and
- if something is wrong with the input, we get a clear "INVALID_URL"
  error.

Note that this function will now throw errors in cases where it
didn't before. This is better than giving a (maybe subtly) wrong
output, but it does suggest we should do some stress-testing with
real data to be sure we weren't somehow taking advantage of the
implementation mistakes that were there before.

So, do that here. On CZO, I didn't encounter any INVALID_URL errors,
even in the following likely-looking places:

- Copying an in-message link to the clipboard -- both a relative
  upload URL and an absolute non-upload URL (Jitsi)
- Avatar URLs in several places
- Realm icon on the AuthScreen on CZO
- Both links on LegalScreen

The new implementation of `getFullUrl` is so simple that it'll be
more transparent to inline it at all call sites, which we'll do in
the next commit.
---
 src/utils/url.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/utils/url.js b/src/utils/url.js
index 3c52066c94a..6716129d7ab 100644
--- a/src/utils/url.js
+++ b/src/utils/url.js
@@ -46,9 +46,9 @@ export const tryParseUrl = (url: string, base?: string | URL): URL | void => {
  * @param url an absolute URL, or a relative URL on the realm.
  * @param realm the URL of the current realm.
  */
-// Uses some crude heuristics. TODO: Revisit this after or during #4081.
+// We'll be inlining this at all call sites very soon.
 export const getFullUrl = (url: string = '', realm: string): string =>
-  !url.startsWith('http') ? `${realm}${url.startsWith('/') ? '' : '/'}${url}` : url;
+  new URL(url, realm).toString();
 
 export const isUrlOnRealm = (url: string = '', realm: string): boolean =>
   url.startsWith('/') || url.startsWith(realm) || !/^(http|www.)/i.test(url);