From a9eb512a68adf51f27d94347cb9791de1324fad0 Mon Sep 17 00:00:00 2001 From: miinhho Date: Wed, 14 Oct 2026 19:49:42 +0900 Subject: [PATCH 1/2] Prevent automatic image requests in AI Assistant answers --- .../projects/zeppelin-react/README.md | 4 +- .../assistant/ui/MarkdownAnswer.css.ts | 52 ++++++ .../assistant/ui/MarkdownAnswer.image.svg | 36 ++++ .../assistant/ui/MarkdownAnswer.spec.tsx | 144 ++++++++++++++++ .../assistant/ui/MarkdownAnswer.stories.tsx | 54 ++++++ .../entities/assistant/ui/MarkdownAnswer.tsx | 92 +++++++++- .../storybook/markdown-answer-images.spec.mjs | 160 ++++++++++++++++++ 7 files changed, 536 insertions(+), 6 deletions(-) create mode 100644 zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/MarkdownAnswer.image.svg create mode 100644 zeppelin-web-angular/projects/zeppelin-react/test/storybook/markdown-answer-images.spec.mjs diff --git a/zeppelin-web-angular/projects/zeppelin-react/README.md b/zeppelin-web-angular/projects/zeppelin-react/README.md index 4c45e4d7ad6..1469d75a6fd 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/README.md +++ b/zeppelin-web-angular/projects/zeppelin-react/README.md @@ -201,4 +201,6 @@ Pass `describeParagraph` and `onOpenParagraph` to enable paragraph reference but HTML is displayed as text and dangerous link URLs are blocked. -See **Assistant / MarkdownAnswer** in Storybook for complete answers, streaming, and paragraph reference examples in both themes. +Images remain unloaded until you select **Load image**. The control shows the destination and warns that the request may transmit information from the answer. Only HTTP(S) image URLs are supported; changing the URL requires approval again. + +See **Assistant / MarkdownAnswer** in Storybook for complete answers, streaming, paragraph references, **ImageLoading** and **StreamingImages** examples in both themes. diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/MarkdownAnswer.css.ts b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/MarkdownAnswer.css.ts index a39fa0701a1..2e54a84c9e0 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/MarkdownAnswer.css.ts +++ b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/MarkdownAnswer.css.ts @@ -119,6 +119,58 @@ export const paragraphReference = style({ } }); +export const imagePlaceholder = style({ + display: 'inline-flex', + flexDirection: 'column', + alignItems: 'flex-start', + gap: vars.space2, + maxWidth: '100%', + margin: `${vars.space1} 0`, + padding: vars.space3, + border: `1px solid ${vars.border}`, + borderRadius: vars.radius, + background: vars.fill, + verticalAlign: 'middle' +}); + +export const imageDescription = style({ + fontWeight: 600 +}); + +export const imageDetails = style({ + display: 'flex', + flexDirection: 'column', + gap: vars.space1, + maxWidth: '100%', + color: vars.textSecondary, + fontSize: vars.fontSizeSm, + overflowWrap: 'anywhere' +}); + +export const loadImage = style({ + padding: `${vars.space1} ${vars.space2}`, + border: `1px solid ${vars.accentBorder}`, + borderRadius: vars.radiusSm, + background: vars.accentBg, + color: vars.accentText, + font: 'inherit', + cursor: 'pointer', + selectors: { + '&:hover': { color: vars.accentTextHover } + } +}); + +export const loadedImage = style({ + display: 'block', + maxWidth: '100%', + height: 'auto' +}); + +export const imageLink = style({ + display: 'inline', + maxWidth: '100%' +}); + export const codeBlock = style({ minWidth: 0, maxWidth: '100%', diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/MarkdownAnswer.image.svg b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/MarkdownAnswer.image.svg new file mode 100644 index 00000000000..27fa8004ee5 --- /dev/null +++ b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/MarkdownAnswer.image.svg @@ -0,0 +1,36 @@ + + + Quarterly revenue + Revenue grows from 120 in January to 180 in February and 240 in March. + + + Quarterly revenue + + + + + + 120 + 180 + 240 + January + February + March + + + diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/MarkdownAnswer.spec.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/MarkdownAnswer.spec.tsx index 629c6ae6209..e0cf59bba3f 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/MarkdownAnswer.spec.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/MarkdownAnswer.spec.tsx @@ -416,6 +416,150 @@ describe('MarkdownAnswer', () => { }); }); + describe('image loading', () => { + const sourceA = 'https://images.example.test/plot.svg?q=SYNTHETIC_NOTEBOOK_VALUE'; + const sourceB = 'https://images.example.test/plot.svg?q=UPDATED_NOTEBOOK_VALUE'; + const imageMarkdown = (source: string) => `![Quarterly revenue](${source} "Revenue plot")`; + + it('shows the resolved destination and warning without an image until explicitly activated', () => { + const { container } = render(); + + expect(screen.queryByRole('img')).toBeNull(); + expect(container.querySelector('img, [src], [srcset], link[rel="preload"]')).toBeNull(); + expect(screen.getByText(`Image URL: ${sourceA}`)).toBeTruthy(); + expect(screen.getByText('Host: images.example.test')).toBeTruthy(); + expect(screen.getByText(/loading.*sends a request.*information from.*answer/i)).toBeTruthy(); + expect(container.textContent).toContain('Quarterly revenue'); + const button = screen.getByRole('button', { name: /load image/i }); + + fireEvent.click(button); + + const image = screen.getByRole('img', { name: 'Quarterly revenue' }); + expect(image.getAttribute('src')).toBe(sourceA); + expect(image.getAttribute('alt')).toBe('Quarterly revenue'); + expect(image.getAttribute('title')).toBe('Revenue plot'); + expect(image.hasAttribute('srcset')).toBe(false); + }); + + it('resolves relative destinations against the actual document base URI', () => { + const relativeSource = '/assistant-plot.svg?q=relative'; + const resolvedSource = new URL(relativeSource, document.baseURI); + render(); + + expect(screen.getByText(`Image URL: ${resolvedSource.href}`)).toBeTruthy(); + expect(screen.getByText(`Host: ${resolvedSource.hostname}`)).toBeTruthy(); + expect(screen.queryByRole('img')).toBeNull(); + fireEvent.click(screen.getByRole('button', { name: /load image/i })); + expect(screen.getByRole('img').getAttribute('src')).toBe(resolvedSource.href); + }); + + it('requires fresh consent for query changes, including returning to a previously loaded URL', () => { + const { rerender } = render(); + fireEvent.click(screen.getByRole('button', { name: /load image/i })); + expect(screen.getByRole('img').getAttribute('src')).toBe(sourceA); + + rerender(); + expect(screen.queryByRole('img')).toBeNull(); + fireEvent.click(screen.getByRole('button', { name: /load image/i })); + expect(screen.getByRole('img').getAttribute('src')).toBe(sourceB); + + rerender(); + expect(screen.queryByRole('img')).toBeNull(); + expect(screen.getByRole('button', { name: /load image/i })).toBeTruthy(); + }); + + it('requires fresh consent when the destination path changes', () => { + const { rerender } = render(); + fireEvent.click(screen.getByRole('button', { name: /load image/i })); + + rerender(); + + expect(screen.queryByRole('img')).toBeNull(); + expect(screen.getByRole('button', { name: /load image/i })).toBeTruthy(); + }); + + it('keeps approval for an unchanged URL while more answer text is appended', () => { + const content = `Revenue increased.\n\n${imageMarkdown(sourceA)}`; + const { rerender } = render(); + fireEvent.click(screen.getByRole('button', { name: /load image/i })); + + rerender(); + + expect(screen.getByRole('img', { name: 'Quarterly revenue' }).getAttribute('src')).toBe(sourceA); + expect(screen.queryByRole('button', { name: /load image/i })).toBeNull(); + expect(screen.getByText('The next quarter also improved.')).toBeTruthy(); + }); + + it('does not share approval with another image in the same answer', () => { + render(); + const buttons = screen.getAllByRole('button', { name: /load image/i }); + + fireEvent.click(buttons[0]); + + expect(screen.getAllByRole('img')).toHaveLength(1); + expect(screen.getByRole('img', { name: 'Quarterly revenue' })).toBeTruthy(); + expect(screen.queryByRole('img', { name: 'Comparison' })).toBeNull(); + expect(screen.getAllByRole('button', { name: /load image/i })).toHaveLength(1); + }); + + it('does not share approval between independently rendered answers', () => { + const first = render(); + fireEvent.click(within(first.container).getByRole('button', { name: /load image/i })); + const second = render(); + + expect(within(first.container).getByRole('img')).toBeTruthy(); + expect(within(second.container).queryByRole('img')).toBeNull(); + expect(within(second.container).getByRole('button', { name: /load image/i })).toBeTruthy(); + }); + + it.each(['javascript:alert%281%29', 'ftp://images.example.test/plot.svg', ''])( + 'does not offer loading for the unsupported or empty image destination %s', + source => { + const { container } = render(); + + expect(screen.queryByRole('img')).toBeNull(); + expect(screen.queryByRole('button', { name: /load image/i })).toBeNull(); + expect(container.querySelector('[src], [srcset]')).toBeNull(); + expect(container.textContent).toContain('Quarterly revenue'); + } + ); + + it('keeps the original link separate from the image loading button and preserves mixed content', () => { + const href = 'https://zeppelin.apache.org/docs/latest/'; + const { container } = render( + + ); + const button = screen.getByRole('button', { name: /load image/i }); + const links = screen.getAllByRole('link'); + + expect(button.closest('a')).toBeNull(); + expect(button.closest('p')).not.toBeNull(); + expect(container.querySelector('p p, p div')).toBeNull(); + expect(container.textContent).toContain('Before'); + expect(screen.getByText('bold').tagName).toBe('STRONG'); + expect(container.textContent).toContain('after'); + for (const link of links) { + expect(link.getAttribute('href')).toBe(href); + expect(link.getAttribute('target')).toBe('_blank'); + expect(link.getAttribute('rel')?.split(/\s+/)).toEqual(expect.arrayContaining(['noopener', 'noreferrer'])); + } + + fireEvent.click(button); + + expect(screen.getByRole('img', { name: 'Quarterly revenue' }).getAttribute('src')).toBe(sourceA); + expect(screen.getAllByRole('link').map(link => link.getAttribute('href'))).toEqual(links.map(() => href)); + }); + + it('retains a navigation link for an image-only Markdown link without nesting its load button', () => { + const href = 'https://zeppelin.apache.org/docs/latest/'; + render(); + + const button = screen.getByRole('button', { name: /load image/i }); + expect(button.closest('a')).toBeNull(); + expect(screen.getByRole('link').getAttribute('href')).toBe(href); + }); + }); + it.each(dangerousUrls)('renders the rejected URL %s as non-navigable text', url => { const { container } = render(); diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/MarkdownAnswer.stories.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/MarkdownAnswer.stories.tsx index 58aa97396d6..a2dcd25783d 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/MarkdownAnswer.stories.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/MarkdownAnswer.stories.tsx @@ -30,6 +30,36 @@ const streamingChunks = [ 'GROUP BY product;\n```\n\nRevenue is grouped by product.' ]; +const imageFixtureUrl = new URL('./MarkdownAnswer.image.svg?no-inline', import.meta.url).href; +const imageQuerySeparator = imageFixtureUrl.includes('?') ? '&' : '?'; +const streamingImageChunks = [ + '### Quarterly revenue\n\n![Quarterly revenue chart][revenue-chart]\n\nRevenue increased across the quarter.', + '\n\nThe strongest growth came from recurring notebook subscriptions.', + '\n\nThe updated chart includes the final month of the quarter.' +]; + +const StreamingImagesExample = (args: MarkdownAnswerProps) => { + const [step, setStep] = useState(0); + const completed = step === streamingImageChunks.length - 1; + const version = completed ? 'B' : 'A'; + const content = `${streamingImageChunks.slice(0, step + 1).join('')}\n\n[revenue-chart]: ${imageFixtureUrl}${imageQuerySeparator}version=${version}`; + + return ( +
+ + +

+ Chunk {step + 1} of {streamingImageChunks.length} +

+ +
+ ); +}; + const ParagraphReferenceExample = ({ onOpenParagraph, ...args }: MarkdownAnswerProps) => { const [selectedParagraph, setSelectedParagraph] = useState('No paragraph selected.'); @@ -237,3 +267,27 @@ export const Streaming: Story = { } } }; + +export const ImageLoading: Story = { + args: { + content: [ + '### Quarterly revenue', + '', + 'Revenue increased across the quarter, led by recurring notebook subscriptions.', + '', + `![Quarterly revenue chart](${imageFixtureUrl} "Quarterly revenue")`, + '', + 'The regional breakdown is shown in the chart accompanying the documentation:', + '', + `[Regional comparison: **revenue** ![Regional revenue chart](${imageFixtureUrl}${imageQuerySeparator}region=north "Regional revenue")](https://zeppelin.apache.org/docs/latest/)`, + '', + 'Northern Europe contributed the largest share.' + ].join('\n') + } +}; + +export const StreamingImages: Story = { + render: args => , + args: { content: streamingImageChunks[0] }, + argTypes: { content: { control: false } } +}; diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/MarkdownAnswer.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/MarkdownAnswer.tsx index b5c69baf096..b76629850ac 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/MarkdownAnswer.tsx +++ b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/MarkdownAnswer.tsx @@ -15,7 +15,15 @@ * limitations under the License. */ -import { createContext, useContext, useId, useMemo, type ComponentPropsWithoutRef, type MouseEvent } from 'react'; +import { + createContext, + useContext, + useId, + useMemo, + useState, + type ComponentPropsWithoutRef, + type MouseEvent +} from 'react'; import Markdown, { type Components, type ExtraProps } from 'react-markdown'; import remarkGfm from 'remark-gfm'; import type { Link, Parent, Root, Text } from 'mdast'; @@ -96,17 +104,75 @@ const remarkPlugins: PluggableList = [remarkGfm, remarkParagraphReferences]; // remark-rehype gives the footnotes heading this fixed id and points every footnote reference at it. const footnoteLabelId = 'footnote-label'; -const AnswerLink = ({ node: _node, href, children, title, ...props }: ComponentPropsWithoutRef<'a'> & ExtraProps) => { +const resolveImageUrl = (src: ComponentPropsWithoutRef<'img'>['src'], base: string | undefined): URL | undefined => { + if (typeof src !== 'string' || !src.trim()) return undefined; + try { + const url = base === undefined ? new URL(src) : new URL(src, base); + return url.protocol === 'http:' || url.protocol === 'https:' ? url : undefined; + } catch { + return undefined; + } +}; + +const LoadableImage = ({ url, alt, title }: { url: URL; alt: string; title: string | undefined }) => { + const [approved, setApproved] = useState(false); + const detailsId = useId(); + + return ( + + Image description: {alt || 'No description provided.'} + + Host: {url.hostname} + Image URL: {url.href} + Loading this image sends a request to its host, which may contain information from the answer. + + {approved ? ( + {alt} + ) : ( + + )} + + ); +}; + +const AnswerImage = ({ src, alt, title }: ComponentPropsWithoutRef<'img'> & ExtraProps) => { + const base = typeof document === 'undefined' ? undefined : document.baseURI; + const url = useMemo(() => resolveImageUrl(src, base), [src, base]); + if (!url) { + return ( + + Image description: {alt || 'No description provided.'} + Image unavailable: only HTTP(S) image URLs can be loaded. + + ); + } + // A changed full URL remounts local consent, including when a stream returns to an earlier URL. + return ; +}; + +const containsImage = (node: NonNullable): boolean => + node.tagName === 'img' || node.children.some(child => child.type === 'element' && containsImage(child)); + +const AnswerLink = ({ node, href, children, title, ...props }: ComponentPropsWithoutRef<'a'> & ExtraProps) => { const { describeParagraph, onOpenParagraph, footnotePrefix } = useContext(AnswerContext); if (!href) return <>{children}; + const imageLink = node ? containsImage(node) : false; const fragment = href.startsWith('#'); const paragraphId = fragment ? href.slice(1) : undefined; if (paragraphId && paragraphIdPattern.test(paragraphId)) { const description = describeParagraph?.(paragraphId); if (description && onOpenParagraph) { - return ( + const reference = ( - )} - - ); -}; - -const AnswerImage = ({ src, alt, title }: ComponentPropsWithoutRef<'img'> & ExtraProps) => { - const base = typeof document === 'undefined' ? undefined : document.baseURI; - const url = useMemo(() => resolveImageUrl(src, base), [src, base]); - if (!url) { - return ( - - Image description: {alt || 'No description provided.'} - Image unavailable: only HTTP(S) image URLs can be loaded. - - ); - } - // A changed full URL remounts local consent, including when a stream returns to an earlier URL. - return ; -}; - const containsImage = (node: NonNullable): boolean => node.tagName === 'img' || node.children.some(child => child.type === 'element' && containsImage(child)); @@ -169,7 +56,7 @@ const AnswerLink = ({ node, href, children, title, ...props }: ComponentPropsWit const fragment = href.startsWith('#'); const paragraphId = fragment ? href.slice(1) : undefined; - if (paragraphId && paragraphIdPattern.test(paragraphId)) { + if (paragraphId && isParagraphId(paragraphId)) { const description = describeParagraph?.(paragraphId); if (description && onOpenParagraph) { const reference = ( @@ -219,7 +106,7 @@ const AnswerLink = ({ node, href, children, title, ...props }: ComponentPropsWit target={fragment ? undefined : '_blank'} rel={fragment ? undefined : 'noopener noreferrer'} > - {imageLink ? 'Open image link' : children} + {imageLink ? 'Open linked page' : children} ); // Keep every original child outside the anchor so an image's native load button is never nested in a link. @@ -266,7 +153,7 @@ const AnswerHeading = ({ node: _node, id, ...props }: ComponentPropsWithoutRef<' const components: Components = { h2: AnswerHeading, a: AnswerLink, - img: AnswerImage, + img: MarkdownAnswerImage, pre: AnswerCodeBlock, table: AnswerTable }; @@ -295,7 +182,9 @@ export const MarkdownAnswer = ({ content, describeParagraph, onOpenParagraph }: return ( -
{markdown}
+ +
{markdown}
+ ); }; diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/MarkdownAnswerImage.tsx b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/MarkdownAnswerImage.tsx new file mode 100644 index 00000000000..1c4d4ab65e9 --- /dev/null +++ b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/MarkdownAnswerImage.tsx @@ -0,0 +1,186 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + createContext, + useContext, + useId, + useEffect, + useMemo, + useReducer, + type Dispatch, + type ComponentPropsWithoutRef, + type ReactNode +} from 'react'; +import type { ExtraProps } from 'react-markdown'; + +import * as styles from './MarkdownAnswer.css'; + +type ImageApproval = Readonly<{ url: string; approved: boolean; owner: string | undefined }>; +type ImageApprovals = ReadonlyMap; +type ImageApprovalAction = + | { type: 'mount' | 'approve'; occurrence: string; url: string; owner: string } + | { type: 'unmount'; occurrence: string; owner: string } + | { type: 'prune' }; + +const imageApprovalsReducer = (state: ImageApprovals, action: ImageApprovalAction): ImageApprovals => { + if (action.type === 'prune') { + let next: Map | undefined; + state.forEach((approval, occurrence) => { + if (approval.owner !== undefined) return; + next ??= new Map(state); + next.delete(occurrence); + }); + return next ?? state; + } + + const previous = state.get(action.occurrence); + let approval: ImageApproval; + switch (action.type) { + case 'mount': + if (previous?.url === action.url && previous.owner === action.owner) return state; + approval = { + url: action.url, + approved: previous?.url === action.url ? previous.approved : false, + owner: action.owner + }; + break; + case 'approve': + if (previous?.url === action.url && previous.owner === action.owner && previous.approved) return state; + approval = { url: action.url, approved: true, owner: action.owner }; + break; + case 'unmount': + if (previous?.owner !== action.owner) return state; + approval = { ...previous, owner: undefined }; + break; + } + const next = new Map(state); + next.set(action.occurrence, approval); + return next; +}; + +// Committed consent belongs to a Markdown source occurrence, not a renderer's tag-ordinal key. +const ImageApprovalsContext = createContext< + { imageApprovals: ImageApprovals; dispatch: Dispatch } | undefined +>(undefined); + +export const MarkdownAnswerImageScope = ({ children }: { children: ReactNode }) => { + const [imageApprovals, dispatch] = useReducer( + imageApprovalsReducer, + undefined, + () => new Map() + ); + let hasUnmountedImage = false; + imageApprovals.forEach(approval => { + if (approval.owner === undefined) hasUnmountedImage = true; + }); + // Same-batch rekeys reclaim their grant; prune real removals before children can read stale consent. + if (hasUnmountedImage) dispatch({ type: 'prune' }); + const context = useMemo(() => ({ imageApprovals, dispatch }), [imageApprovals, dispatch]); + return {children}; +}; + +const resolveImageUrl = (src: ComponentPropsWithoutRef<'img'>['src'], base: string | undefined): URL | undefined => { + if (typeof src !== 'string' || !src.trim()) return undefined; + try { + const url = base === undefined ? new URL(src) : new URL(src, base); + return url.protocol === 'http:' || url.protocol === 'https:' ? url : undefined; + } catch { + return undefined; + } +}; + +const useImageApproval = (occurrence: string | undefined, href: string, owner: string) => { + const context = useContext(ImageApprovalsContext); + if (context === undefined) throw new Error('MarkdownAnswerImage must be rendered within MarkdownAnswerImageScope.'); + const { imageApprovals, dispatch } = context; + const occurrenceKey = occurrence ?? owner; + const approval = imageApprovals.get(occurrenceKey); + + useEffect(() => { + dispatch({ type: 'mount', occurrence: occurrenceKey, url: href, owner }); + return () => { + dispatch({ type: 'unmount', occurrence: occurrenceKey, owner }); + }; + }, [dispatch, occurrenceKey, href, owner]); + + return { + approved: approval?.url === href && approval.approved, + approve: () => dispatch({ type: 'approve', occurrence: occurrenceKey, url: href, owner }) + }; +}; + +type LoadableImageProps = { + url: URL; + alt: string; + title: string | undefined; + occurrence: string | undefined; +}; + +const LoadableImage = ({ url, alt, title, occurrence }: LoadableImageProps) => { + const detailsId = useId(); + const { approved, approve } = useImageApproval(occurrence, url.href, detailsId); + + return ( + + Image description: {alt || 'No description provided.'} + + Host: {url.hostname} + Image URL: {url.href} + + Loading this image sends a request to this host. The request may include information from the answer. + + + {approved ? ( + {alt} + ) : ( + + )} + + ); +}; + +export const MarkdownAnswerImage = ({ node, src, alt, title }: ComponentPropsWithoutRef<'img'> & ExtraProps) => { + const base = typeof document === 'undefined' ? undefined : document.baseURI; + const url = useMemo(() => resolveImageUrl(src, base), [src, base]); + const start = node?.position?.start.offset; + const end = node?.position?.end.offset; + // Reference definitions can acquire a title later without changing the image occurrence. + const occurrence = useMemo( + () => (start === undefined || end === undefined ? undefined : JSON.stringify([start, end, alt])), + [start, end, alt] + ); + if (!url) { + return ( + + Image description: {alt || 'No description provided.'} + Image unavailable: only HTTP(S) image URLs can be loaded. + + ); + } + return ( + + ); +}; diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/remarkParagraphReferences.ts b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/remarkParagraphReferences.ts new file mode 100644 index 00000000000..65c96c3152b --- /dev/null +++ b/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/remarkParagraphReferences.ts @@ -0,0 +1,74 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import type { Link, Parent, Root, Text } from 'mdast'; +import type { Plugin } from 'unified'; + +const paragraphIdPattern = /^paragraph_\d+_-?\d+$/; + +export const isParagraphId = (value: string): boolean => paragraphIdPattern.test(value); + +const textReferences = (node: Text): Array | undefined => { + // Match a complete ID without splitting Unicode letters, numbers or combining marks. + const pattern = new RegExp('(^|[^\\p{L}\\p{N}\\p{M}_+-])(paragraph_\\d+_-?\\d+)(?![\\p{L}\\p{N}\\p{M}_+-])', 'gu'); + let references: Array | undefined; + let offset = 0; + let match: RegExpExecArray | null; + + while ((match = pattern.exec(node.value)) !== null) { + const id = match[2]; + const start = match.index + match[1].length; + references ??= []; + if (start > offset) { + references.push({ type: 'text', value: node.value.slice(offset, start) }); + } + references.push({ type: 'link', url: `#${id}`, children: [{ type: 'text', value: id }] }); + offset = start + id.length; + } + + if (references && offset < node.value.length) { + references.push({ type: 'text', value: node.value.slice(offset) }); + } + return references; +}; + +const transformReferences = (parent: Parent): void => { + let children: Parent['children'] | undefined; + + for (let index = 0; index < parent.children.length; index++) { + const child = parent.children[index]; + let references: Array | undefined; + if (child.type === 'text') { + references = textReferences(child); + } else if (child.type === 'inlineCode' && isParagraphId(child.value)) { + references = [{ type: 'link', url: `#${child.value}`, children: [child] }]; + } else if (child.type !== 'link' && child.type !== 'linkReference' && 'children' in child) { + transformReferences(child); + } + + if (references) { + children ??= parent.children.slice(0, index); + children.push(...references); + } else { + children?.push(child); + } + } + + if (children) parent.children = children; +}; + +export const remarkParagraphReferences: Plugin<[], Root> = () => transformReferences; diff --git a/zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/MarkdownAnswer.image.svg b/zeppelin-web-angular/projects/zeppelin-react/test/fixtures/markdown-answer-image.svg similarity index 100% rename from zeppelin-web-angular/projects/zeppelin-react/src/entities/assistant/ui/MarkdownAnswer.image.svg rename to zeppelin-web-angular/projects/zeppelin-react/test/fixtures/markdown-answer-image.svg diff --git a/zeppelin-web-angular/projects/zeppelin-react/test/storybook/markdown-answer-images.spec.mjs b/zeppelin-web-angular/projects/zeppelin-react/test/storybook/markdown-answer-images.spec.mjs index 8b88224fb95..ba258f7d8cf 100644 --- a/zeppelin-web-angular/projects/zeppelin-react/test/storybook/markdown-answer-images.spec.mjs +++ b/zeppelin-web-angular/projects/zeppelin-react/test/storybook/markdown-answer-images.spec.mjs @@ -18,25 +18,26 @@ import { readFileSync } from 'node:fs'; import { test, expect } from '@playwright/test'; -const fixture = readFileSync(new URL('../../src/entities/assistant/ui/MarkdownAnswer.image.svg', import.meta.url)); -const syntheticSource = 'https://images.example.test/plot.svg?q=SYNTHETIC_NOTEBOOK_VALUE'; +const fixture = readFileSync(new URL('../fixtures/markdown-answer-image.svg', import.meta.url)); +const source = 'https://images.example.test/plot.svg?q=SYNTHETIC_NOTEBOOK_VALUE'; +const storyId = 'assistant-markdownanswer--image-loading'; +const includesText = value => new RegExp(value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')); -async function interceptImages(page) { +async function interceptImages(page, baseURL) { const requests = []; - const isImageDestination = request => { - const url = new URL(request.url()); - return ( - url.hostname === 'images.example.test' || - (url.pathname.includes('MarkdownAnswer.image') && url.pathname.endsWith('.svg')) - ); - }; - // Capture every request type, including fetch, HEAD, preload and navigation. + const origin = new URL(baseURL).origin; + const isFixture = url => url.pathname.includes('markdown-answer-image') && url.pathname.endsWith('.svg'); + const isObserved = url => /^https?:$/.test(url.protocol) && (url.origin !== origin || isFixture(url)); page.on('request', request => { - if (isImageDestination(request)) requests.push(request.url()); + if (isObserved(new URL(request.url()))) requests.push(request.url()); }); await page.route('**/*', async route => { - if (isImageDestination(route.request())) { + const url = new URL(route.request().url()); + if (url.hostname === 'images.example.test' || isFixture(url)) { await route.fulfill({ status: 200, contentType: 'image/svg+xml', body: fixture }); + } else if (url.origin !== origin) { + // Unexpected external requests fail the assertions without contacting a real server. + await route.abort(); } else { await route.continue(); } @@ -44,117 +45,95 @@ async function interceptImages(page) { return requests; } -async function expectLoadedImage(image, source) { - await expect(image).toHaveAttribute('src', source); - await expect.poll(() => image.evaluate(element => element.complete && element.naturalWidth > 0)).toBe(true); -} - -test('ImageLoading sends no request before keyboard approval and resets approval for a changed query', async ({ - page -}) => { - const requests = await interceptImages(page); - const storyId = 'assistant-markdownanswer--image-loading'; +async function openImages(page) { await page.goto(`/iframe.html?id=${storyId}&viewMode=story`); const root = page.locator('#storybook-root'); await expect(root.getByRole('button', { name: /load image/i }).first()).toBeVisible(); - await page.waitForFunction(() => Boolean(window.__STORYBOOK_PREVIEW__?.channel)); + return root; +} + +async function setContent(page, content) { await page.evaluate( - ({ storyId, source }) => { - window.__STORYBOOK_PREVIEW__.channel.emit('updateStoryArgs', { - storyId, - updatedArgs: { content: `Revenue improved over the quarter.\n\n![Quarterly revenue chart](${source})` } - }); + ({ id, content }) => { + window.__STORYBOOK_PREVIEW__.channel.emit('updateStoryArgs', { storyId: id, updatedArgs: { content } }); }, - { storyId, source: syntheticSource } + { id: storyId, content } ); +} + +async function expectLoadedImage(image, url) { + await expect(image).toHaveAttribute('src', url); + await expect.poll(() => image.evaluate(element => element.complete && element.naturalWidth > 0)).toBe(true); +} - await expect(root.getByText(`Image URL: ${syntheticSource}`, { exact: true })).toBeVisible(); - await expect(root.getByText('Host: images.example.test', { exact: true })).toBeVisible(); - await expect(root.getByText(/loading.*sends a request.*information from.*answer/i)).toBeVisible(); +test('only keyboard-confirmed destinations are requested, including after a query change', async ({ + page, + baseURL +}) => { + const requests = await interceptImages(page, baseURL); + const root = await openImages(page); + await setContent(page, `Revenue improved.\n\n![Quarterly revenue chart](${source})`); + const load = root.getByRole('button', { name: /load image/i }); + await expect(load).toHaveAccessibleDescription(includesText(source)); await expect(root.locator('img, [srcset], link[rel="preload"]')).toHaveCount(0); await page.waitForLoadState('networkidle'); expect(requests).toEqual([]); - const load = root.getByRole('button', { name: /load image/i }); await load.focus(); await load.press('Enter'); - await expectLoadedImage(root.getByRole('img', { name: 'Quarterly revenue chart' }), syntheticSource); - expect(requests).toEqual([syntheticSource]); + await expectLoadedImage(root.getByRole('img', { name: 'Quarterly revenue chart' }), source); + expect(requests).toEqual([source]); - const changedSource = syntheticSource.replace('SYNTHETIC_NOTEBOOK_VALUE', 'UPDATED_NOTEBOOK_VALUE'); - await page.evaluate( - ({ storyId, source }) => { - window.__STORYBOOK_PREVIEW__.channel.emit('updateStoryArgs', { - storyId, - updatedArgs: { - content: `The revised quarter includes the final sales.\n\n![Quarterly revenue chart](${source})` - } - }); - }, - { storyId, source: changedSource } - ); - await expect(root.getByText(`Image URL: ${changedSource}`, { exact: true })).toBeVisible(); + const revised = source.replace('SYNTHETIC_NOTEBOOK_VALUE', 'UPDATED_NOTEBOOK_VALUE'); + await setContent(page, `The revised quarter includes final sales.\n\n![Quarterly revenue chart](${revised})`); + await expect(load).toHaveAccessibleDescription(includesText(revised)); await expect(root.getByRole('img')).toHaveCount(0); - await expect(load).toBeVisible(); await page.waitForLoadState('networkidle'); - expect(requests).toEqual([syntheticSource]); - + expect(requests).toEqual([source]); await load.focus(); await load.press('Space'); - await expectLoadedImage(root.getByRole('img', { name: 'Quarterly revenue chart' }), changedSource); - expect(requests).toEqual([syntheticSource, changedSource]); + await expectLoadedImage(root.getByRole('img', { name: 'Quarterly revenue chart' }), revised); + expect(requests).toEqual([source, revised]); }); -test('StreamingImages preserves unchanged approval and requires fresh approval for A to B to A', async ({ page }) => { - const requests = await interceptImages(page); +test('streaming reference images preserve approval on append and require approval for a changed destination', async ({ + page, + baseURL +}) => { + const requests = await interceptImages(page, baseURL); await page.goto('/iframe.html?id=assistant-markdownanswer--streaming-images&viewMode=story'); const root = page.locator('#storybook-root'); const load = root.getByRole('button', { name: /load image/i }); await expect(load).toBeVisible(); - const sourceLabel = root.getByText(/^Image URL: https?:\/\/.*[?&]version=A$/); - await expect(sourceLabel).toBeVisible(); - const sourceA = (await sourceLabel.textContent()).replace('Image URL: ', ''); - expect(new URL(sourceA).protocol).toMatch(/^https?:$/); + const descriptionId = await load.getAttribute('aria-describedby'); + const disclosed = await page.locator(`[id="${descriptionId}"]`).textContent(); await expect(root.getByRole('img')).toHaveCount(0); await page.waitForLoadState('networkidle'); expect(requests).toEqual([]); - - await load.focus(); await load.press('Enter'); const image = root.getByRole('img', { name: 'Quarterly revenue chart' }); - await expectLoadedImage(image, sourceA); + await expect.poll(() => image.evaluate(element => element.complete && element.naturalWidth > 0)).toBe(true); + const sourceA = await image.getAttribute('src'); + expect(disclosed).toContain(sourceA); expect(requests).toEqual([sourceA]); + const initialProgress = await root.getByRole('status').textContent(); await root.getByRole('button', { name: 'Append next chunk', exact: true }).click(); - await expect(root.getByRole('status')).toHaveText('Chunk 2 of 3'); + await expect(root.getByRole('status')).not.toHaveText(initialProgress); await expectLoadedImage(image, sourceA); await expect(load).toHaveCount(0); await page.waitForLoadState('networkidle'); expect(requests).toEqual([sourceA]); + const revised = new URL(sourceA); + revised.searchParams.set('version', 'B'); + const sourceB = revised.href; await root.getByRole('button', { name: 'Append next chunk', exact: true }).click(); - await expect(root.getByRole('status')).toHaveText('Chunk 3 of 3'); - const sourceB = sourceA.replace('version=A', 'version=B'); - await expect(root.getByText(`Image URL: ${sourceB}`, { exact: true })).toBeVisible(); + await expect(load).toHaveAccessibleDescription(includesText(sourceB)); await expect(image).toHaveCount(0); - await expect(load).toBeVisible(); await page.waitForLoadState('networkidle'); expect(requests).toEqual([sourceA]); - - await load.focus(); await load.press('Space'); await expectLoadedImage(image, sourceB); expect(requests).toEqual([sourceA, sourceB]); - - await root.getByRole('button', { name: 'Restart stream', exact: true }).click(); - await expect(root.getByRole('status')).toHaveText('Chunk 1 of 3'); - await expect(root.getByText(`Image URL: ${sourceA}`, { exact: true })).toBeVisible(); - await expect(image).toHaveCount(0); - await expect(load).toBeVisible(); - await page.waitForLoadState('networkidle'); - expect(requests).toEqual([sourceA, sourceB]); - - await load.focus(); - await load.press('Enter'); - await expectLoadedImage(image, sourceA); });