From fd838d4e2d2d92970fdef17a85d8f4b74c4ae658 Mon Sep 17 00:00:00 2001 From: fundon Date: Mon, 19 May 2025 17:05:06 +0000 Subject: [PATCH] fix(editor): should add HTTP protocol into link automatically (#11934) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes: [BS-3291](https://linear.app/affine-design/issue/BS-3291/工具栏展开时报错，链接无法点击打开) ## Summary by CodeRabbit - **New Features** - URLs entered without a protocol (e.g., "github.com/...") are now automatically normalized to use "https://", ensuring links are secure and consistently formatted. - **Bug Fixes** - Improved handling and validation of links to prevent issues with missing or invalid protocols in bookmarks and inline links. - Simplified URL validation logic by leveraging native URL parsing, removing complex regex and email-specific checks. - Streamlined toolbar link actions to operate only on valid normalized URLs. - Refined URL detection in markdown preprocessing to exclude lines containing spaces from being treated as URLs. - **Tests** - Added tests to verify that links without a protocol are correctly normalized and displayed across different views. - Updated URL validation tests to better reflect valid and invalid URL formats, including IP addresses and domain variants. - **Style** - Updated snapshots to reflect the use of "https://" in links. --- .../blocks/bookmark/src/bookmark-block.ts | 11 +- .../src/adapters/markdown/preprocessor.ts | 6 +- .../inlines/link/src/link-node/affine-link.ts | 3 +- .../link/src/link-node/configs/toolbar.ts | 30 ++-- .../src/__tests__/utils/url.unit.spec.ts | 8 +- blocksuite/affine/shared/src/utils/url.ts | 147 ++++++++---------- .../core/src/modules/navigation/utils.ts | 31 ++-- tests/affine-local/e2e/links.spec.ts | 40 +++++ .../e2e/clipboard/clipboard.spec.ts | 2 +- .../link.spec.ts/create-link-with-paste.json | 2 +- 10 files changed, 160 insertions(+), 120 deletions(-) diff --git a/blocksuite/affine/blocks/bookmark/src/bookmark-block.ts b/blocksuite/affine/blocks/bookmark/src/bookmark-block.ts index 4d63bed35c..226857a1b9 100644 --- a/blocksuite/affine/blocks/bookmark/src/bookmark-block.ts +++ b/blocksuite/affine/blocks/bookmark/src/bookmark-block.ts @@ -11,6 +11,7 @@ import { DocModeProvider, LinkPreviewServiceIdentifier, } from '@blocksuite/affine-shared/services'; +import { normalizeUrl } from '@blocksuite/affine-shared/utils'; import { BlockSelection } from '@blocksuite/std'; import { computed, type ReadonlySignal, signal } from '@preact/signals-core'; import { html } from 'lit'; @@ -99,12 +100,12 @@ export class BookmarkBlockComponent extends CaptionedBlockComponent { - let link = this.model.props.url; - if (!link.match(/^[a-zA-Z]+:\/\//)) { - link = 'https://' + link; - } - window.open(link, '_blank'); + window.open(this.link, '_blank'); }; refreshData = () => { diff --git a/blocksuite/affine/blocks/code/src/adapters/markdown/preprocessor.ts b/blocksuite/affine/blocks/code/src/adapters/markdown/preprocessor.ts index ae24961055..47564f9ae7 100644 --- a/blocksuite/affine/blocks/code/src/adapters/markdown/preprocessor.ts +++ b/blocksuite/affine/blocks/code/src/adapters/markdown/preprocessor.ts @@ -48,7 +48,11 @@ const codePreprocessor: MarkdownAdapterPreprocessor = { } trimmedLine = trimmedLine.trimEnd(); - if (!trimmedLine.startsWith('<') && !trimmedLine.endsWith('>')) { + if ( + !trimmedLine.startsWith('<') && + !trimmedLine.endsWith('>') && + !trimmedLine.includes(' ') + ) { // check if it is a url link and wrap it with the angle brackets // sometimes the url includes emphasis `_` that will break URL parsing // diff --git a/blocksuite/affine/inlines/link/src/link-node/affine-link.ts b/blocksuite/affine/inlines/link/src/link-node/affine-link.ts index b78e1dd797..f0f7091dac 100644 --- a/blocksuite/affine/inlines/link/src/link-node/affine-link.ts +++ b/blocksuite/affine/inlines/link/src/link-node/affine-link.ts @@ -7,6 +7,7 @@ import { } from '@blocksuite/affine-shared/services'; import { affineTextStyles } from '@blocksuite/affine-shared/styles'; import type { AffineTextAttributes } from '@blocksuite/affine-shared/types'; +import { normalizeUrl } from '@blocksuite/affine-shared/utils'; import { WithDisposable } from '@blocksuite/global/lit'; import type { BlockComponent, BlockStdScope } from '@blocksuite/std'; import { BLOCK_ID_ATTR, ShadowlessElement } from '@blocksuite/std'; @@ -120,7 +121,7 @@ export class AffineLink extends WithDisposable(ShadowlessElement) { } get link() { - return this.delta.attributes?.link ?? ''; + return normalizeUrl(this.delta.attributes?.link ?? ''); } get selfInlineRange() { diff --git a/blocksuite/affine/inlines/link/src/link-node/configs/toolbar.ts b/blocksuite/affine/inlines/link/src/link-node/configs/toolbar.ts index 80fe5da4a1..70a4841b59 100644 --- a/blocksuite/affine/inlines/link/src/link-node/configs/toolbar.ts +++ b/blocksuite/affine/inlines/link/src/link-node/configs/toolbar.ts @@ -38,6 +38,7 @@ export const builtinInlineLinkToolbarConfig = { if (!(target instanceof AffineLink)) return null; const { link } = target; + if (!link) return null; return html``; }, @@ -115,6 +116,9 @@ export const builtinInlineLinkToolbarConfig = { if (!(target instanceof AffineLink)) return; if (!target.block) return; + const url = target.link; + if (!url) return; + const { block: { model }, inlineEditor, @@ -124,9 +128,6 @@ export const builtinInlineLinkToolbarConfig = { if (!inlineEditor || !selfInlineRange || !parent) return; - const url = inlineEditor.getFormat(selfInlineRange).link; - if (!url) return; - // Clears ctx.reset(); @@ -182,6 +183,9 @@ export const builtinInlineLinkToolbarConfig = { if (!(target instanceof AffineLink)) return false; if (!target.block) return false; + const url = target.link; + if (!url) return false; + const { block: { model }, inlineEditor, @@ -191,9 +195,6 @@ export const builtinInlineLinkToolbarConfig = { if (!inlineEditor || !selfInlineRange || !parent) return false; - const url = inlineEditor.getFormat(selfInlineRange).link; - if (!url) return false; - // check if the url can be embedded as iframe block const embedIframeService = ctx.std.get(EmbedIframeService); const canEmbedAsIframe = embedIframeService.canEmbed(url); @@ -208,6 +209,9 @@ export const builtinInlineLinkToolbarConfig = { if (!(target instanceof AffineLink)) return; if (!target.block) return; + const url = target.link; + if (!url) return; + const { block: { model }, inlineEditor, @@ -217,9 +221,6 @@ export const builtinInlineLinkToolbarConfig = { if (!inlineEditor || !selfInlineRange || !parent) return; - const url = inlineEditor.getFormat(selfInlineRange).link; - if (!url) return; - // Clears ctx.reset(); @@ -306,16 +307,7 @@ export const builtinInlineLinkToolbarConfig = { ) return false; - const { link } = target; - try { - const url = new URL(link); - if (!url.protocol.startsWith('http')) { - return false; - } - } catch (err) { - console.error(err); - return false; - } + if (!target.link.startsWith('http')) return false; const { model } = target.block; const parent = model.parent; diff --git a/blocksuite/affine/shared/src/__tests__/utils/url.unit.spec.ts b/blocksuite/affine/shared/src/__tests__/utils/url.unit.spec.ts index 6331b8bdfe..80d80469be 100644 --- a/blocksuite/affine/shared/src/__tests__/utils/url.unit.spec.ts +++ b/blocksuite/affine/shared/src/__tests__/utils/url.unit.spec.ts @@ -29,13 +29,13 @@ describe('isValidUrl: determining whether a URL is valid is very complicated', ( expect(isValidUrl('www.example.com')).toEqual(true); expect(isValidUrl('example.co')).toEqual(true); expect(isValidUrl('example.cm')).toEqual(true); - expect(isValidUrl('1.1.1.1')).toEqual(true); + expect(isValidUrl('1.1.1.1')).toEqual(false); expect(isValidUrl('example.c')).toEqual(false); }); test('special cases', () => { - expect(isValidUrl('example.com.')).toEqual(true); + expect(isValidUrl('example.com.')).toEqual(false); // I don't know why // private & local networks is excluded @@ -44,8 +44,8 @@ describe('isValidUrl: determining whether a URL is valid is very complicated', ( expect(isValidUrl('localhost')).toEqual(false); expect(isValidUrl('0.0.0.0')).toEqual(false); - expect(isValidUrl('128.0.0.1')).toEqual(true); - expect(isValidUrl('1.0.0.1')).toEqual(true); + expect(isValidUrl('128.0.0.1')).toEqual(false); + expect(isValidUrl('1.0.0.1')).toEqual(false); }); test('email link is a valid URL', () => { diff --git a/blocksuite/affine/shared/src/utils/url.ts b/blocksuite/affine/shared/src/utils/url.ts index edb51b7e42..9265195933 100644 --- a/blocksuite/affine/shared/src/utils/url.ts +++ b/blocksuite/affine/shared/src/utils/url.ts @@ -1,75 +1,66 @@ -export const ALLOWED_SCHEMES = [ +// https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml +const ALLOWED_SCHEMES = new Set([ 'http', 'https', 'ftp', 'sftp', 'mailto', 'tel', - // may need support other schemes -]; -// I guess you don't want to use the regex base the RFC 5322 Official Standard -// For more detail see https://stackoverflow.com/questions/201323/how-can-i-validate-an-email-address-using-a-regular-expression/1917982#1917982 -const MAIL_REGEX = - /^[a-zA-Z0-9.!#$%&’*+/=?^_`{|}~-]+@[a-zA-Z0-9-]+(?:\.[a-zA-Z0-9-]+)*$/; +]); -// For more detail see https://stackoverflow.com/questions/8667070/javascript-regular-expression-to-validate-url -const URL_REGEX = new RegExp( - '^' + - // protocol identifier (optional) - // short syntax // still required - '(?:(?:(?:https?|ftp):)?\\/\\/)' + - // user:pass BasicAuth (optional) - '(?:\\S+(?::\\S*)?@)?' + - '(?:' + - // IP address exclusion - // private & local networks - '(?!(?:10|127)(?:\\.\\d{1,3}){3})' + - '(?!(?:169\\.254|192\\.168)(?:\\.\\d{1,3}){2})' + - '(?!172\\.(?:1[6-9]|2\\d|3[0-1])(?:\\.\\d{1,3}){2})' + - // IP address dotted notation octets - // excludes loopback network 0.0.0.0 - // excludes reserved space >= 224.0.0.0 - // excludes network & broadcast addresses - // (first & last IP address of each class) - '(?:[1-9]\\d?|1\\d\\d|2[01]\\d|22[0-3])' + - '(?:\\.(?:1?\\d{1,2}|2[0-4]\\d|25[0-5])){2}' + - '(?:\\.(?:[1-9]\\d?|1\\d\\d|2[0-4]\\d|25[0-4]))' + - '|' + - // host & domain names, may end with dot - // can be replaced by a shortest alternative - // (?![-_])(?:[-\\w\\u00a1-\\uffff]{0,63}[^-_]\\.)+ - '(?:' + - '(?:' + - '[a-z0-9\\u00a1-\\uffff]' + - '[a-z0-9\\u00a1-\\uffff_-]{0,62}' + - ')?' + - '[a-z0-9\\u00a1-\\uffff]\\.' + - ')+' + - // TLD identifier name, may end with dot - // Addition: We limit the TLD to 2-6 characters, because it can cover most of the cases. - '(?:[a-z\\u00a1-\\uffff]{2,6}\\.?)' + - ')' + - // port number (optional) - '(?::\\d{2,5})?' + - // resource path (optional) - '(?:[/?#]\\S*)?' + - '$', - 'i' -); +// https://publicsuffix.org/ +const TLD_REGEXP = /(?:\.[a-zA-Z]+)?(\.[a-zA-Z]{2,})$/; -export function normalizeUrl(url: string) { - const includeScheme = ALLOWED_SCHEMES.find(scheme => - url.startsWith(scheme + ':') - ); - if (includeScheme) { - // Any link include schema is a valid url - return url; +const toURL = (str: string) => { + try { + if (!URL.canParse(str)) return null; + + return new URL(str); + } catch { + return null; } - const isEmail = MAIL_REGEX.test(url); - if (isEmail) { - return 'mailto:' + url; +}; + +function resolveURL(str: string) { + const url = toURL(str); + if (!url) return null; + + const protocol = url.protocol.substring(0, url.protocol.length - 1); + const hostname = url.hostname; + + let allowed = ALLOWED_SCHEMES.has(protocol); + if (allowed && hostname.includes('.')) { + allowed = TLD_REGEXP.test(hostname); } - return 'http://' + url; + + return { url, allowed }; +} + +export function normalizeUrl(str: string) { + str = str.trim(); + + let url = toURL(str); + + if (!url) { + const hasScheme = str.match(/^https?:\/\//); + + if (!hasScheme) { + const dotIdx = str.indexOf('.'); + if (dotIdx > 0 && dotIdx < str.length - 1) { + url = toURL(`https://${str}`); + } + } + } + + // Formatted + if (url) { + if (!str.endsWith('/') && url.href.endsWith('/')) { + return url.href.substring(0, url.href.length - 1); + } + return url.href; + } + + return str; } /** @@ -78,20 +69,23 @@ export function normalizeUrl(url: string) { * For more detail see https://www.ietf.org/rfc/rfc1738.txt */ export function isValidUrl(str: string) { - if (!str) { - return false; - } - const url = normalizeUrl(str); - if (url === str) { - // Skip check if user input scheme manually - try { - new URL(url); - } catch { - return false; + str = str.trim(); + + let result = resolveURL(str); + + if (result && !result.allowed) return false; + + if (!result) { + const hasScheme = str.match(/^https?:\/\//); + if (!hasScheme) { + const dotIdx = str.indexOf('.'); + if (dotIdx > 0 && dotIdx < str.length - 1) { + result = resolveURL(`https://${str}`); + } } - return true; } - return URL_REGEX.test(url); + + return result?.allowed ?? false; } // https://en.wikipedia.org/wiki/Top-level_domain @@ -119,10 +113,7 @@ const COMMON_TLDS = new Set([ ]); function isCommonTLD(url: URL) { - const tld = url.hostname.split('.').pop(); - if (!tld) { - return false; - } + const tld = url.hostname.split('.').pop() ?? ''; return COMMON_TLDS.has(tld); } diff --git a/packages/frontend/core/src/modules/navigation/utils.ts b/packages/frontend/core/src/modules/navigation/utils.ts index 2f59d7d7bb..34aaf40d04 100644 --- a/packages/frontend/core/src/modules/navigation/utils.ts +++ b/packages/frontend/core/src/modules/navigation/utils.ts @@ -74,23 +74,34 @@ export const resolveRouteLinkMeta = ( } }; -export const isLink = (href: string) => { +const toURL = (str: string) => { try { - const hasScheme = href.match(/^https?:\/\//); + if (!URL.canParse(str)) return null; - if (!hasScheme) { - const dotIdx = href.indexOf('.'); - if (dotIdx > 0 && dotIdx < href.length - 1) { - href = `https://${href}`; - } - } - - return Boolean(URL.canParse?.(href) ?? new URL(href)); + return new URL(str); } catch { return null; } }; +export const isLink = (str: string) => { + str = str.trim(); + + let url = toURL(str); + + if (!url) { + const hasScheme = str.match(/^https?:\/\//); + if (!hasScheme) { + const dotIdx = str.indexOf('.'); + if (dotIdx > 0 && dotIdx < str.length - 1) { + url = toURL(`https://${str}`); + } + } + } + + return Boolean(url); +}; + /** * @see /packages/frontend/core/src/router.tsx */ diff --git a/tests/affine-local/e2e/links.spec.ts b/tests/affine-local/e2e/links.spec.ts index b55f56c2fe..200eca998a 100644 --- a/tests/affine-local/e2e/links.spec.ts +++ b/tests/affine-local/e2e/links.spec.ts @@ -1271,3 +1271,43 @@ test('should display date as the original title of journal', async ({ await expect(toolbar).toBeVisible(); await expect(linkedDocTitle).toBeHidden(); }); + +test('should add HTTP protocol into link automatically', async ({ page }) => { + await page.keyboard.press('Enter'); + + await page.keyboard.type('github.com'); + await page.keyboard.type('/'); + await page.keyboard.type('toeverything'); + await page.keyboard.type('/'); + await page.keyboard.type('affine'); + + await page.keyboard.press('Space'); + + const link = 'https://github.com/toeverything/affine'; + + const { toolbar, switchViewBtn, cardViewBtn } = toolbarButtons(page); + + const inlineLink = page.locator('affine-link'); + + await expect(inlineLink).toBeVisible(); + + let url = await inlineLink.locator('a').getAttribute('href'); + expect(url).toBe(link); + + await inlineLink.hover(); + + const linkPreview = toolbar.locator('affine-link-preview'); + + url = await linkPreview.locator('a').getAttribute('href'); + expect(url).toBe(link); + + await switchViewBtn.click(); + await cardViewBtn.click(); + + const cardLink = page.locator('affine-bookmark'); + + await expect(cardLink).toBeVisible(); + + url = await linkPreview.locator('a').getAttribute('href'); + expect(url).toBe(link); +}); diff --git a/tests/blocksuite/e2e/clipboard/clipboard.spec.ts b/tests/blocksuite/e2e/clipboard/clipboard.spec.ts index d4508c48e2..6d22eaf409 100644 --- a/tests/blocksuite/e2e/clipboard/clipboard.spec.ts +++ b/tests/blocksuite/e2e/clipboard/clipboard.spec.ts @@ -227,7 +227,7 @@ test(scoped`auto identify url`, async ({ page }, testInfo) => { // set up clipboard data using html const clipData = { - 'text/plain': `test https://www.google.com`, + 'text/plain': 'test https://www.google.com', }; await waitNextFrame(page); await page.evaluate( diff --git a/tests/blocksuite/snapshots/link.spec.ts/create-link-with-paste.json b/tests/blocksuite/snapshots/link.spec.ts/create-link-with-paste.json index 2fdb29747b..54f6da0d4b 100644 --- a/tests/blocksuite/snapshots/link.spec.ts/create-link-with-paste.json +++ b/tests/blocksuite/snapshots/link.spec.ts/create-link-with-paste.json @@ -14,7 +14,7 @@ "delta": [ { "attributes": { - "link": "http://affine.pro" + "link": "https://affine.pro" }, "insert": "aaa" }