addressing feedback

src/util.test.ts

@@ -1157,25 +1157,37 @@ describe('util', () => {
     });
   });
 
+  describe('addUrlProtocolPrefix', () => {
+    it('should return a URL with https:// prepended if input URL does not already have it', () => {
+      expect(util.addUrlProtocolPrefix(IFPS_GATEWAY)).toStrictEqual(
+        `https://${IFPS_GATEWAY}`,
+      );
+    });
+
+    it('should return a URL as is if https:// is already prepended', () => {
+      expect(util.addUrlProtocolPrefix(SOME_API)).toStrictEqual(SOME_API);
+    });
+  });
+
   describe('getIpfsCIDv1AndPath', () => {
     it('should return content identifier from default ipfs url format', () => {
       expect(
         util.getIpfsCIDv1AndPath(`${DEFAULT_IPFS_URL_FORMAT}${IPFS_CID_V0}`),
-      ).toStrictEqual({ cid: IPFS_CID_V1, path: '' });
+      ).toStrictEqual({ cid: IPFS_CID_V1, path: undefined });
     });
 
     it('should return content identifier from alternative ipfs url format', () => {
       expect(
         util.getIpfsCIDv1AndPath(
           `${ALTERNATIVE_IPFS_URL_FORMAT}${IPFS_CID_V0}`,
         ),
-      ).toStrictEqual({ cid: IPFS_CID_V1, path: '' });
+      ).toStrictEqual({ cid: IPFS_CID_V1, path: undefined });
     });
 
     it('should return unchanged content identifier if already v1', () => {
       expect(
         util.getIpfsCIDv1AndPath(`${DEFAULT_IPFS_URL_FORMAT}${IPFS_CID_V1}`),
-      ).toStrictEqual({ cid: IPFS_CID_V1, path: '' });
+      ).toStrictEqual({ cid: IPFS_CID_V1, path: undefined });
     });
 
     it('should return a path when url contains one', () => {

src/util.ts

@@ -770,11 +770,11 @@ export function validateMinimumIncrease(proposed: string, min: string) {
 }
 
 /**
- * Removes ipfs protocol prefix from ipfs-url.
+ * Removes IPFS protocol prefix from input string.
  *
- * @param ipfsUrl - ipfs url
- * @returns Ipfs content identifier and (possibly) path in a string
- * @throws will throw if the url passed is not ipfs.
+ * @param ipfsUrl - An IPFS url (e.g. ipfs://{content id})
+ * @returns IPFS content identifier and (possibly) path in a string
+ * @throws Will throw if the url passed is not IPFS.
  */
 export function removeIpfsProtocolPrefix(ipfsUrl: string) {
   if (ipfsUrl.startsWith('ipfs://ipfs/')) {
@@ -787,10 +787,10 @@ export function removeIpfsProtocolPrefix(ipfsUrl: string) {
 }
 
 /**
- * Extracts content identifier and path from ipfs url.
+ * Extracts content identifier and path from an input string.
  *
- * @param ipfsUrl - ipfs url
- * @returns Ipfs content identifier as string and path as string.
+ * @param ipfsUrl - An IPFS URL minus the IPFS protocol prefix
+ * @returns IFPS content identifier (cid) and sub path as string.
  * @throws Will throw if the url passed is not ipfs.
  */
 export function getIpfsCIDv1AndPath(
@@ -802,20 +802,21 @@ export function getIpfsCIDv1AndPath(
   // (CID is everything preceding first forward slash, path is everything after)
   const index = url.indexOf('/');
   const cid = index !== -1 ? url.substring(0, index) : url;
-  const path = index !== -1 ? url.substring(index) : '';
+  const path = index !== -1 ? url.substring(index) : undefined;
 
-  // we want to ensure that the CID is v1 (https://docs.ipfs.io/concepts/content-addressing/#identifier-formats)
+  // We want to ensure that the CID is v1 (https://docs.ipfs.io/concepts/content-addressing/#identifier-formats)
+  // for security and use with IPFS subdomains
   return {
     cid: CID.parse(cid).toV1().toString(),
     path,
   };
 }
 
 /**
- * Adds URL protocol prefix to url string if missing.
+ * Adds URL protocol prefix to input URL string if missing.
  *
- * @param urlString - Ipfs url.
- * @returns string.
+ * @param urlString - An IPFS URL.
+ * @returns A URL with a https:// prepended.
  */
 export function addUrlProtocolPrefix(urlString: string): string {
   if (!urlString.match(/(^http:\/\/)|(^https:\/\/)/u)) {
@@ -825,26 +826,23 @@ export function addUrlProtocolPrefix(urlString: string): string {
 }
 
 /**
- * Formats url correctly for use retrieving assets hosted on IPFS.
+ * Formats URL correctly for use retrieving assets hosted on IPFS.
  *
- * @param ipfsGateway - the user preferred ipfsGateway.
- * @param ipfsUrl - the ipfs url pointed at the asset.
- * @param subdomainSupported - boolean indicating whether the url should be formatted with subdomains or not
- * @returns string.
+ * @param ipfsGateway - The users preferred IPFS gateway (full URL or just host).
+ * @param ipfsUrl - The IFPS URL pointed at the asset.
+ * @param subdomainSupported - Boolean indicating whether the URL should be formatted with subdomains or not.
+ * @returns A formatted URL, with the user's preferred IPFS gateway and format (subdomain or not), pointing to an asset hosted on IPFS.
  */
 export function getFormattedIpfsUrl(
   ipfsGateway: string,
   ipfsUrl: string,
   subdomainSupported: boolean,
 ): string {
+  const { host, protocol, origin } = new URL(addUrlProtocolPrefix(ipfsGateway));
   if (subdomainSupported) {
-    const gatewayHost = new URL(addUrlProtocolPrefix(ipfsGateway)).host;
     const { cid, path } = getIpfsCIDv1AndPath(ipfsUrl);
-    return `https://${cid}.ipfs.${gatewayHost}${path}`;
+    return `${protocol}//${cid}.ipfs.${host}${path ?? ''}`;
   }
   const cidAndPath = removeIpfsProtocolPrefix(ipfsUrl);
-  const gateway = ipfsGateway.endsWith('/ipfs/')
-    ? ipfsGateway
-    : `${ipfsGateway}/ipfs/`;
-  return `${gateway}${cidAndPath}`;
+  return `${origin}/ipfs/${cidAndPath}`;
 }