fix(servers): prevent UI crash when chaning Server with variables

Closes #7525
swagger-api · Nov 4, 2021 · fb7e98a · fb7e98a
1 parent 62031f3
commit fb7e98a
Show file tree

Hide file tree

Showing 5 changed files with 105 additions and 13 deletions.
diff --git a/src/core/components/info.jsx b/src/core/components/info.jsx
@@ -2,7 +2,7 @@ import React from "react"
 import PropTypes from "prop-types"
 import ImPropTypes from "react-immutable-proptypes"
 import { sanitizeUrl } from "core/utils"
-import { buildUrl } from "core/utils/url"
+import { safeBuildUrl } from "core/utils/url"
 
 
 export class InfoBasePath extends React.Component {
@@ -35,7 +35,7 @@ class Contact extends React.Component {
   render(){
     let { data, getComponent, selectedServer, url: specUrl} = this.props
     let name = data.get("name") || "the developer"
-    let url = buildUrl(data.get("url"), specUrl, {selectedServer})
+    let url = safeBuildUrl(data.get("url"), specUrl, {selectedServer})
     let email = data.get("email")
 
     const Link = getComponent("Link")
@@ -66,8 +66,8 @@ class License extends React.Component {
     let { license, getComponent, selectedServer, url: specUrl } = this.props
 
     const Link = getComponent("Link")
-    let name = license.get("name") || "License"  
-    let url = buildUrl(license.get("url"), specUrl, {selectedServer})
+    let name = license.get("name") || "License"
+    let url = safeBuildUrl(license.get("url"), specUrl, {selectedServer})
 
     return (
       <div className="info__license">
@@ -113,11 +113,11 @@ export default class Info extends React.Component {
     let version = info.get("version")
     let description = info.get("description")
     let title = info.get("title")
-    let termsOfServiceUrl = buildUrl(info.get("termsOfService"), specUrl, {selectedServer})
+    let termsOfServiceUrl = safeBuildUrl(info.get("termsOfService"), specUrl, {selectedServer})
     let contact = info.get("contact")
     let license = info.get("license")
     let rawExternalDocsUrl = externalDocs && externalDocs.get("url")
-    let externalDocsUrl = buildUrl(rawExternalDocsUrl, specUrl, {selectedServer})
+    let externalDocsUrl = safeBuildUrl(rawExternalDocsUrl, specUrl, {selectedServer})
     let externalDocsDescription = externalDocs && externalDocs.get("description")
 
     const Markdown = getComponent("Markdown", true)

diff --git a/src/core/components/operation-tag.jsx b/src/core/components/operation-tag.jsx
@@ -3,7 +3,7 @@ import PropTypes from "prop-types"
 import ImPropTypes from "react-immutable-proptypes"
 import Im from "immutable"
 import { createDeepLinkPath, escapeDeepLinkPath, sanitizeUrl } from "core/utils"
-import { buildUrl } from "core/utils/url"
+import { safeBuildUrl } from "core/utils/url"
 import { isFunc } from "core/utils"
 
 export default class OperationTag extends React.Component {
@@ -59,7 +59,7 @@ export default class OperationTag extends React.Component {
     let rawTagExternalDocsUrl = tagObj.getIn(["tagDetails", "externalDocs", "url"])
     let tagExternalDocsUrl
     if (isFunc(oas3Selectors) && isFunc(oas3Selectors.selectedServer)) {
-      tagExternalDocsUrl = buildUrl( rawTagExternalDocsUrl, specUrl, { selectedServer: oas3Selectors.selectedServer() } )
+      tagExternalDocsUrl = safeBuildUrl( rawTagExternalDocsUrl, specUrl, { selectedServer: oas3Selectors.selectedServer() } )
     } else {
       tagExternalDocsUrl = rawTagExternalDocsUrl
     }

diff --git a/src/core/components/operation.jsx b/src/core/components/operation.jsx
@@ -2,7 +2,7 @@ import React, { PureComponent } from "react"
 import PropTypes from "prop-types"
 import { getList } from "core/utils"
 import { getExtensions, sanitizeUrl, escapeDeepLinkPath } from "core/utils"
-import { buildUrl } from "core/utils/url"
+import { safeBuildUrl } from "core/utils/url"
 import { Iterable, List } from "immutable"
 import ImPropTypes from "react-immutable-proptypes"
 
@@ -82,7 +82,7 @@ export default class Operation extends PureComponent {
       schemes
     } = op
 
-    const externalDocsUrl = externalDocs ? buildUrl(externalDocs.url, specSelectors.url(), { selectedServer: oas3Selectors.selectedServer() }) : ""
+    const externalDocsUrl = externalDocs ? safeBuildUrl(externalDocs.url, specSelectors.url(), { selectedServer: oas3Selectors.selectedServer() }) : ""
     let operation = operationProps.getIn(["op"])
     let responses = operation.get("responses")
     let parameters = getList(operation, ["parameters"])

diff --git a/src/core/utils/url.js b/src/core/utils/url.js
@@ -12,11 +12,11 @@ export function buildBaseUrl(selectedServer, specUrl) {
   if (!selectedServer) return specUrl
   if (isAbsoluteUrl(selectedServer)) return addProtocol(selectedServer)
 
-  return new URL(selectedServer, specUrl).href    
+  return new URL(selectedServer, specUrl).href
 }
 
 export function buildUrl(url, specUrl, { selectedServer="" } = {}) {
-  if (!url) return
+  if (!url) return undefined
   if (isAbsoluteUrl(url)) return url
 
   const baseUrl = buildBaseUrl(selectedServer, specUrl)
@@ -25,3 +25,15 @@ export function buildUrl(url, specUrl, { selectedServer="" } = {}) {
   }
   return new URL(url, baseUrl).href
 }
+
+/**
+ * Safe version of buildUrl function. `selectedServer` can contain server variables
+ * which can fail the URL resolution.
+ */
+export function safeBuildUrl(url, specUrl, { selectedServer="" } = {}) {
+  try {
+    return buildUrl(url, specUrl, { selectedServer })
+  } catch {
+    return undefined
+  }
+}
diff --git a/test/unit/core/utils.js b/test/unit/core/utils.js
@@ -1,4 +1,3 @@
-
 import { Map, fromJS } from "immutable"
 import {
   mapToList,
@@ -36,6 +35,7 @@ import {
   isAbsoluteUrl,
   buildBaseUrl,
   buildUrl,
+  safeBuildUrl,
 } from "core/utils/url"
 
 import win from "core/window"
@@ -1445,6 +1445,7 @@ describe("utils", () => {
     const absoluteServerUrl = "https://server-example.com/base-path/path"
     const serverUrlRelativeToBase = "server-example/base-path/path"
     const serverUrlRelativeToHost = "/server-example/base-path/path"
+    const serverUrlWithVariables = "https://api.example.com:{port}/{basePath}"
 
     const specUrlAsInvalidUrl = "./examples/test.yaml"
     const specUrlOas2NonUrlString = "an allowed OAS2 TermsOfService description string"
@@ -1488,6 +1489,85 @@ describe("utils", () => {
     it("build relative url when no servers defined AND specUrl is OAS2 non-url string", () => {
       expect(buildUrl(urlRelativeToHost, specUrlOas2NonUrlString, { selectedServer: noServerSelected })).toBe("http://localhost/relative-url/base-path/path")
     })
+
+    it("throws error when server url contains non-transcluded server variables", () => {
+      const buildUrlThunk = () => buildUrl(urlRelativeToHost, specUrl, { selectedServer: serverUrlWithVariables })
+
+      expect(buildUrlThunk).toThrow(/^Invalid base URL/)
+    })
+  })
+
+  describe("safeBuildUrl", () => {
+    const { location } = window
+    beforeAll(() => {
+      delete window.location
+      window.location = {
+        href: "http://localhost/",
+      }
+    })
+    afterAll(() => {
+      window.location = location
+    })
+
+    const specUrl = "https://petstore.swagger.io/v2/swagger.json"
+
+    const noUrl = ""
+    const absoluteUrl = "https://example.com/base-path/path"
+    const urlRelativeToBase = "relative-url/base-path/path"
+    const urlRelativeToHost = "/relative-url/base-path/path"
+
+    const noServerSelected = ""
+    const absoluteServerUrl = "https://server-example.com/base-path/path"
+    const serverUrlRelativeToBase = "server-example/base-path/path"
+    const serverUrlRelativeToHost = "/server-example/base-path/path"
+    const serverUrlWithVariables = "https://api.example.com:{port}/{basePath}"
+
+    const specUrlAsInvalidUrl = "./examples/test.yaml"
+    const specUrlOas2NonUrlString = "an allowed OAS2 TermsOfService description string"
+
+    it("build no url", () => {
+      expect(safeBuildUrl(noUrl, specUrl, { selectedServer: absoluteServerUrl })).toBe(undefined)
+      expect(safeBuildUrl(noUrl, specUrl, { selectedServer: serverUrlRelativeToBase })).toBe(undefined)
+      expect(safeBuildUrl(noUrl, specUrl, { selectedServer: serverUrlRelativeToHost })).toBe(undefined)
+    })
+
+    it("build absolute url", () => {
+      expect(safeBuildUrl(absoluteUrl, specUrl, { selectedServer: absoluteServerUrl })).toBe("https://example.com/base-path/path")
+      expect(safeBuildUrl(absoluteUrl, specUrl, { selectedServer: serverUrlRelativeToBase })).toBe("https://example.com/base-path/path")
+      expect(safeBuildUrl(absoluteUrl, specUrl, { selectedServer: serverUrlRelativeToHost })).toBe("https://example.com/base-path/path")
+    })
+
+    it("build relative url with no server selected", () => {
+      expect(safeBuildUrl(urlRelativeToBase, specUrl, { selectedServer: noServerSelected })).toBe("https://petstore.swagger.io/v2/relative-url/base-path/path")
+      expect(safeBuildUrl(urlRelativeToHost, specUrl, { selectedServer: noServerSelected })).toBe("https://petstore.swagger.io/relative-url/base-path/path")
+    })
+
+    it("build relative url with absolute server url", () => {
+      expect(safeBuildUrl(urlRelativeToBase, specUrl, { selectedServer: absoluteServerUrl })).toBe("https://server-example.com/base-path/relative-url/base-path/path")
+      expect(safeBuildUrl(urlRelativeToHost, specUrl, { selectedServer: absoluteServerUrl })).toBe("https://server-example.com/relative-url/base-path/path")
+    })
+
+    it("build relative url with server url relative to base", () => {
+      expect(safeBuildUrl(urlRelativeToBase, specUrl, { selectedServer: serverUrlRelativeToBase })).toBe("https://petstore.swagger.io/v2/server-example/base-path/relative-url/base-path/path")
+      expect(safeBuildUrl(urlRelativeToHost, specUrl, { selectedServer: serverUrlRelativeToBase })).toBe("https://petstore.swagger.io/relative-url/base-path/path")
+    })
+
+    it("build relative url with server url relative to host", () => {
+      expect(safeBuildUrl(urlRelativeToBase, specUrl, { selectedServer: serverUrlRelativeToHost })).toBe("https://petstore.swagger.io/server-example/base-path/relative-url/base-path/path")
+      expect(safeBuildUrl(urlRelativeToHost, specUrl, { selectedServer: serverUrlRelativeToHost })).toBe("https://petstore.swagger.io/relative-url/base-path/path")
+    })
+
+    it("build relative url when no servers defined AND specUrl is invalid Url", () => {
+      expect(safeBuildUrl(urlRelativeToHost, specUrlAsInvalidUrl, { selectedServer: noServerSelected })).toBe("http://localhost/relative-url/base-path/path")
+    })
+
+    it("build relative url when no servers defined AND specUrl is OAS2 non-url string", () => {
+      expect(safeBuildUrl(urlRelativeToHost, specUrlOas2NonUrlString, { selectedServer: noServerSelected })).toBe("http://localhost/relative-url/base-path/path")
+    })
+
+    it("build no url when server url contains non-transcluded server variables", () => {
+      expect(safeBuildUrl(urlRelativeToHost, specUrl, { selectedServer: serverUrlWithVariables })).toBe(undefined)
+    })
   })
 
   describe("requiresValidationURL", () => {