feat: add image opening functionality to DownloadActionBar by ferencsarai · Pull Request #725 · ethersphere/bee-dashboard

ferencsarai · 2026-03-05T11:40:44Z

This PR adds an Open Image action in the download bar and updates the open logic to resolve and open the actual image file path (instead of only the root BZZ URL). The existing download action remains unchanged, so users can now explicitly choose between opening the image and downloading it.

…components

Copilot

Pull request overview

Adds an explicit Open Image action to the file share download bar and updates the open behavior so image shares can open the concrete image file path (not just the manifest root BZZ URL).

Changes:

Update Share.onOpen() to detect image shares and open a resolved image path under /bzz/:hash/:path.
Extend DownloadActionBar with a new isImage prop and render an Open Image button when applicable.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
`src/pages/files/Share.tsx`	Adds image-aware open logic to resolve an image file path and passes `isImage` into the action bar.
`src/pages/files/DownloadActionBar.tsx`	Adds `isImage` prop and conditionally displays the new Open Image action.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-05T11:47:13Z

src/pages/files/Share.tsx

+      const imagePath = Object.keys(swarmEntries).find(path => /\.(jpg|jpeg|png|gif|webp|svg)$/i.test(path))
+      const candidatePath = imagePath || metadata?.name


For image manifests with multiple image entries, Object.keys(swarmEntries).find(...) will pick the first matching key based on object iteration order, which may not be deterministic and may open an unintended image. Since you already compute indexDocument from the manifest metadata, prefer using indexDocument when it’s an image (or fall back to the single entry when there’s only one file) to make the Open Image target stable and predictable.

Suggested change

const imagePath = Object.keys(swarmEntries).find(path => /\.(jpg|jpeg|png|gif|webp|svg)$/i.test(path))

const candidatePath = imagePath || metadata?.name

const imageExtensions = /\.(jpg|jpeg|png|gif|webp|svg)$/i

let candidatePath: string | undefined

// Prefer indexDocument when it is an image

if (indexDocument && imageExtensions.test(indexDocument)) {

candidatePath = indexDocument

} else {

const entryKeys = Object.keys(swarmEntries)

// If there is exactly one entry and it is an image, use it

if (entryKeys.length === 1 && imageExtensions.test(entryKeys[0])) {

candidatePath = entryKeys[0]

} else {

// Fall back to metadata.name (as before) if available

candidatePath = metadata?.name

}

}

Copilot · 2026-03-05T11:47:13Z

src/pages/files/Share.tsx

