-
Notifications
You must be signed in to change notification settings - Fork 63k
Closed
Labels
engineeringWill involve Docs EngineeringWill involve Docs Engineering
Description
What is the current behavior?
Three of the tests related to homepage calculation. The are all of the following flavor.
FAIL tests/rendering/header.js (66.489 s)
● header › mobile-only product dropdown links › include github and admin, and emphasize the current product
expect(received).toBe(expected) // Object.is equality
Expected: 1
Received: 0
72 |
73 | const github = $(`#homepages a.active[href="/en/${nonEnterpriseDefaultVersion}/github"]`)
> 74 | expect(github.length).toBe(1)
| ^
75 | expect(github.text().trim()).toBe('GitHub.com')
76 | expect(github.attr('class').includes('active')).toBe(true)
77 |
at Object.<anonymous> (tests/rendering/header.js:74:29)
and here is a sample of the HTML being returned from get
. As you can see, Windows path separators () are being used indicating that path.join
is likely involved. I've not isolated the particular code as yet.
class=\"position-md-absolute nav-desktop-productDropdown p-md-4 left-md-n4
top-md-6\" style=\"z-index: 6;\">\r\n \r\n <a
href=\"/en\\free-pro-team@latest\\github\"\r\n class=\"d-block py-2\r\n
text-blue-mktg text-underline active\">\r\n GitHub.com\r\n \r\n </a>\r\n
\r\n <a href=\"/en\\[email protected]\\admin\"\r\n class=\"d-block
py-2\r\n link-gray-dark no-underline\">\r\n Enterprise
Administrators\r\n \r\n </a>\r\n \r\n <a
href=\"/en\\free-pro-team@latest\\actions\"\r\n class=\"d-block py-2\r\n
link-gray-dark no-underline\">\r\n GitHub Actions\r\n \r\n </a>\r\n \r\n
<a href=\"/en\\free-pro-team@latest\\packages\"\r\n class=\"d-block py-2\r\n
What changes are you suggesting?
Suggest we do an audit of all the places that inject URLs into content or headers and ensure they don't use path.join
. In fact, we should make a helper (e.g., safeJoin
) that does something like (...segments) => slash(path.join(segments))
and then use that everywhere that has anything to do with web results.
cc @github/docs-engineering , @chiedo
Metadata
Metadata
Assignees
Labels
engineeringWill involve Docs EngineeringWill involve Docs Engineering