From 41fb77c8cdc99fa5db5c3acc93d1b4f8278616bd Mon Sep 17 00:00:00 2001 From: Romuald Brillout Date: Wed, 22 Sep 2021 12:39:42 +0200 Subject: [PATCH] fix: correctly handle base URL that contains a URL origin (fix #149) --- vite-plugin-ssr/node/baseUrlHandling.ts | 133 ++++++++++++++------- vite-plugin-ssr/node/renderPage.ts | 51 ++++---- vite-plugin-ssr/node/ssrEnv.ts | 6 +- vite-plugin-ssr/shared/utils/getFileUrl.ts | 15 ++- vite-plugin-ssr/shared/utils/parseUrl.ts | 69 +++++++++-- 5 files changed, 192 insertions(+), 82 deletions(-) diff --git a/vite-plugin-ssr/node/baseUrlHandling.ts b/vite-plugin-ssr/node/baseUrlHandling.ts index f71e32c0c57..04b109a1d7a 100644 --- a/vite-plugin-ssr/node/baseUrlHandling.ts +++ b/vite-plugin-ssr/node/baseUrlHandling.ts @@ -1,59 +1,108 @@ -import { assert, assertUsage, slice } from '../shared/utils' +import { addUrlOrigin, assert, assertUsage, handleUrlOrigin, slice } from '../shared/utils' import { getSsrEnv } from './ssrEnv' +export { analyzeBaseUrl } export { prependBaseUrl } -export { removeBaseUrl } -export { startsWithBaseUrl } export { assertBaseUrl } -// Possible values: +// Possible Base URL values: // `base: '/some-nested-path/'` -// `base: 'https://another-origin.example.org/'` -// `base: './'` - -function prependBaseUrl(url: string): string { - let baseUrl = getNormalizedBaseUrl() - if (baseUrl === '/') return url - assert(baseUrl.endsWith('/')) - baseUrl = slice(baseUrl, 0, -1) - assert(!baseUrl.endsWith('/')) - assert(url.startsWith('/')) - return `${baseUrl}${url}` +// `base: 'http://another-origin.example.org/'` +// `base: './'` (WIP: not supported yet) +function assertBaseUrl(baseUrl: string, userErrorMessagePrefix?: string) { + if (!userErrorMessagePrefix) { + assert(baseUrl.startsWith('/') || baseUrl.startsWith('http')) + return + } + assertUsage( + baseUrl.startsWith('/') || baseUrl.startsWith('http') || baseUrl.startsWith('./'), + userErrorMessagePrefix + 'Wrong `base` value `' + baseUrl + '`; `base` should start with `/`, `./`, or `http`.' + ) + assertUsage( + !baseUrl.startsWith('./'), + 'Relative Base URLs are not supported yet (`baseUrl` that starts with `./`). Open a new GitHub ticket so we can discuss adding support for your use case.' + ) } -function startsWithBaseUrl(url: string): boolean { - const baseUrl = getNormalizedBaseUrl() - if (baseUrl === '/') return true - return url.startsWith(baseUrl) -} -function removeBaseUrl(url: string): string { - const baseUrl = getNormalizedBaseUrl() - if (baseUrl === '/') return url - assert(startsWithBaseUrl(url)) +function analyzeBaseUrl(url_: string) { + // Unmutable + const urlPristine = url_ + // Mutable + let url = url_ + + assert(url.startsWith('/') || url.startsWith('http')) + const baseUrl = getBaseUrl() + assert(baseUrl.startsWith('/') || baseUrl.startsWith('http')) + + if (baseUrl === '/') { + return { urlWithoutBaseUrl: urlPristine, hasBaseUrl: true } + } + + const { urlWithoutOrigin, urlOrigin } = handleUrlOrigin(url) + let urlOriginHasBeenRemoved = false + { + const baseUrlOrigin = handleUrlOrigin(baseUrl).urlOrigin + const baseUrlHasOrigin = baseUrlOrigin !== null + let urlHasOrigin = urlOrigin !== null + assertUsage( + !baseUrlHasOrigin || urlHasOrigin, + `You provided a \`baseUrl\` (\`${baseUrl}\`) that contains a URL origin (\`${baseUrlOrigin!}\`) but the \`pageContext.url\` (\`${url}\`) you provided in your server middleware (\`const renderPage = createPageRenderer(/*...*/); renderPage(pageContext);\`) does not contain a URL origin. Either remove the URL origin from your \`baseUrl\` or make sure to always provide the URL origin in \`pageContext.url\`.` + ) + if (urlHasOrigin && !baseUrlHasOrigin) { + urlOriginHasBeenRemoved = true + url = urlWithoutOrigin + urlHasOrigin = false + } + assert(urlHasOrigin === baseUrlHasOrigin) + } + + if (!url.startsWith(baseUrl)) { + return { urlWithoutBaseUrl: urlPristine, hasBaseUrl: false } + } + assert(url.startsWith('/') || url.startsWith('http')) url = url.slice(baseUrl.length) + /* url can actually start with `httpsome-pathname` + assert(!url.startsWith('http')) + */ + /* `handleUrlOrigin('some-pathname-without-leading-slash')` fails + assert((handleUrlOrigin(url).urlOrigin===null)) + */ if (!url.startsWith('/')) url = '/' + url - return url -} + assert(url.startsWith('/')) -function getNormalizedBaseUrl(): string { - let { baseUrl } = getSsrEnv() - baseUrl = normalizeBaseUrl(baseUrl) - return baseUrl + if (urlOriginHasBeenRemoved) { + assert(urlOrigin !== null) + assert(urlOrigin.startsWith('http')) + assert(url.startsWith('/')) + url = addUrlOrigin(url, urlOrigin) + assert(url.startsWith('http')) + } + + return { urlWithoutBaseUrl: url, hasBaseUrl: true } } -function normalizeBaseUrl(baseUrl: string): string { - if (!baseUrl) baseUrl = '/' - if (!baseUrl.endsWith('/')) baseUrl = `${baseUrl}/` - if (!baseUrl.startsWith('/') && !baseUrl.startsWith('http') && !baseUrl.startsWith('./')) baseUrl = `/${baseUrl}` - assert(baseUrl.startsWith('/') || baseUrl.startsWith('http') || baseUrl.startsWith('./')) - assert(baseUrl.endsWith('/')) - return baseUrl +function prependBaseUrl(url: string): string { + let baseUrl = getBaseUrl() + + // Probably safer to remove the origin; `prependBaseUrl()` is used when injecting static assets in HTML; + // origin is useless in static asset URLs, while the origin causes trouble upon `https`/`http` mismatch. + baseUrl = handleUrlOrigin(baseUrl).urlWithoutOrigin + + if (baseUrl === '/') return url + + if (baseUrl.endsWith('/')) { + baseUrl = slice(baseUrl, 0, -1) + } + + // We can and should expect `baseUrl` to not contain `/` doublets. (We cannot expect url to not contain `/` doublets.) + assert(!baseUrl.endsWith('/')) + assert(url.startsWith('/')) + return `${baseUrl}${url}` } -function assertBaseUrl(baseUrl: string, errorMessagePrefix = '') { - assertUsage( - baseUrl.startsWith('/') || baseUrl.startsWith('http') || baseUrl.startsWith('./'), - errorMessagePrefix + 'Wrong `base` value `' + baseUrl + '`; `base` should start with `/`, `./`, or `http`.' - ) +function getBaseUrl(): string { + const { baseUrl } = getSsrEnv() + assertBaseUrl(baseUrl) + return baseUrl } diff --git a/vite-plugin-ssr/node/renderPage.ts b/vite-plugin-ssr/node/renderPage.ts index 04c2a771a5c..0306bc09217 100644 --- a/vite-plugin-ssr/node/renderPage.ts +++ b/vite-plugin-ssr/node/renderPage.ts @@ -12,10 +12,8 @@ import { cast, assertWarning, hasProp, - isPageContextUrl, - removePageContextUrlSuffix, + handlePageContextRequestSuffix, getUrlPathname, - getUrlFull, isPlainObject, isObject, getUrlParsed, @@ -24,9 +22,10 @@ import { PromiseType, compareString, assertExports, - stringifyStringArray + stringifyStringArray, + handleUrlOrigin } from '../shared/utils' -import { removeBaseUrl, startsWithBaseUrl } from './baseUrlHandling' +import { analyzeBaseUrl } from './baseUrlHandling' import { getPageAssets, PageAssets } from './html/injectAssets' import { loadPageView } from '../shared/loadPageView' import { sortPageContext } from '../shared/sortPageContext' @@ -815,18 +814,25 @@ function assertArguments(...args: unknown[]) { ) assertUsage( typeof pageContext.url === 'string', - '`renderPage(pageContext)`: `pageContext.url` should be a string but we got `typeof pageContext.url === "' + + '`renderPage(pageContext)`: `pageContext.url` should be a string but `typeof pageContext.url === "' + typeof pageContext.url + '"`.' ) + assertUsage( + pageContext.url.startsWith('/') || pageContext.url.startsWith('http'), + '`renderPage(pageContext)`: `pageContext.url` should start with `/` (e.g. `/product/42`) or `http` (e.g. `http://example.org/product/42`) but `pageContext.url === "' + + pageContext.url + + '"`.' + ) try { - removeOrigin(pageContext.url) + const { url } = pageContext + const urlWithOrigin = url.startsWith('http') ? url : 'http://fake-origin.example.org' + url + // `new URL()` conveniently throws if URL is not an URL + new URL(urlWithOrigin) } catch (err) { assertUsage( false, - '`renderPage(pageContext)`: argument `pageContext.url` should be a URL but we got `url==="' + - pageContext.url + - '"`.' + '`renderPage(pageContext)`: `pageContext.url` should be a URL but `pageContext.url==="' + pageContext.url + '"`.' ) } const len = args.length @@ -938,11 +944,6 @@ function handleError(err: unknown) { console.error(errStr) } -function removeOrigin(url: string): string { - const urlFull = getUrlFull(url) - return urlFull -} - type PageContextUrls = { urlNormalized: string; urlPathname: string; urlParsed: UrlParsed } function analyzeUrl(url: string): { @@ -950,19 +951,19 @@ function analyzeUrl(url: string): { isPageContextRequest: boolean hasBaseUrl: boolean } { - const isPageContextRequest = isPageContextUrl(url) - if (isPageContextRequest) { - url = removePageContextUrlSuffix(url) - } - url = removeOrigin(url) - assert(url.startsWith('/')) + assert(url.startsWith('/') || url.startsWith('http')) - const hasBaseUrl = startsWithBaseUrl(url) - if (hasBaseUrl) { - url = removeBaseUrl(url) - } + const { urlWithoutPageContextRequestSuffix, isPageContextRequest } = handlePageContextRequestSuffix(url) + url = urlWithoutPageContextRequestSuffix + + const { urlWithoutBaseUrl, hasBaseUrl } = analyzeBaseUrl(url) + url = urlWithoutBaseUrl + + url = handleUrlOrigin(url).urlWithoutOrigin + assert(url.startsWith('/')) const urlNormalized = url + assert(urlNormalized.startsWith('/')) return { urlNormalized, isPageContextRequest, hasBaseUrl } } diff --git a/vite-plugin-ssr/node/ssrEnv.ts b/vite-plugin-ssr/node/ssrEnv.ts index 4379e92e045..3ff133be247 100644 --- a/vite-plugin-ssr/node/ssrEnv.ts +++ b/vite-plugin-ssr/node/ssrEnv.ts @@ -1,4 +1,5 @@ import { ViteDevServer } from 'vite' +import { assertBaseUrl } from './baseUrlHandling' export { setSsrEnv } export { getSsrEnv } @@ -19,10 +20,13 @@ type SsrEnv = } function getSsrEnv(): SsrEnv { - return global.__vite_ssr_plugin + const ssrEnv = global.__vite_ssr_plugin + assertBaseUrl(ssrEnv.baseUrl) + return ssrEnv } function setSsrEnv(ssrEnv: SsrEnv) { + assertBaseUrl(ssrEnv.baseUrl) global.__vite_ssr_plugin = ssrEnv } diff --git a/vite-plugin-ssr/shared/utils/getFileUrl.ts b/vite-plugin-ssr/shared/utils/getFileUrl.ts index 32de3e71d79..4e0bbe72cdb 100644 --- a/vite-plugin-ssr/shared/utils/getFileUrl.ts +++ b/vite-plugin-ssr/shared/utils/getFileUrl.ts @@ -4,8 +4,7 @@ import { slice } from './slice' const pageContextUrlSuffix = '.pageContext.json' export { getFileUrl } -export { isPageContextUrl } -export { removePageContextUrlSuffix } +export { handlePageContextRequestSuffix } /** (`/`, `.html`) -> `/index.html` @@ -43,12 +42,18 @@ function getFileUrl( return `${pathnameModified}${fileExtension}${searchString}${hashString}` } -function isPageContextUrl(url: string): boolean { +function handlePageContextRequestSuffix(url: string): { + urlWithoutPageContextRequestSuffix: string + isPageContextRequest: boolean +} { const urlPathname = getUrlPathname(url) - return urlPathname.endsWith(pageContextUrlSuffix) + if (!urlPathname.endsWith(pageContextUrlSuffix)) { + return { urlWithoutPageContextRequestSuffix: url, isPageContextRequest: false } + } + return { urlWithoutPageContextRequestSuffix: removePageContextUrlSuffix(url), isPageContextRequest: true } } + function removePageContextUrlSuffix(url: string): string { - assert(isPageContextUrl(url), { url }) let { origin, pathname, searchString, hashString } = getUrlParts(url) assert(url === `${origin}${pathname}${searchString}${hashString}`, { url }) assert(pathname.endsWith(pageContextUrlSuffix), { url }) diff --git a/vite-plugin-ssr/shared/utils/parseUrl.ts b/vite-plugin-ssr/shared/utils/parseUrl.ts index 581b3c87b34..7c9b26d9617 100644 --- a/vite-plugin-ssr/shared/utils/parseUrl.ts +++ b/vite-plugin-ssr/shared/utils/parseUrl.ts @@ -1,4 +1,8 @@ -import { assert } from '.' +import { assert } from './assert' +import { slice } from './slice' + +export { handleUrlOrigin } +export { addUrlOrigin } export { getUrlFull } export { getUrlPathname } @@ -7,18 +11,37 @@ export { getUrlParts } export { getUrlFullWithoutHash } export type { UrlParsed } +function handleUrlOrigin(url: string): { urlWithoutOrigin: string; urlOrigin: null | string } { + assert(url.startsWith('/') || url.startsWith('http')) + if (url.startsWith('/')) { + return { urlWithoutOrigin: url, urlOrigin: null } + } else { + const urlOrigin = parseWithNewUrl(url).origin + assert(urlOrigin !== '', { url }) + assert(urlOrigin.startsWith('http'), { url }) + assert(url.startsWith(urlOrigin), { url }) + const urlWithoutOrigin = url.slice(urlOrigin.length) + assert(`${urlOrigin}${urlWithoutOrigin}` === url, { url }) + assert(urlWithoutOrigin.startsWith('/'), { url }) + return { urlWithoutOrigin, urlOrigin } + } +} +function addUrlOrigin(url: string, urlOrigin: string): string { + assert(urlOrigin.startsWith('http'), { url, urlOrigin }) + if (urlOrigin.endsWith('/')) { + urlOrigin = slice(urlOrigin, 0, -1) + } + assert(!urlOrigin.endsWith('/'), { url, urlOrigin }) + assert(url.startsWith('/'), { url, urlOrigin }) + return `${urlOrigin}${url}` +} + /** Returns `${pathname}${search}${hash}`. (Basically removes the origin.) */ function getUrlFull(url?: string): string { - // TODO url = retrieveUrl(url) - const { origin } = parseWithNewUrl(url) - assert(url.startsWith(origin), { url }) - const urlFull = url.slice(origin.length) - assert(`${origin}${urlFull}` === url, { url }) - assert(urlFull.startsWith('/'), { url }) - return urlFull + return handleUrlOrigin(url).urlWithoutOrigin } /** @@ -94,7 +117,35 @@ function parseWithNewUrl(url: string) { return { origin, pathname } } catch (err) { assert(url.startsWith('/'), { url }) - const { pathname } = new URL('https://fake-origin.example.org' + url) + const { pathname } = new URL('http://fake-origin.example.org' + url) return { origin: '', pathname } } } + +/* Tempting to also apply `cleanUrl()` on `pageContext.urlNormalized` but AFAICT no one needs this; `pageContext.urlParsed` is enough. + * +function cleanUrl(url: string): string { + return getUrlFromParsed(getUrlParsed(url)) +} + +function getUrlFromParsed(urlParsed: UrlParsed): string { + const { origin, pathname, search, hash } = urlParsed + + const searchParams = new URLSearchParams('') + assert(Array.from(searchParams.keys()).length === 0) + Object.entries(search || {}).forEach(([key, val]) => { + searchParams.set(key, val) + }) + const searchString = searchParams.toString() + + assert(hash === null || !hash.startsWith('#')) + const hashString = hash === null ? '' : '#' + hash + + assert(origin === '' || origin.startsWith('http')) + assert(pathname.startsWith('/')) + assert(searchString === '' || searchString.startsWith('?')) + assert(hashString === '' || hashString.startsWith('#')) + return `${origin}${pathname}${searchString}${hashString}` +} +* +*/