+      const normalizedPath = candidatePath?.replace(/^\/+/, '')
+
+      if (normalizedPath) {
+        window.open(`${apiUrl}/bzz/${hash}/${encodeURI(normalizedPath)}`, '_blank')


encodeURI(normalizedPath) won’t encode reserved characters like ? and #, so image filenames containing those characters will produce a broken URL (everything after ? becomes a query string, # becomes a fragment). Consider encoding per path segment (e.g., split on / and encodeURIComponent each segment) so the generated /bzz/:hash/:path URL reliably targets the intended file.

Suggested change

window.open(`${apiUrl}/bzz/${hash}/${encodeURI(normalizedPath)}`, '_blank')

const encodedPath = normalizedPath.split('/').map(segment => encodeURIComponent(segment)).join('/')

window.open(`${apiUrl}/bzz/${hash}/${encodedPath}`, '_blank')

Copilot · 2026-03-05T11:47:13Z

src/pages/files/Share.tsx

+
+      if (normalizedPath) {
+        window.open(`${apiUrl}/bzz/${hash}/${encodeURI(normalizedPath)}`, '_blank')


normalizedPath is used directly in the URL path, but it isn’t validated for dot-segments (e.g. ../). Browsers normalize these, which can change the final request path and potentially drop the intended :hash segment (opening a different BZZ reference than expected). Consider rejecting/stripping . and .. segments before constructing the URL, or ensuring the path is built in a way that prevents dot-segment normalization.

Suggested change

if (normalizedPath) {

window.open(`${apiUrl}/bzz/${hash}/${encodeURI(normalizedPath)}`, '_blank')

const safePath = normalizedPath

? normalizedPath

.split('/')

.filter(segment => segment !== '.' && segment !== '..')

.join('/')

: ''

if (safePath) {

window.open(`${apiUrl}/bzz/${hash}/${encodeURI(safePath)}`, '_blank')

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-05T14:54:24Z

src/utils/file.ts

+  return path
+    .replaceAll('\\', '/')
+    .replace(/^\/+/, '')
+    .replace(/(?:\.\.\/)+|[?#].*/g, '')


sanitizeDisplayPath can return a path starting with / because it strips leading slashes before removing traversal segments. Example: sanitizeDisplayPath('../..//secret/image.png') becomes /secret/image.png, which then produces a double-slash in the constructed bzz URL. Consider re-stripping leading slashes after traversal removal, or (more robustly) normalizing via path segment splitting and removing ./.. segments instead of regex replacement.

Suggested change

return path

.replaceAll('\\', '/')

.replace(/^\/+/, '')

.replace(/(?:\.\.\/)+|[?#].*/g, '')

// Strip query string and fragment first

const withoutQuery = path.split(/[?#]/, 1)[0]

// Normalize separators and remove leading slashes

const normalized = withoutQuery

.replaceAll('\\', '/')

.replace(/^\/+/, '')

const segments = normalized.split('/')

const stack: string[] = []

for (const segment of segments) {

if (!segment || segment === '.') {

continue

}

if (segment === '..') {

if (stack.length > 0) {

stack.pop()

}

continue

}

stack.push(segment)

}

// Joining without a leading slash ensures the result never starts with '/'

return stack.join('/')

src/pages/files/Share.tsx

Copilot · 2026-03-05T14:54:25Z

tests/unit/file.spec.ts

+    expect(sanitizeDisplayPath('/folder/../image.png')).toBe('folder/image.png')
+    expect(sanitizeDisplayPath('..\\secret\\image.png')).toBe('secret/image.png')
+    expect(sanitizeDisplayPath('image.png?download=1#preview')).toBe('image.png')
+    expect(sanitizeDisplayPath('/nested/path/image.png')).toBe('nested/path/image.png')


The new sanitizeDisplayPath tests cover basic traversal removal, but they don’t include the case where traversal stripping reintroduces a leading slash (e.g. '../..//secret/image.png'). Adding a regression test for that scenario would prevent the current bug in sanitizeDisplayPath from slipping through.

Suggested change

expect(sanitizeDisplayPath('/nested/path/image.png')).toBe('nested/path/image.png')

expect(sanitizeDisplayPath('/nested/path/image.png')).toBe('nested/path/image.png')

expect(sanitizeDisplayPath('../..//secret/image.png')).toBe('secret/image.png')

I updated this to always open /bzz/${hash} only.
Bee treats the hash as a manifest, so root path resolves the index document for single-file uploads. This avoids guessing filenames (and potential 404s) and keeps noopener,noreferrer for safety.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-05T15:07:12Z

src/utils/file.ts

+  return path
+    .replaceAll('\\', '/')
+    .replace(/^\/+/, '')
+    .replace(/(?:\.\.\/)+|[?#].*/g, '')
+    .replace(/^\/+/, '')


sanitizeDisplayPath doesn’t fully neutralize path traversal: inputs like ".." or "../.." end up returning ".." (because the regex only strips "../"), which can then be used in URLs and be normalized by the browser to navigate outside the intended /bzz// prefix. Consider implementing dot-segment normalization (split on "/", drop "." segments, clamp ".." segments so they can’t escape root) and stripping query/fragment before normalization.

Suggested change

return path

.replaceAll('\\', '/')

.replace(/^\/+/, '')

.replace(/(?:\.\.\/)+|[?#].*/g, '')

.replace(/^\/+/, '')

// Normalize slashes

let normalized = path.replaceAll('\\', '/')

// Strip query and fragment components

const queryIndex = normalized.search(/[?#]/)

if (queryIndex !== -1) {

normalized = normalized.slice(0, queryIndex)

}

// Remove leading slashes to keep the path relative

normalized = normalized.replace(/^\/+/, '')

// Dot-segment normalization

const segments = normalized.split('/')

const result: string[] = []

for (const segment of segments) {

if (!segment || segment === '.') {

continue

}

if (segment === '..') {

if (result.length > 0) {

result.pop()

}

// If there is nothing to pop, clamp at root by ignoring extra ".."

continue

}

result.push(segment)

}

return result.join('/')

Copilot · 2026-03-05T15:07:13Z

tests/unit/file.spec.ts

+    expect(sanitizeDisplayPath('..\\secret\\image.png')).toBe('secret/image.png')
+    expect(sanitizeDisplayPath('../..//secret/image.png')).toBe('secret/image.png')
+    expect(sanitizeDisplayPath('image.png?download=1#preview')).toBe('image.png')
+    expect(sanitizeDisplayPath('/nested/path/image.png')).toBe('nested/path/image.png')


The sanitizeDisplayPath unit test suite doesn’t cover edge cases where the input is a bare dot-segment (e.g. "..", ".", "../.."), which currently can survive sanitization and be normalized by the browser when used in a URL. Adding assertions for these cases would help prevent regressions once the sanitization logic is hardened.

Suggested change

expect(sanitizeDisplayPath('/nested/path/image.png')).toBe('nested/path/image.png')

expect(sanitizeDisplayPath('/nested/path/image.png')).toBe('nested/path/image.png')

expect(sanitizeDisplayPath('.')).toBe('')

expect(sanitizeDisplayPath('..')).toBe('')

expect(sanitizeDisplayPath('../..')).toBe('')

feat: add image opening functionality to DownloadActionBar and Share …

42e1bb5

…components

ferencsarai requested a review from Cafe137 as a code owner March 5, 2026 11:40

ferencsarai linked an issue Mar 5, 2026 that may be closed by this pull request

Files / Download page does not have a real Download button (Download button opens a file, not downloads) ethersphere/swarm-desktop#510

Closed

ferencsarai requested a review from Copilot March 5, 2026 11:43

Copilot started reviewing on behalf of ferencsarai March 5, 2026 11:44 View session

Copilot AI reviewed Mar 5, 2026

View reviewed changes

fix: add sanitizeDisplayPath utility function and tests

9c5fa80

ferencsarai requested a review from Copilot March 5, 2026 14:49

Copilot started reviewing on behalf of ferencsarai March 5, 2026 14:49 View session

Copilot AI reviewed Mar 5, 2026

View reviewed changes

ferencsarai and others added 2 commits March 5, 2026 15:57

fix: add noopener, noreferrer to window.open

a07b2cd

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

fix: enhance sanitizeDisplayPath to handle multiple leading slashes

6676a3e

ferencsarai requested a review from Copilot March 5, 2026 15:02

Copilot started reviewing on behalf of ferencsarai March 5, 2026 15:03 View session

Copilot AI reviewed Mar 5, 2026

View reviewed changes

fix: remove unused sanitizeDisplayPath function and related tests

31167f7

ferencsarai merged commit 3ff645c into master Mar 5, 2026
2 checks passed

ferencsarai deleted the 510-files-download-page-does-not-have-a-real-download-button-download-button-opens-a-file-not-downloads branch March 5, 2026 15:51

bee-worker mentioned this pull request Mar 5, 2026

chore(master): release 0.34.0 #723

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add image opening functionality to DownloadActionBar#725

feat: add image opening functionality to DownloadActionBar#725
ferencsarai merged 5 commits intomasterfrom
510-files-download-page-does-not-have-a-real-download-button-download-button-opens-a-file-not-downloads

ferencsarai commented Mar 5, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 5, 2026

Uh oh!

Copilot AI Mar 5, 2026

Uh oh!

Copilot AI Mar 5, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 5, 2026

Uh oh!

Uh oh!

Copilot AI Mar 5, 2026

Uh oh!

ferencsarai Mar 5, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 5, 2026

Uh oh!

Copilot AI Mar 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		const imagePath = Object.keys(swarmEntries).find(path => /\.(jpg\|jpeg\|png\|gif\|webp\|svg)$/i.test(path))
		const candidatePath = imagePath \|\| metadata?.name

-      const imagePath = Object.keys(swarmEntries).find(path => /\.(jpg|jpeg|png|gif|webp|svg)$/i.test(path))
-      const candidatePath = imagePath || metadata?.name
+      const imageExtensions = /\.(jpg|jpeg|png|gif|webp|svg)$/i
+      let candidatePath: string | undefined
+      // Prefer indexDocument when it is an image
+      if (indexDocument && imageExtensions.test(indexDocument)) {
+        candidatePath = indexDocument
+      } else {
+        const entryKeys = Object.keys(swarmEntries)
+        // If there is exactly one entry and it is an image, use it
+        if (entryKeys.length === 1 && imageExtensions.test(entryKeys[0])) {
+          candidatePath = entryKeys[0]
+        } else {
+          // Fall back to metadata.name (as before) if available
+          candidatePath = metadata?.name
+        }
+      }

	window.open(`${apiUrl}/bzz/${hash}/${encodeURI(normalizedPath)}`, '_blank')
	const encodedPath = normalizedPath.split('/').map(segment => encodeURIComponent(segment)).join('/')
	window.open(`${apiUrl}/bzz/${hash}/${encodedPath}`, '_blank')


		if (normalizedPath) {
		window.open(`${apiUrl}/bzz/${hash}/${encodeURI(normalizedPath)}`, '_blank')

-      if (normalizedPath) {
-        window.open(`${apiUrl}/bzz/${hash}/${encodeURI(normalizedPath)}`, '_blank')
+      const safePath = normalizedPath
+        ? normalizedPath
+            .split('/')
+            .filter(segment => segment !== '.' && segment !== '..')
+            .join('/')
+        : ''
+      if (safePath) {
+        window.open(`${apiUrl}/bzz/${hash}/${encodeURI(safePath)}`, '_blank')

-  return path
-    .replaceAll('\\', '/')
-    .replace(/^\/+/, '')
-    .replace(/(?:\.\.\/)+|[?#].*/g, '')
+  // Strip query string and fragment first
+  const withoutQuery = path.split(/[?#]/, 1)[0]
+  // Normalize separators and remove leading slashes
+  const normalized = withoutQuery
+    .replaceAll('\\', '/')
+    .replace(/^\/+/, '')
+  const segments = normalized.split('/')
+  const stack: string[] = []
+  for (const segment of segments) {
+    if (!segment || segment === '.') {
+      continue
+    }
+    if (segment === '..') {
+      if (stack.length > 0) {
+        stack.pop()
+      }
+      continue
+    }
+    stack.push(segment)
+  }
+  // Joining without a leading slash ensures the result never starts with '/'
+  return stack.join('/')

	expect(sanitizeDisplayPath('/nested/path/image.png')).toBe('nested/path/image.png')
	expect(sanitizeDisplayPath('/nested/path/image.png')).toBe('nested/path/image.png')
	expect(sanitizeDisplayPath('../..//secret/image.png')).toBe('secret/image.png')

-  return path
-    .replaceAll('\\', '/')
-    .replace(/^\/+/, '')
-    .replace(/(?:\.\.\/)+|[?#].*/g, '')
-    .replace(/^\/+/, '')
+  // Normalize slashes
+  let normalized = path.replaceAll('\\', '/')
+  // Strip query and fragment components
+  const queryIndex = normalized.search(/[?#]/)
+  if (queryIndex !== -1) {
+    normalized = normalized.slice(0, queryIndex)
+  }
+  // Remove leading slashes to keep the path relative
+  normalized = normalized.replace(/^\/+/, '')
+  // Dot-segment normalization
+  const segments = normalized.split('/')
+  const result: string[] = []
+  for (const segment of segments) {
+    if (!segment || segment === '.') {
+      continue
+    }
+    if (segment === '..') {
+      if (result.length > 0) {
+        result.pop()
+      }
+      // If there is nothing to pop, clamp at root by ignoring extra ".."
+      continue
+    }
+    result.push(segment)
+  }
+  return result.join('/')

-    expect(sanitizeDisplayPath('/nested/path/image.png')).toBe('nested/path/image.png')
+    expect(sanitizeDisplayPath('/nested/path/image.png')).toBe('nested/path/image.png')
+    expect(sanitizeDisplayPath('.')).toBe('')
+    expect(sanitizeDisplayPath('..')).toBe('')
+    expect(sanitizeDisplayPath('../..')).toBe('')

Conversation

ferencsarai commented Mar 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

ferencsarai Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants