Trim exported APIs from odsp-driver and odsp-urlresolver, and strengthen url checks

packages/drivers/odsp-driver/api-report/odsp-driver.api.md

@@ -97,9 +97,6 @@ export type FetchType = "blob" | "createBlob" | "createFile" | "joinSession" | "
 // @alpha (undocumented)
 export type FetchTypeInternal = FetchType | "cache";
 
-// @internal
-export function getApiRoot(origin: string): string;
-
 // @alpha
 export function getHashedDocumentId(driveId: string, itemId: string): Promise<string>;
 
@@ -193,16 +190,13 @@ export interface ISnapshotContentsWithProps extends ISnapshot {
 }
 
 // @internal
-export function isOdcOrigin(origin: string): boolean;
-
-// @internal
-export function isOdcUrl(url: string | URL): boolean;
+export function isOdcUrl(url: URL): boolean;
 
 // @internal
 export function isOdspResolvedUrl(resolvedUrl: IResolvedUrl): resolvedUrl is IOdspResolvedUrl;
 
 // @internal
-export function isSpoUrl(url: string): boolean;
+export function isSpoUrl(url: URL): boolean;
 
 // @alpha
 export const locatorQueryParamName = "nav";

packages/drivers/odsp-driver/package.json

@@ -178,6 +178,14 @@
 			},
 			"ClassDeclaration_OdspDriverUrlResolverForShareLink": {
 				"forwardCompat": false
+			},
+			"RemovedFunctionDeclaration_getApiRoot": {
+				"backCompat": false,
+				"forwardCompat": false
+			},
+			"RemovedFunctionDeclaration_isOdcOrigin": {
+				"backCompat": false,
+				"forwardCompat": false
 			}
 		}
 	}

packages/drivers/odsp-driver/src/createFile.ts

@@ -33,7 +33,6 @@ import {
 	INewFileInfo,
 	buildOdspShareLinkReqParams,
 	createCacheSnapshotKey,
-	getOrigin,
 	getWithRetryForTokenRefresh,
 } from "./odspUtils.js";
 import { pkgVersion as driverVersion } from "./packageVersion.js";
@@ -171,7 +170,7 @@ export async function createNewEmptyFluidFile(
 	const filePath = newFileInfo.filePath ? encodeURIComponent(`/${newFileInfo.filePath}`) : "";
 	// add .tmp extension to empty file (host is expected to rename)
 	const encodedFilename = encodeURIComponent(`${newFileInfo.filename}.tmp`);
-	const initialUrl = `${getApiRoot(getOrigin(newFileInfo.siteUrl))}/drives/${
+	const initialUrl = `${getApiRoot(new URL(newFileInfo.siteUrl))}/drives/${
 		newFileInfo.driveId
 	}/items/root:/${filePath}/${encodedFilename}:/content?@name.conflictBehavior=rename&select=id,name,parentReference`;
 
@@ -234,7 +233,7 @@ export async function createNewFluidFileFromSummary(
 	const filePath = newFileInfo.filePath ? encodeURIComponent(`/${newFileInfo.filePath}`) : "";
 	const encodedFilename = encodeURIComponent(newFileInfo.filename);
 	const baseUrl =
-		`${getApiRoot(getOrigin(newFileInfo.siteUrl))}/drives/${newFileInfo.driveId}/items/root:` +
+		`${getApiRoot(new URL(newFileInfo.siteUrl))}/drives/${newFileInfo.driveId}/items/root:` +
 		`${filePath}/${encodedFilename}`;
 
 	const containerSnapshot = convertSummaryIntoContainerSnapshot(createNewSummary);

packages/drivers/odsp-driver/src/createNewContainerOnExistingFile.ts

@@ -24,7 +24,7 @@ import { createOdspUrl } from "./createOdspUrl.js";
 import { EpochTracker } from "./epochTracker.js";
 import { OdspDriverUrlResolver } from "./odspDriverUrlResolver.js";
 import { getApiRoot } from "./odspUrlHelper.js";
-import { IExistingFileInfo, createCacheSnapshotKey, getOrigin } from "./odspUtils.js";
+import { IExistingFileInfo, createCacheSnapshotKey } from "./odspUtils.js";
 
 /**
  * Creates a new Fluid container on an existing file.
@@ -52,7 +52,7 @@ export async function createNewContainerOnExistingFile(
 		throw new UsageError("createNewSummary must exist to create a new container");
 	}
 
-	const baseUrl = `${getApiRoot(getOrigin(fileInfo.siteUrl))}/drives/${fileInfo.driveId}/items/${
+	const baseUrl = `${getApiRoot(new URL(fileInfo.siteUrl))}/drives/${fileInfo.driveId}/items/${
 		fileInfo.itemId
 	}`;
 

packages/drivers/odsp-driver/src/index.ts

@@ -18,7 +18,7 @@ export {
 export { checkUrl } from "./checkUrl.js";
 export { createOdspUrl } from "./createOdspUrl.js";
 export { getHashedDocumentId, ISnapshotContents } from "./odspPublicUtils.js";
-export { getApiRoot, getOdspUrlParts, isOdcOrigin, isOdcUrl, isSpoUrl } from "./odspUrlHelper.js";
+export { getOdspUrlParts, isOdcUrl, isSpoUrl } from "./odspUrlHelper.js";
 
 // prefetch latest snapshot before container load
 export { prefetchLatestSnapshot } from "./prefetchLatestSnapshot.js";

packages/drivers/odsp-driver/src/odspDocumentService.ts

@@ -34,7 +34,7 @@ import { IOdspCache } from "./odspCache.js";
 import type { OdspDelayLoadedDeltaStream } from "./odspDelayLoadedDeltaStream.js";
 import { OdspDeltaStorageService, OdspDeltaStorageWithCache } from "./odspDeltaStorageService.js";
 import { OdspDocumentStorageService } from "./odspDocumentStorageManager.js";
-import { isOdcOrigin } from "./odspUrlHelper.js";
+import { hasOdcOrigin } from "./odspUrlHelper.js";
 import { getOdspResolvedUrl } from "./odspUtils.js";
 import { OpsCache } from "./opsCaching.js";
 import { RetryErrorsStorageAdapter } from "./retryErrorsStorageAdapter.js";
@@ -143,9 +143,7 @@ export class OdspDocumentService
 			logger,
 			properties: {
 				all: {
-					odc: isOdcOrigin(
-						new URL(this.odspResolvedUrl.endpoints.snapshotStorageUrl).origin,
-					),
+					odc: hasOdcOrigin(new URL(this.odspResolvedUrl.endpoints.snapshotStorageUrl)),
 				},
 			},
 		});

packages/drivers/odsp-driver/src/odspDriverUrlResolver.ts

@@ -27,9 +27,8 @@ function getUrlBase(
 	itemId: string,
 	fileVersion?: string,
 ): string {
-	const siteOrigin = new URL(siteUrl).origin;
 	const version = fileVersion ? `versions/${fileVersion}/` : "";
-	return `${getApiRoot(siteOrigin)}/drives/${driveId}/items/${itemId}/${version}`;
+	return `${getApiRoot(new URL(siteUrl))}/drives/${driveId}/items/${itemId}/${version}`;
 }
 
 function getSnapshotUrl(

packages/drivers/odsp-driver/src/odspUrlHelper.ts

@@ -8,72 +8,70 @@ import { IOdspUrlParts } from "@fluidframework/odsp-driver-definitions/internal"
 // Centralized store for all ODC/SPO logic
 
 /**
- * Checks whether or not the given URL origin is an ODC origin
- * @param origin - The URL origin to check
+ * Checks whether or not the given URL has an ODC origin
+ * @param url - The URL to check
  * @internal
  */
-export function isOdcOrigin(origin: string): boolean {
+export function hasOdcOrigin(url: URL): boolean {
 	return (
 		// Primary API endpoint and several test endpoints
-		origin.includes("onedrive.com") ||
+		url.origin.endsWith("onedrive.com") ||
 		// *storage.live.com hostnames
-		origin.includes("storage.live.com") ||
+		url.origin.endsWith("storage.live.com") ||
 		// live-int
-		origin.includes("storage.live-int.com") ||
+		url.origin.endsWith("storage.live-int.com") ||
 		// Test endpoints
-		origin.includes("onedrive-tst.com")
+		url.origin.endsWith("onedrive-tst.com")
 	);
 }
 
 /**
  * Gets the correct API root for the given ODSP url, e.g. 'https://foo-my.sharepoint.com/_api/v2.1'
- * @param origin - The URL origin
+ * @param url - The URL
  * @internal
  */
-export function getApiRoot(origin: string): string {
+export function getApiRoot(url: URL): string {
 	let prefix = "_api/";
-	if (isOdcOrigin(origin)) {
+	if (hasOdcOrigin(url)) {
 		prefix = "";
 	}
 
-	return `${origin}/${prefix}v2.1`;
+	return `${url.origin}/${prefix}v2.1`;
 }
 
 /**
  * Whether or not the given URL is a valid SPO/ODB URL
  * @param url - The URL to check
  * @internal
  */
-export function isSpoUrl(url: string): boolean {
-	const urlLower = url.toLowerCase();
-
+export function isSpoUrl(url: URL): boolean {
 	// Format: foo.sharepoint.com/_api/v2.1./drives/bar/items/baz and foo.sharepoint-df.com/...
-	const spoRegex = /(.*\.sharepoint(-df)*\.com)\/_api\/v2.1\/drives\/([^/]*)\/items\/([^/]*)/;
-	return spoRegex.test(urlLower);
+	const hostRegex = /\.sharepoint(?:-df)?\.com$/;
+	const pathRegex = /^\/_api\/v2\.1\/drives\/[^/]+\/items\/[^/]+/;
+
+	return hostRegex.test(url.host.toLowerCase()) && pathRegex.test(url.pathname.toLowerCase());
 }
 
 /**
  * Whether or not the given URL is a valid ODC URL
  * @param url - The URL to check
  * @internal
  */
-export function isOdcUrl(url: string | URL): boolean {
-	const urlObj = typeof url === "string" ? new URL(url) : url;
-
-	if (!isOdcOrigin(urlObj.origin)) {
+export function isOdcUrl(url: URL): boolean {
+	if (!hasOdcOrigin(url)) {
 		return false;
 	}
 
-	const path = urlObj.pathname.toLowerCase();
+	const path = url.pathname.toLowerCase();
 
 	// Splitting the regexes so we don't have regex soup
 	// Format: /v2.1/drive/items/ABC123!123 and /v2.1/drives/ABC123/items/ABC123!123
-	const odcRegex = /\/v2.1\/(drive|drives\/[^/]+)\/items\/([\da-z]+)!(\d+)/;
+	const odcRegex = /^\/v2\.1\/(?:drive|drives\/[^/]+)\/items\/[\dA-Za-z]+!\d+/;
 
 	// Format: /v2.1/drives('ABC123')/items('ABC123!123')
-	const odcODataRegex = /\/v2.1\/drives\('[^/]+'\)\/items\('[\da-z]+!\d+'\)/;
+	const odcODataRegex = /^\/v2\.1\/drives\('[^/]+'\)\/items\('[\dA-Za-z]+!\d+'\)/;
 
-	return !!(odcRegex.exec(path) ?? odcODataRegex.exec(path));
+	return odcRegex.test(path) || odcODataRegex.test(path);
 }
 
 /**
@@ -89,7 +87,7 @@ export async function getOdspUrlParts(url: URL): Promise<IOdspUrlParts | undefin
 	// Pick a regex based on the hostname
 	// TODO This will only support ODC using api.onedrive.com, update to handle the future (share links etc)
 	let joinSessionMatch: RegExpExecArray | null;
-	if (isOdcOrigin(url.origin)) {
+	if (hasOdcOrigin(url)) {
 		// Capture groups:
 		// 0: match
 		// 1: origin
@@ -118,7 +116,7 @@ export async function getOdspUrlParts(url: URL): Promise<IOdspUrlParts | undefin
 
 		return { siteUrl: `${url.origin}${url.pathname}`, driveId, itemId };
 	} else {
-		joinSessionMatch = /(.*)\/_api\/v2.1\/drives\/([^/]*)\/items\/([^/]*)(.*)/.exec(pathname);
+		joinSessionMatch = /(.*)\/_api\/v2\.1\/drives\/([^/]*)\/items\/([^/]*)(.*)/.exec(pathname);
 
 		if (joinSessionMatch === null) {
 			return undefined;

packages/drivers/odsp-driver/src/odspUtils.ts

@@ -52,11 +52,6 @@ import { pkgVersion as driverVersion } from "./packageVersion.js";
 
 export const getWithRetryForTokenRefreshRepeat = "getWithRetryForTokenRefreshRepeat";
 
-/**
- * Parse the given url and return the origin (host name)
- */
-export const getOrigin = (url: string): string => new URL(url).origin;
-
 /**
  * @alpha
  */