Skip to content

Commit 3c3b7e0

Browse files
shockeyshockey
authored
bug(deeplinking): escaping breaks whitespaces & underscored tags/ids (via #4953)
* add tests for operation lacking an operationId * add deep linking tests for tags/operationIds with underscores * migrate from `_` to `%20` for deeplink hash whitespace escaping * add backwards compatibility for `_` whitespace escaping * update util unit tests
1 parent 94b1355 commit 3c3b7e0

File tree

7 files changed

+231
-12
lines changed

7 files changed

+231
-12
lines changed

‎src/core/components/operation.jsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import React, { PureComponent } from "react"
22
import PropTypes from "prop-types"
33
import { getList } from "core/utils"
4-
import { getExtensions, sanitizeUrl, createDeepLinkPath } from "core/utils"
4+
import { getExtensions, sanitizeUrl, escapeDeepLinkPath } from "core/utils"
55
import { Iterable, List } from "immutable"
66
import ImPropTypes from "react-immutable-proptypes"
77

@@ -112,7 +112,7 @@ export default class Operation extends PureComponent {
112112
let onChangeKey = [ path, method ] // Used to add values to _this_ operation ( indexed by path and method )
113113

114114
return (
115-
<div className={deprecated ? "opblock opblock-deprecated" : isShown ? `opblock opblock-${method} is-open` : `opblock opblock-${method}`} id={createDeepLinkPath(isShownKey.join("-"))} >
115+
<div className={deprecated ? "opblock opblock-deprecated" : isShown ? `opblock opblock-${method} is-open` : `opblock opblock-${method}`} id={escapeDeepLinkPath(isShownKey.join("-"))} >
116116
<OperationSummary operationProps={operationProps} toggleShown={toggleShown} getComponent={getComponent} authActions={authActions} authSelectors={authSelectors} specPath={specPath} />
117117
<Collapse isOpened={isShown}>
118118
<div className="opblock-body">

‎src/core/plugins/deep-linking/layout.js‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,18 +73,35 @@ export const parseDeepLinkHash = (rawHash) => ({ layoutActions, layoutSelectors,
7373
hash = hash.slice(1)
7474
}
7575

76-
const hashArray = hash.split("/").map(val => (val || "").replace(/_/g, " "))
76+
const hashArray = hash.split("/").map(val => (val || "").replace(/%20/g, " "))
7777

7878
const isShownKey = layoutSelectors.isShownKeyFromUrlHashArray(hashArray)
7979

80-
const [type, tagId] = isShownKey
80+
const [type, tagId, maybeOperationId] = isShownKey
8181

8282
if(type === "operations") {
8383
// we're going to show an operation, so we need to expand the tag as well
84-
layoutActions.show(layoutSelectors.isShownKeyFromUrlHashArray([tagId]))
84+
const tagIsShownKey = layoutSelectors.isShownKeyFromUrlHashArray([tagId])
85+
layoutActions.show(tagIsShownKey)
86+
87+
// If an `_` is present, trigger the legacy escaping behavior to be safe
88+
// TODO: remove this in v4.0, it is deprecated
89+
if(tagId.indexOf("_") > -1) {
90+
console.warn("Warning: escaping deep link whitespace with `_` will be unsupported in v4.0, use `%20` instead.")
91+
layoutActions.show(tagIsShownKey.map(val => val.replace(/_/g, " ")), true)
92+
}
93+
}
94+
95+
layoutActions.show(isShownKey, true)
96+
97+
// If an `_` is present, trigger the legacy escaping behavior to be safe
98+
// TODO: remove this in v4.0, it is deprecated
99+
if (tagId.indexOf("_") > -1 || maybeOperationId.indexOf("_") > -1) {
100+
console.warn("Warning: escaping deep link whitespace with `_` will be unsupported in v4.0, use `%20` instead.")
101+
layoutActions.show(isShownKey.map(val => val.replace(/_/g, " ")), true)
85102
}
86103

87-
layoutActions.show(isShownKey, true) // TODO: 'show' operation tag
104+
// Scroll to the newly expanded entity
88105
layoutActions.scrollTo(isShownKey)
89106
}
90107
}

‎src/core/utils.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -733,8 +733,10 @@ export function getAcceptControllingResponse(responses) {
733733
return suitable2xxResponse || suitableDefaultResponse
734734
}
735735

736-
export const createDeepLinkPath = (str) => typeof str == "string" || str instanceof String ? str.trim().replace(/\s/g, "_") : ""
737-
export const escapeDeepLinkPath = (str) => cssEscape( createDeepLinkPath(str) )
736+
// suitable for use in URL fragments
737+
export const createDeepLinkPath = (str) => typeof str == "string" || str instanceof String ? str.trim().replace(/\s/g, "%20") : ""
738+
// suitable for use in CSS classes and ids
739+
export const escapeDeepLinkPath = (str) => cssEscape( createDeepLinkPath(str).replace(/%20/g, "_") )
738740

739741
export const getExtensions = (defObj) => defObj.filter((v, k) => /^x-/.test(k))
740742
export const getCommonExtensions = (defObj) => defObj.filter((v, k) => /^pattern|maxLength|minLength|maximum|minimum/.test(k))

‎test/core/utils.js‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -991,12 +991,12 @@ describe("utils", function() {
991991
describe("createDeepLinkPath", function() {
992992
it("creates a deep link path replacing spaces with underscores", function() {
993993
const result = createDeepLinkPath("tag id with spaces")
994-
expect(result).toEqual("tag_id_with_spaces")
994+
expect(result).toEqual("tag%20id%20with%20spaces")
995995
})
996996

997997
it("trims input when creating a deep link path", function() {
998998
let result = createDeepLinkPath(" spaces before and after ")
999-
expect(result).toEqual("spaces_before_and_after")
999+
expect(result).toEqual("spaces%20before%20and%20after")
10001000

10011001
result = createDeepLinkPath(" ")
10021002
expect(result).toEqual("")
@@ -1036,6 +1036,16 @@ describe("utils", function() {
10361036
const result = escapeDeepLinkPath("hello#world")
10371037
expect(result).toEqual("hello\\#world")
10381038
})
1039+
1040+
it("escapes a deep link path with a space", function() {
1041+
const result = escapeDeepLinkPath("hello world")
1042+
expect(result).toEqual("hello_world")
1043+
})
1044+
1045+
it("escapes a deep link path with a percent-encoded space", function() {
1046+
const result = escapeDeepLinkPath("hello%20world")
1047+
expect(result).toEqual("hello_world")
1048+
})
10391049
})
10401050

10411051
describe("getExtensions", function() {

‎test/e2e-cypress/static/documents/features/deep-linking.openapi.yaml‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,29 @@ paths:
1818
operationId: "my Operation"
1919
tags: ["my Tag"]
2020
summary: an operation
21+
responses:
22+
'200':
23+
description: a pet to be returned
24+
content:
25+
application/json:
26+
schema:
27+
type: object
28+
/withUnderscores:
29+
patch:
30+
operationId: "underscore_Operation"
31+
tags: ["underscore_Tag"]
32+
summary: an operation
33+
responses:
34+
'200':
35+
description: a pet to be returned
36+
content:
37+
application/json:
38+
schema:
39+
type: object
40+
/noOperationId:
41+
put:
42+
tags: ["tagTwo"]
43+
summary: some operations are anonymous...
2144
responses:
2245
'200':
2346
description: a pet to be returned

‎test/e2e-cypress/static/documents/features/deep-linking.swagger.yaml‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,3 +17,18 @@ paths:
1717
responses:
1818
"200":
1919
description: ok
20+
/withUnderscores:
21+
patch:
22+
operationId: "underscore_Operation"
23+
tags: ["underscore_Tag"]
24+
summary: an operation
25+
responses:
26+
"200":
27+
description: ok
28+
/noOperationId:
29+
put:
30+
tags: ["tagTwo"]
31+
summary: an operation
32+
responses:
33+
"200":
34+
description: ok

‎test/e2e-cypress/tests/deep-linking.js‎

Lines changed: 154 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,83 @@ describe("Deep linking feature", () => {
4141
describe("Operation with whitespace in tag+id", () => {
4242
const elementToGet = ".opblock-post"
4343
const correctElementId = "operations-my_Tag-my_Operation"
44-
const correctFragment = "#/my_Tag/my_Operation"
44+
const correctFragment = "#/my%20Tag/my%20Operation"
45+
const legacyFragment = "#/my_Tag/my_Operation"
46+
47+
it("should generate a correct element ID", () => {
48+
cy.get(elementToGet)
49+
.should("have.id", correctElementId)
50+
})
51+
52+
it("should add the correct element fragment to the URL when expanded", () => {
53+
cy.get(elementToGet)
54+
.click()
55+
.window()
56+
.should("have.deep.property", "location.hash", correctFragment)
57+
})
58+
59+
it("should provide an anchor link that has the correct fragment as href", () => {
60+
cy.get(elementToGet)
61+
.find("a")
62+
.should("have.attr", "href", correctFragment)
63+
.click()
64+
.should("have.attr", "href", correctFragment) // should be valid after expanding
65+
66+
})
67+
68+
it("should expand the operation when reloaded", () => {
69+
cy.visit(`${baseUrl}${correctFragment}`)
70+
.reload()
71+
.get(`${elementToGet}.is-open`)
72+
.should("exist")
73+
})
74+
75+
it("should expand the operation when reloaded and provided the legacy fragment", () => {
76+
cy.visit(`${baseUrl}${legacyFragment}`)
77+
.reload()
78+
.get(`${elementToGet}.is-open`)
79+
.should("exist")
80+
})
81+
})
82+
83+
describe("Operation with underscores in tag+id", () => {
84+
const elementToGet = ".opblock-patch"
85+
const correctElementId = "operations-underscore_Tag-underscore_Operation"
86+
const correctFragment = "#/underscore_Tag/underscore_Operation"
87+
88+
it("should generate a correct element ID", () => {
89+
cy.get(elementToGet)
90+
.should("have.id", correctElementId)
91+
})
92+
93+
it("should add the correct element fragment to the URL when expanded", () => {
94+
cy.get(elementToGet)
95+
.click()
96+
.window()
97+
.should("have.deep.property", "location.hash", correctFragment)
98+
})
99+
100+
it("should provide an anchor link that has the correct fragment as href", () => {
101+
cy.get(elementToGet)
102+
.find("a")
103+
.should("have.attr", "href", correctFragment)
104+
.click()
105+
.should("have.attr", "href", correctFragment) // should be valid after expanding
106+
107+
})
108+
109+
it("should expand the operation when reloaded", () => {
110+
cy.visit(`${baseUrl}${correctFragment}`)
111+
.reload()
112+
.get(`${elementToGet}.is-open`)
113+
.should("exist")
114+
})
115+
})
116+
117+
describe("Operation with no operationId", () => {
118+
const elementToGet = ".opblock-put"
119+
const correctElementId = "operations-tagTwo-put_noOperationId"
120+
const correctFragment = "#/tagTwo/put_noOperationId"
45121

46122
it("should generate a correct element ID", () => {
47123
cy.get(elementToGet)
@@ -127,7 +203,83 @@ describe("Deep linking feature", () => {
127203
describe("Operation with whitespace in tag+id", () => {
128204
const elementToGet = ".opblock-post"
129205
const correctElementId = "operations-my_Tag-my_Operation"
130-
const correctFragment = "#/my_Tag/my_Operation"
206+
const correctFragment = "#/my%20Tag/my%20Operation"
207+
const legacyFragment = "#/my_Tag/my_Operation"
208+
209+
it("should generate a correct element ID", () => {
210+
cy.get(elementToGet)
211+
.should("have.id", correctElementId)
212+
})
213+
214+
it("should add the correct element fragment to the URL when expanded", () => {
215+
cy.get(elementToGet)
216+
.click()
217+
.window()
218+
.should("have.deep.property", "location.hash", correctFragment)
219+
})
220+
221+
it("should provide an anchor link that has the correct fragment as href", () => {
222+
cy.get(elementToGet)
223+
.find("a")
224+
.should("have.attr", "href", correctFragment)
225+
.click()
226+
.should("have.attr", "href", correctFragment) // should be valid after expanding
227+
228+
})
229+
230+
it("should expand the operation when reloaded", () => {
231+
cy.visit(`${baseUrl}${correctFragment}`)
232+
.reload()
233+
.get(`${elementToGet}.is-open`)
234+
.should("exist")
235+
})
236+
237+
it("should expand the operation when reloaded and provided the legacy fragment", () => {
238+
cy.visit(`${baseUrl}${legacyFragment}`)
239+
.reload()
240+
.get(`${elementToGet}.is-open`)
241+
.should("exist")
242+
})
243+
})
244+
245+
describe("Operation with underscores in tag+id", () => {
246+
const elementToGet = ".opblock-patch"
247+
const correctElementId = "operations-underscore_Tag-underscore_Operation"
248+
const correctFragment = "#/underscore_Tag/underscore_Operation"
249+
250+
it("should generate a correct element ID", () => {
251+
cy.get(elementToGet)
252+
.should("have.id", correctElementId)
253+
})
254+
255+
it("should add the correct element fragment to the URL when expanded", () => {
256+
cy.get(elementToGet)
257+
.click()
258+
.window()
259+
.should("have.deep.property", "location.hash", correctFragment)
260+
})
261+
262+
it("should provide an anchor link that has the correct fragment as href", () => {
263+
cy.get(elementToGet)
264+
.find("a")
265+
.should("have.attr", "href", correctFragment)
266+
.click()
267+
.should("have.attr", "href", correctFragment) // should be valid after expanding
268+
269+
})
270+
271+
it("should expand the operation when reloaded", () => {
272+
cy.visit(`${baseUrl}${correctFragment}`)
273+
.reload()
274+
.get(`${elementToGet}.is-open`)
275+
.should("exist")
276+
})
277+
})
278+
279+
describe("Operation with no operationId", () => {
280+
const elementToGet = ".opblock-put"
281+
const correctElementId = "operations-tagTwo-put_noOperationId"
282+
const correctFragment = "#/tagTwo/put_noOperationId"
131283

132284
it("should generate a correct element ID", () => {
133285
cy.get(elementToGet)

0 commit comments

Comments
 (0)