-
Notifications
You must be signed in to change notification settings - Fork 1
feat(api): add directory manifest support for folder uploads #81
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
base: main
Are you sure you want to change the base?
Conversation
This leads to problems serializing TOML, see also: status-im/nim-serialization#104
Implements directory manifests for organizing multiple files into a tree:
- New DirectoryCodec (0xCD04) for directory manifest CIDs
- DirectoryManifest type with entries, totalSize, filesCount
- Two-phase upload: files uploaded individually, then /directory endpoint
finalizes them into a tree structure with POST JSON body
- HTML directory browsing with Archivist branding at /data/{cid}
- JSON directory listing when Accept header doesn't include text/html
- Path resolution within directories via /data/{cid}/path?p=subdir/file
- Auto-promotion of single-child root directories
API changes:
- POST /api/archivist/v1/directory - finalize directory from uploaded files
- GET /api/archivist/v1/data/{cid} - now serves HTML for directories
- Added MIME types: audio/mpeg, audio/flac, video/webm, etc.
- Relaxed filename validation to allow paths like "Album/track.mp3"
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
nph
[nph] reported by reviewdog 🐶
archivist-node/archivist/rest/api.nim
Line 569 in 4688eab
| return RestApiResponse.response($json, contentType = "application/json", headers = headers) |
[nph] reported by reviewdog 🐶
archivist-node/archivist/rest/api.nim
Line 682 in 4688eab
| let pathParts = if pathStr.len > 0: pathStr.split('/') else: @[] |
[nph] reported by reviewdog 🐶
archivist-node/archivist/rest/api.nim
Lines 686 to 688 in 4688eab
| return RestApiResponse.redirect( | |
| Http307, "/api/archivist/v1/data/" & $cidVal | |
| ) |
[nph] reported by reviewdog 🐶
archivist-node/archivist/rest/api.nim
Lines 712 to 714 in 4688eab
| return RestApiResponse.error( | |
| Http404, "Path not found: " & part, headers = headers | |
| ) |
[nph] reported by reviewdog 🐶
archivist-node/archivist/rest/api.nim
Line 739 in 4688eab
| without subDir =? (await fetchDirectoryManifest(node.networkStore, foundEntry.cid)), err: |
[nph] reported by reviewdog 🐶
| const ArchivistLogoSvg* = """<svg version="1.0" xmlns="http://www.w3.org/2000/svg" width="32" height="32" viewBox="0 0 400 400" preserveAspectRatio="xMidYMid meet"> |
[nph] reported by reviewdog 🐶
| const DirectoryListingCss* = """ |
[nph] reported by reviewdog 🐶
| mime == "application/gzip" or mime == "application/x-7z-compressed": |
[nph] reported by reviewdog 🐶
| dirName = if directory.name.len > 0: directory.name else: cidStr[0 ..< 12] & "..." |
[nph] reported by reviewdog 🐶
| pathParts = if basePath.len > 0: basePath.strip(chars = {'/'}).split('/') else: @[] |
[nph] reported by reviewdog 🐶
| var html = fmt"""<!DOCTYPE html> |
[nph] reported by reviewdog 🐶
| html &= fmt""" <span class="separator">/</span> |
[nph] reported by reviewdog 🐶
| html &= fmt""" </nav> |
[nph] reported by reviewdog 🐶
| html &= fmt""" <div class="file-row"> |
[nph] reported by reviewdog 🐶
| let parentPath = if pathParts.len > 1: |
[nph] reported by reviewdog 🐶
| html &= fmt""" <div class="file-row"> |
[nph] reported by reviewdog 🐶
| html &= """ <div class="empty-dir">This directory is empty</div> |
[nph] reported by reviewdog 🐶
archivist-node/archivist/rest/directoryhtml.nim
Lines 463 to 465 in 4688eab
| displayName = if entry.isDirectory: entryName & "/" else: entryName | |
| html &= fmt""" <div class="file-row"> |
[nph] reported by reviewdog 🐶
| html &= fmt""" </div> |
| ProtoBuffer, initProtoBuffer, getField, getRequiredRepeatedField, | ||
| write, finish, ProtoResult |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| ProtoBuffer, initProtoBuffer, getField, getRequiredRepeatedField, | |
| write, finish, ProtoResult | |
| ProtoBuffer, initProtoBuffer, getField, getRequiredRepeatedField, write, finish, | |
| ProtoResult |
| entries.add(DirectoryEntry( | ||
| name: entryName, | ||
| cid: entryCid, | ||
| size: size.NBytes, | ||
| isDirectory: isDir != 0, | ||
| mimetype: mimetype, | ||
| )) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| entries.add(DirectoryEntry( | |
| name: entryName, | |
| cid: entryCid, | |
| size: size.NBytes, | |
| isDirectory: isDir != 0, | |
| mimetype: mimetype, | |
| )) | |
| entries.add( | |
| DirectoryEntry( | |
| name: entryName, | |
| cid: entryCid, | |
| size: size.NBytes, | |
| isDirectory: isDir != 0, | |
| mimetype: mimetype, | |
| ) | |
| ) |
| success DirectoryManifest( | ||
| entries: entries, | ||
| totalSize: totalSize.NBytes, | ||
| name: name, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| success DirectoryManifest( | |
| entries: entries, | |
| totalSize: totalSize.NBytes, | |
| name: name, | |
| ) | |
| success DirectoryManifest(entries: entries, totalSize: totalSize.NBytes, name: name) |
| cid = blk.cid, | ||
| entries = directory.entries.len, | ||
| totalSize = directory.totalSize |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| cid = blk.cid, | |
| entries = directory.entries.len, | |
| totalSize = directory.totalSize | |
| cid = blk.cid, entries = directory.entries.len, totalSize = directory.totalSize |
| cid*: Cid | ||
| size*: NBytes | ||
| isDirectory*: bool | ||
| mimetype*: string # Empty string = not set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| mimetype*: string # Empty string = not set | |
| mimetype*: string # Empty string = not set |
| return RestApiResponse.error( | ||
| Http400, "Invalid JSON body", headers = headers | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| return RestApiResponse.error( | |
| Http400, "Invalid JSON body", headers = headers | |
| ) | |
| return RestApiResponse.error(Http400, "Invalid JSON body", headers = headers) |
| return RestApiResponse.error( | ||
| Http400, "'entries' must be an array", headers = headers | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| return RestApiResponse.error( | |
| Http400, "'entries' must be an array", headers = headers | |
| ) | |
| return | |
| RestApiResponse.error(Http400, "'entries' must be an array", headers = headers) |
| return RestApiResponse.error( | ||
| Http400, "No entries provided", headers = headers | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| return RestApiResponse.error( | |
| Http400, "No entries provided", headers = headers | |
| ) | |
| return RestApiResponse.error(Http400, "No entries provided", headers = headers) |
|
|
||
| if not entry.hasKey("path") or not entry.hasKey("cid"): | ||
| return RestApiResponse.error( | ||
| Http400, "Entry " & $i & " missing required 'path' or 'cid'", headers = headers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| Http400, "Entry " & $i & " missing required 'path' or 'cid'", headers = headers | |
| Http400, | |
| "Entry " & $i & " missing required 'path' or 'cid'", | |
| headers = headers, |
| # Validate and normalize path | ||
| var normalPath = pathStr.replace("\\", "/") | ||
| while normalPath.len > 0 and normalPath[0] == '/': | ||
| normalPath = normalPath[1..^1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| normalPath = normalPath[1..^1] | |
| normalPath = normalPath[1 ..^ 1] |
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
| import ../manifest/directory | ||
| import ../units | ||
|
|
||
| const ArchivistLogoSvg* = """<svg version="1.0" xmlns="http://www.w3.org/2000/svg" width="32" height="32" viewBox="0 0 400 400" preserveAspectRatio="xMidYMid meet"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| const ArchivistLogoSvg* = """<svg version="1.0" xmlns="http://www.w3.org/2000/svg" width="32" height="32" viewBox="0 0 400 400" preserveAspectRatio="xMidYMid meet"> | |
| const ArchivistLogoSvg* = | |
| """<svg version="1.0" xmlns="http://www.w3.org/2000/svg" width="32" height="32" viewBox="0 0 400 400" preserveAspectRatio="xMidYMid meet"> |
| </g> | ||
| </svg>""" | ||
|
|
||
| const DirectoryListingCss* = """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| const DirectoryListingCss* = """ | |
| const DirectoryListingCss* = | |
| """ |
| elif mime == "application/pdf": | ||
| return "📕" # book icon | ||
| elif mime == "application/zip" or mime == "application/x-tar" or | ||
| mime == "application/gzip" or mime == "application/x-7z-compressed": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| mime == "application/gzip" or mime == "application/x-7z-compressed": | |
| mime == "application/gzip" or mime == "application/x-7z-compressed": |
|
|
||
| let | ||
| cidStr = $dirCid | ||
| dirName = if directory.name.len > 0: directory.name else: cidStr[0 ..< 12] & "..." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| dirName = if directory.name.len > 0: directory.name else: cidStr[0 ..< 12] & "..." | |
| dirName = | |
| if directory.name.len > 0: | |
| directory.name | |
| else: | |
| cidStr[0 ..< 12] & "..." |
| cidStr = $dirCid | ||
| dirName = if directory.name.len > 0: directory.name else: cidStr[0 ..< 12] & "..." | ||
| escapedDirName = escapeHtml(dirName) | ||
| pathParts = if basePath.len > 0: basePath.strip(chars = {'/'}).split('/') else: @[] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| pathParts = if basePath.len > 0: basePath.strip(chars = {'/'}).split('/') else: @[] | |
| pathParts = | |
| if basePath.len > 0: | |
| basePath.strip(chars = {'/'}).split('/') | |
| else: | |
| @[] |
| """ | ||
| elif basePath.len > 0: | ||
| # Parent is same directory, just go up one path level | ||
| let parentPath = if pathParts.len > 1: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| let parentPath = if pathParts.len > 1: | |
| let parentPath = | |
| if pathParts.len > 1: |
| "/" & pathParts[0 ..< ^1].join("/") | ||
| else: | ||
| "" | ||
| html &= fmt""" <div class="file-row"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| html &= fmt""" <div class="file-row"> | |
| html &= | |
| fmt""" <div class="file-row"> |
| let sortedEntries = directory.sortedEntries() | ||
|
|
||
| if sortedEntries.len == 0: | ||
| html &= """ <div class="empty-dir">This directory is empty</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| html &= """ <div class="empty-dir">This directory is empty</div> | |
| html &= | |
| """ <div class="empty-dir">This directory is empty</div> |
| displayName = if entry.isDirectory: entryName & "/" else: entryName | ||
|
|
||
| html &= fmt""" <div class="file-row"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| displayName = if entry.isDirectory: entryName & "/" else: entryName | |
| html &= fmt""" <div class="file-row"> | |
| displayName = | |
| if entry.isDirectory: | |
| entryName & "/" | |
| else: | |
| entryName | |
| html &= | |
| fmt""" <div class="file-row"> |
| </div> | ||
| """ | ||
|
|
||
| html &= fmt""" </div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| html &= fmt""" </div> | |
| html &= | |
| fmt""" </div> |
- Add HEAD endpoint for /api/archivist/v1/data/{cid} to support
metadata preflight checks (content-type, size) without full download
- Add HEAD endpoint for /api/archivist/v1/data/{cid}/network/stream
to support jsmediatags ID3 tag reading
- Fix directory traversal check to allow ".." in filenames (e.g.
"F.R.E.S.H..mp3") while still blocking actual traversal attacks
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
| for part in pathParts: | ||
| if part == "..": | ||
| return RestApiResponse.error( | ||
| Http400, "Invalid path (directory traversal not allowed): " & pathStr, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| Http400, "Invalid path (directory traversal not allowed): " & pathStr, | |
| Http400, | |
| "Invalid path (directory traversal not allowed): " & pathStr, |
| inputEntries.add(InputEntry( | ||
| path: normalPath, | ||
| cid: cidVal, | ||
| size: size, | ||
| mimetype: mimetypeOpt, | ||
| )) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| inputEntries.add(InputEntry( | |
| path: normalPath, | |
| cid: cidVal, | |
| size: size, | |
| mimetype: mimetypeOpt, | |
| )) | |
| inputEntries.add( | |
| InputEntry(path: normalPath, cid: cidVal, size: size, mimetype: mimetypeOpt) | |
| ) |
| current.subdirs[part] = DirNode( | ||
| name: part, | ||
| subdirs: initTable[string, DirNode](), | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| current.subdirs[part] = DirNode( | |
| name: part, | |
| subdirs: initTable[string, DirNode](), | |
| ) | |
| current.subdirs[part] = | |
| DirNode(name: part, subdirs: initTable[string, DirNode]()) |
| current.files.add(( | ||
| name: pathParts[^1], | ||
| cid: entry.cid, | ||
| size: entry.size, | ||
| mimetype: entry.mimetype, | ||
| )) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| current.files.add(( | |
| name: pathParts[^1], | |
| cid: entry.cid, | |
| size: entry.size, | |
| mimetype: entry.mimetype, | |
| )) | |
| current.files.add( | |
| ( | |
| name: pathParts[^1], | |
| cid: entry.cid, | |
| size: entry.size, | |
| mimetype: entry.mimetype, | |
| ) | |
| ) |
| without subdirManifest =? ( | ||
| await fetchDirectoryManifest(node.networkStore, subdirCid) | ||
| ), err: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| without subdirManifest =? ( | |
| await fetchDirectoryManifest(node.networkStore, subdirCid) | |
| ), err: | |
| without subdirManifest =? | |
| (await fetchDirectoryManifest(node.networkStore, subdirCid)), err: |
| json["name"] = %directory.name | ||
| json["totalSize"] = %(directory.totalSize.int) | ||
| json["entries"] = entriesJson | ||
| return RestApiResponse.response($json, contentType = "application/json", headers = headers) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| return RestApiResponse.response($json, contentType = "application/json", headers = headers) | |
| return RestApiResponse.response( | |
| $json, contentType = "application/json", headers = headers | |
| ) |
| if pOpt =? p: | ||
| if pRes =? pOpt: | ||
| pathStr = pRes | ||
| let pathParts = if pathStr.len > 0: pathStr.split('/') else: @[] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| let pathParts = if pathStr.len > 0: pathStr.split('/') else: @[] | |
| let pathParts = | |
| if pathStr.len > 0: | |
| pathStr.split('/') | |
| else: | |
| @[] |
| return RestApiResponse.redirect( | ||
| Http307, "/api/archivist/v1/data/" & $cidVal | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| return RestApiResponse.redirect( | |
| Http307, "/api/archivist/v1/data/" & $cidVal | |
| ) | |
| return RestApiResponse.redirect(Http307, "/api/archivist/v1/data/" & $cidVal) |
| return RestApiResponse.error( | ||
| Http404, "Path not found: " & part, headers = headers | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| return RestApiResponse.error( | |
| Http404, "Path not found: " & part, headers = headers | |
| ) | |
| return | |
| RestApiResponse.error(Http404, "Path not found: " & part, headers = headers) |
| Http400, "Path component is not a directory: " & part, headers = headers | ||
| ) | ||
|
|
||
| without subDir =? (await fetchDirectoryManifest(node.networkStore, foundEntry.cid)), err: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nph] reported by reviewdog 🐶
| without subDir =? (await fetchDirectoryManifest(node.networkStore, foundEntry.cid)), err: | |
| without subDir =? | |
| (await fetchDirectoryManifest(node.networkStore, foundEntry.cid)), err: |
Implements directory manifests for organizing multiple files into a tree:
finalizes them into a tree structure with POST JSON body
API changes:
All of these are prerequisites to be able to use Flagship (https://riff.cc) with Archivist, as we need folder support to be able to playback albums and make everything a bit easier (we depend heavily on this functionality)
Builds on @markspanbroek's v2.26 work, apologies for the lack of a properly clean PR (but I needed to ship this one :D)