Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(extension-link): make the javascript link detection case insensitive #5153

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions packages/extension-link/src/link.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,7 @@ export const Link = Mark.create<LinkOptions>({
},

renderHTML({ HTMLAttributes }) {
// False positive; we're explicitly checking for javascript: links to ignore them
// eslint-disable-next-line no-script-url
if (HTMLAttributes.href?.startsWith('javascript:')) {
if (HTMLAttributes.href?.substring(0, HTMLAttributes.href.indexOf(':')).toLowerCase().trim() === 'javascript') {
// strip out the href
return ['a', mergeAttributes(this.options.HTMLAttributes, { ...HTMLAttributes, href: '' }), 0]
}
Expand Down
84 changes: 47 additions & 37 deletions tests/cypress/integration/extensions/link.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,45 +20,55 @@ describe('extension-link', () => {
}
const getEditorEl = () => document.querySelector(`.${editorElClass}`)

it('does not output src tag for javascript schema', () => {
editor = new Editor({
element: createEditorEl(),
extensions: [
Document,
Text,
Paragraph,
Link,
],
content: {
type: 'doc',
content: [
{
type: 'paragraph',
content: [
{
type: 'text',
text: 'hello world!',
marks: [
{
type: 'link',
attrs: {
// We have to disable the eslint rule here because we're trying to purposely test eval urls
// eslint-disable-next-line no-script-url
href: 'javascript:alert(window.origin)',
},
},
],
},
],
},
const invalidUrls = [
// We have to disable the eslint rule here because we're trying to purposely test eval urls
// eslint-disable-next-line no-script-url
'javascript:alert(window.origin)',
// eslint-disable-next-line no-script-url
' javascript:alert(window.origin)',
// eslint-disable-next-line no-script-url
'jAvAsCrIpT:alert(window.origin)',
]

invalidUrls.forEach(url => {
it('does not output src tag for javascript schema', () => {
editor = new Editor({
element: createEditorEl(),
extensions: [
Document,
Text,
Paragraph,
Link,
],
},
})
content: {
type: 'doc',
content: [
{
type: 'paragraph',
content: [
{
type: 'text',
text: 'hello world!',
marks: [
{
type: 'link',
attrs: {
href: url,
},
},
],
},
],
},
],
},
})

// eslint-disable-next-line no-script-url
expect(editor.getHTML()).to.not.include('javascript:alert(window.origin)')
// eslint-disable-next-line no-script-url
expect(editor.getHTML()).to.not.include('javascript:alert(window.origin)')

editor?.destroy()
getEditorEl()?.remove()
editor?.destroy()
getEditorEl()?.remove()
})
})
})
4 changes: 4 additions & 0 deletions tests/cypress/integration/extensions/youtube.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ describe('extension-youtube', () => {
// We have to disable the eslint rule here because we're trying to purposely test eval urls
// eslint-disable-next-line no-script-url
'javascript:alert(window.origin)//embed/',
// eslint-disable-next-line no-script-url
' javascript:alert(window.origin)',
// eslint-disable-next-line no-script-url
'jAvAsCrIpT:alert(window.origin)//embed/',
'https://youtube.google.com/embed/fdsafsdf',
'https://youtube.com.bad/embed',
]
Expand Down