chore: merge upstream 2.54.0 #88
Conversation
94% of minimum 50% translated source file: 'frontend/src/i18n/en.json' on 'es'. Sync of partially translated files: untranslated content is included with an empty translation or source language content depending on file format
Co-authored-by: transifex-integration[bot] <43880903+transifex-integration[bot]@users.noreply.github.com>
Co-authored-by: transifex-integration[bot] <43880903+transifex-integration[bot]@users.noreply.github.com>
100% translated source file: 'frontend/src/i18n/en.json' on 'no'.
Co-authored-by: Henrique Dias <[email protected]>
…nity MB/s (filebrowser#5456) Co-authored-by: Henrique Dias <[email protected]>
Co-authored-by: transifex-integration[bot] <43880903+transifex-integration[bot]@users.noreply.github.com>
Co-authored-by: Ryan Miller <[email protected]>
Bumps [vite](https://github.com/vitejs/vite/tree/HEAD/packages/vite) from 6.3.6 to 6.4.1. - [Release notes](https://github.com/vitejs/vite/releases) - [Changelog](https://github.com/vitejs/vite/blob/main/packages/vite/CHANGELOG.md) - [Commits](https://github.com/vitejs/vite/commits/[email protected]/packages/vite) --- updated-dependencies: - dependency-name: vite dependency-version: 6.4.1 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: transifex-integration[bot] <43880903+transifex-integration[bot]@users.noreply.github.com>
Co-authored-by: manx98 <[email protected]>
Co-authored-by: Henrique Dias <[email protected]>
…lebrowser#5637) Co-authored-by: FadedAtlas <[email protected]>
Co-authored-by: transifex-integration[bot] <43880903+transifex-integration[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: transifex-integration[bot] <43880903+transifex-integration[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: transifex-integration[bot] <43880903+transifex-integration[bot]@users.noreply.github.com>
📝 WalkthroughWalkthroughThis PR introduces FileBrowser v2.54.0, featuring a major refactor with new CI/CD workflows, Taskfile-based build automation, package namespace updates (errors→fberrors, http→fbhttp), Viper-based configuration management, store abstraction, new user preference fields (redirectAfterCopyMove, aceEditorTheme), CSV file preview support, streaming search, frontend modernization (Vue 3, updated dependencies), extensive localization additions, and comprehensive documentation reorganization. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (Frontend)
participant Server as API Server (HTTP)
participant Store as Data Store
participant Auth as Auth Module
Client->>Server: POST /users/add {user, currentPassword}
Server->>Auth: Verify JSONAuth enabled
alt JSONAuth enabled
Server->>Store: Get existing user password
Server->>Auth: Compare currentPassword with stored
alt Password mismatch
Server-->>Client: 400 ErrCurrentPasswordIncorrect
else Password matches
Server->>Store: Save new user
Server-->>Client: 200 User created
end
else JSONAuth disabled
Server->>Store: Save new user directly
Server-->>Client: 200 User created
end
sequenceDiagram
participant Client as Client (Frontend)
participant Server as Search API
participant Worker as File Walker
participant Response as HTTP Response Writer
Client->>Server: GET /search?query=term (with AbortSignal)
Server->>Server: Create cancellable context
Server->>Server: Start background goroutine
Server-->>Client: 200 (streaming response)
Server->>Worker: Start search with callback
loop Search progress
Worker->>Server: Found match via callback
alt Context not cancelled
Server->>Response: Write JSON line + newline
Response-->>Client: Chunk received
else Context cancelled
Worker->>Server: Return context error
end
end
Server->>Response: Write heartbeat periodically
Response-->>Client: Keep-alive packet
Worker-->>Server: Search complete
Server->>Server: Wait for goroutine completion
Server-->>Client: End stream
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
frontend/src/api/pub.ts (1)
46-54: Filenames containing commas will be misinterpreted.When a filename contains a comma, the server will split it incorrectly since the entire string is encoded after joining. Encoding each file individually before joining preserves literal commas within filenames.
Proposed fix
} else { - let arg = ""; - - for (const file of files) { - arg += file + ","; - } - - arg = arg.substring(0, arg.length - 1); - arg = encodeURIComponent(arg); + const arg = files.map((f) => encodeURIComponent(f)).join(","); url += `/?files=${arg}&`; }cmd/rules_add.go (1)
39-41:regexp.MustCompilewill panic on invalid regex input.If a user provides an invalid regex expression,
MustCompilewill panic and crash the CLI instead of returning a user-friendly error. Consider usingregexp.Compileand returning the error gracefully.🐛 Proposed fix
if regex { - regexp.MustCompile(exp) + if _, err := regexp.Compile(exp); err != nil { + return fmt.Errorf("invalid regex expression: %w", err) + } }Note: This would require adding
"fmt"to the imports.files/file.go (1)
450-471: Duplicate image resolution calculation in directory listings.When
calcImgResis true, image resolution is calculated twice for each image file:
- First at lines 450-457 in
readListingbefore callingdetectType- Again at lines 256-263 inside
detectTypewhen it's called at line 467This doubles the I/O and CPU overhead for image-heavy directories.
Proposed fix - remove duplicate calculation from readListing
- if !file.IsDir && strings.HasPrefix(mime.TypeByExtension(file.Extension), "image/") && calcImgRes { - resolution, err := calculateImageResolution(file.Fs, file.Path) - if err != nil { - log.Printf("Error calculating resolution for image %s: %v", file.Path, err) - } else { - file.Resolution = resolution - } - } - if file.IsDir { listing.NumDirs++ } else {The resolution calculation in
detectType(lines 256-263) will handle this when the file type is detected as "image".frontend/src/i18n/index.ts (2)
61-93: Remove unreachable duplicate locale handlers.Lines 76-78 (
ru), 88-90 (tr), and 91-93 (uk) are unreachable code. The earlier patterns/^ru.*/i,/^tr.*/i, and/^uk.*/i(lines 61-69) match any locale starting with these language codes, so the narrower word-boundary patterns/^ru\b/,/^tr\b/, and/^uk\b/that appear later will never execute. Additionally, no JSON translation files exist forru.json,tr.json, oruk.json—onlyru_RU.json,tr_TR.json, anduk_UA.jsonare available. Delete the duplicate cases.
105-108: Add missing dayjs locale import for Norwegian.The code detects Norwegian locales (lines 105-108) and maps them to
"no", but there's no corresponding dayjs locale import. This causesdayjs.locale()to silently fall back to the default locale when Norwegian-speaking users access the application. The correct dayjs locale identifier for Norwegian Bokmål is"nb".Suggested fix
import("dayjs/locale/zh-tw"); +import("dayjs/locale/nb");Additionally, consider updating lines 105-108 to map to
"nb"instead of"no"to match the dayjs locale identifier.frontend/src/views/settings/User.vue (1)
170-178:originalUseris never assigned, making this spread ineffective.
originalUseris declared on line 76 but never assigned a value, sooriginalUser?.valuewill always beundefined. The spread...originalUser?.valuehas no effect.Suggested fix
If
originalUserwas intended to store the initial user state for comparison, it should be assigned infetchData. Otherwise, remove the unused reference:const newUser: IUser = { - ...originalUser?.value, ...user.value, };cmd/users.go (1)
30-50: Header and format string column mismatch.The header on line 30 has 15 columns, but the format string on line 33 has 16 format specifiers (including the new
RedirectAfterCopyMovefield at line 40). The header is missing a column label forRedirectAfterCopyMove.🐛 Proposed fix
- fmt.Fprintln(w, "ID\tUsername\tScope\tLocale\tV. Mode\tS.Click\tAdmin\tExecute\tCreate\tRename\tModify\tDelete\tShare\tDownload\tPwd Lock") + fmt.Fprintln(w, "ID\tUsername\tScope\tLocale\tV. Mode\tS.Click\tRedirect\tAdmin\tExecute\tCreate\tRename\tModify\tDelete\tShare\tDownload\tPwd Lock")frontend/src/views/settings/Profile.vue (1)
164-171: Bug:currentPasswordvalidation check runs unconditionally.The validation at line 167 checks if
currentPassword.value === "", but this check runs even whenisCurrentPasswordRequiredisfalse. This will prevent password updates when current password is not required (e.g., for non-JSON auth methods).🐛 Proposed fix
if ( password.value !== passwordConf.value || password.value === "" || - currentPassword.value === "" || + (isCurrentPasswordRequired.value && currentPassword.value === "") || authStore.user === null ) { return; }cmd/root.go (2)
483-484: Typo in log message."initialize wth" should be "initialized with".
📝 Fix
- log.Printf("User '%s' initialize wth user-provided password\n", username) + log.Printf("User '%s' initialized with user-provided password\n", username)
401-417: Fix typo andAceEditorThemehardcoded empty value.At line 406,
AceEditorThemereads fromv.GetString("defaults.aceEditorTheme"), but theaceEditorThemeflag is only registered inaddUserFlags()(cmd/users.go:87), which is not called by the root command'sinit()function. Consequently, this key is never bound to Viper and will always return an empty string during quick setup. Either add the flag to the root command or set a default via config.Also, fix the typo at line 483: "initialize wth" should be "initialized with".
🤖 Fix all issues with AI agents
In `@cmd/cmds_rm.go`:
- Line 57: The slice removal at s.Commands[evt] using indices i and f can panic
if i or f are out of bounds; before performing s.Commands[evt] =
append(s.Commands[evt][:i], s.Commands[evt][f+1:]...), validate that evt exists
and that 0 <= i <= f < len(s.Commands[evt]) (or similar inclusive checks) and
return an error (e.g., fmt.Errorf("index out of range")) when the check fails to
avoid a runtime panic. Ensure you reference s.Commands, evt, i, and f when
adding the conditional guard.
In `@cmd/users_update.go`:
- Around line 82-95: The boolean flag assignments for user.LockPassword,
user.DateFormat, and user.HideDotfiles always overwrite existing values because
flags.GetBool returns a default when the flag isn't provided; change the logic
in the users update handler (where user.LockPassword, user.DateFormat,
user.HideDotfiles are set using flags.GetBool) to first check
flags.Changed("lockPassword"), flags.Changed("dateFormat"), and
flags.Changed("hideDotfiles") respectively and only call flags.GetBool and
assign when Changed() is true, preserving existing user fields otherwise and
still returning any errors from GetBool.
In `@cmd/utils.go`:
- Around line 120-129: The code incorrectly uses errors.Is(err,
viper.ConfigParseError{}) which compares values not types, so parse errors slip
through; replace that check with errors.As to detect a *viper.ConfigParseError
(or viper.ConfigParseError) via var cfgErr viper.ConfigParseError; if
errors.As(err, &cfgErr) return nil, err so parse errors are returned, otherwise
treat missing config as before and log "No config file used"; keep the existing
log of v.ConfigFileUsed() in the else branch.
In `@frontend/src/api/pub.ts`:
- Around line 43-44: The single-file branch concatenates the raw filename into
the URL (the if block using files and url with files[0]) which breaks for names
with spaces or special characters; replace the direct append with an encoded
filename using encodeURIComponent(files[0]) (i.e., url +=
encodeURIComponent(files[0]) + "?") so the path is properly URL-encoded.
In `@frontend/src/api/search.ts`:
- Around line 26-50: The streaming loop that reads from reader.read() can exit
with a non-empty buffer and drop the last line; after the while(true) loop (or
immediately when done is true), process any remaining buffer: split buffer into
lines, JSON.parse each non-empty line into a ResourceItem, set item.url using
`/files${base}` + url.encodePath(item.path) and append "/" if item.isDir, then
invoke callback(item); ensure buffer is cleared afterward so the final item
isn't lost when the response lacks a trailing newline.
In `@frontend/src/components/prompts/DiscardEditorChanges.vue`:
- Around line 18-26: The Save Changes button calls currentPrompt.saveAction
unguarded (and similarly the Discard button calls currentPrompt.confirm), which
can be undefined per the PopupProps interface; add a guard so the click handlers
only invoke the functions when present (e.g., change the `@click` to check
currentPrompt.saveAction before calling it or use v-if on the button) and do the
same for currentPrompt.confirm to avoid runtime errors when those callbacks are
not provided.
In `@frontend/src/components/prompts/Move.vue`:
- Around line 111-113: The else branch after checking
this.user.redirectAfterCopyMove currently sets this.reload = true but never
calls closeHovers(), leaving the modal open; update the Move.vue logic so the
non-redirect path calls this.closeHovers() (e.g., call this.closeHovers() and
then set this.reload = true) to mirror the unmount behavior of the
this.$router.push path and ensure the move dialog is closed after a successful
operation.
In `@frontend/src/components/prompts/Share.vue`:
- Around line 267-275: The buildDownloadLink function currently omits the
optional share.token when calling api.pub.getDownloadURL; update
buildDownloadLink to include token in the payload when present (e.g., include
...(share.token && { token: share.token }) alongside hash and path) so
getDownloadURL receives the token and returns authenticated download URLs
correctly.
In `@frontend/src/components/prompts/UploadFiles.vue`:
- Around line 114-124: Throttling in throttledCalculateSpeed causes a mismatch
between the bytes delta (oldSentBytes from the Vue watcher) and the time delta
(lastSpeedUpdate from the last non-throttled run), underestimating speed; fix by
introducing and maintaining a lastProcessedSentBytes variable that is updated
whenever throttledCalculateSpeed actually calls calculateSpeed and use
lastProcessedSentBytes instead of oldSentBytes when computing the delta inside
calculateSpeed (also reset lastProcessedSentBytes in the calculateSpeed reset
block alongside whatever resets lastSpeedUpdate).
In `@frontend/src/stores/auth.ts`:
- Around line 39-44: The logout flow can clear the store (authStore.clearUser()
/ clearUser()) which $reset() nulls out logoutTimer but does not clear the
underlying timeout, so an active timer can still fire and call
logout("inactivity") again; modify the logout() implementation in
frontend/src/utils/auth.ts to call clearTimeout(authStore.logoutTimer) (or the
equivalent stored timer ID) and set authStore.setLogoutTimer(null) before
calling authStore.clearUser(), mirroring the order used in parseToken() (lines
27-29), to ensure the timer is cancelled prior to resetting state.
In `@frontend/src/utils/auth.ts`:
- Around line 31-37: The code uses a non-null assertion on data.exp which can be
undefined; update the logic around expiresAt/timeout so you first check whether
data.exp is present and numeric (e.g., typeof data.exp === "number"), compute
expiresAt and timeout only when valid, and only call authStore.setLogoutTimer
with setSafeTimeout if timeout is finite and > 0; if exp is missing or timeout
is invalid, avoid scheduling the logout (or clear any existing timer via
authStore) and handle that branch explicitly instead of relying on data.exp!.
Reference the symbols data.exp, expiresAt, timeout, authStore.setLogoutTimer,
setSafeTimeout, and logout when implementing the guard and fallback behavior.
In `@http/search.go`:
- Around line 25-57: The loop uses time.NewTimer (timeout) which fires once, so
heartbeats stop after the first tick; replace the timer with a time.Ticker
(e.g., ticker := time.NewTicker(searchPingInterval * time.Second), defer
ticker.Stop()) and read from ticker.C in the select instead of timeout.C so the
heartbeat case fires repeatedly, leaving the rest of the logic (reading from
response, json.Marshal, writes, cancel on error, flusher.Flush) unchanged;
ensure you rename references from timeout to ticker and stop the ticker in defer
to avoid leaks.
🟡 Minor comments (48)
frontend/src/views/files/FileListing.vue-1155-1164 (1)
1155-1164: Remove redundant selector in empty area click handler.
target.closest("item")looks for an HTML element with tag name<item>, which doesn't exist in the DOM. Vue components render to standard HTML elements. The<Item>component likely renders as a<div class="item">, so onlytarget.closest(".item")is needed.Suggested fix
const handleEmptyAreaClick = (e: MouseEvent) => { const target = e.target; if (!(target instanceof HTMLElement)) return; - if (target.closest("item") || target.closest(".item")) return; + if (target.closest(".item")) return; if (target.closest(".context-menu")) return; fileStore.selected = []; };http/users.go-163-182 (1)
163-182: Typo:sensibleFieldsshould besensitiveFields.The variable name
sensibleFieldsappears to be a typo. In security contexts, "sensitive" (requiring careful handling) is the correct term, not "sensible" (practical/reasonable).✏️ Suggested fix
if d.settings.AuthMethod == auth.MethodJSONAuth { - var sensibleFields = map[string]struct{}{ + var sensitiveFields = map[string]struct{}{ "all": {}, "username": {}, "password": {}, "scope": {}, "lockPassword": {}, "commands": {}, "perm": {}, } for _, field := range req.Which { - if _, ok := sensibleFields[field]; ok { + if _, ok := sensitiveFields[field]; ok { if !users.CheckPwd(req.CurrentPassword, d.user.Password) { return http.StatusBadRequest, fberrors.ErrCurrentPasswordIncorrect } break } } }frontend/tsconfig.node.json-2-7 (1)
2-7: LGTM! Node.js 24 upgrade is appropriate.Node.js 24 "Krypton" was promoted to Active LTS on October 28, 2025, and will continue receiving maintenance through April 30, 2028. The upgrade from
@tsconfig/node24is properly configured and the package is installed at^24.0.2inpackage.json.This new TypeScript configuration file appropriately includes the tools in use: Vite, Vitest, Cypress, and Nightwatch.
CHANGELOG.md-298-323 (1)
298-323: Duplicate v2.43.0 section detected.Lines 266-297 and 298-323 contain duplicate entries for version 2.43.0 with the same date (2025-09-13). One of these sections should be removed.
🔧 Suggested fix
Remove the duplicate section at lines 298-323, keeping only the first occurrence at lines 266-297.
frontend/test-results/.last-run.json-1-4 (1)
1-4: Addfrontend/test-results/to.gitignore.The
frontend/test-results/.last-run.jsonfile is a Playwright test artifact that is currently tracked in version control. Test result files change with every test run and should not be committed. Addfrontend/test-results/to the root.gitignoreto prevent these artifacts from being tracked.frontend/src/components/prompts/Copy.vue-114-115 (1)
114-115: Add reload when redirect is disabled and destination differs from current path.Copy.vue is inconsistent with Move.vue. When
this.user.redirectAfterCopyMoveis false and the route path differs from the destination, Copy.vue performs no action, leaving the user without feedback. Move.vue handles this by reloading (line 113). Add an else clause for consistency:Suggested fix
if (this.user.redirectAfterCopyMove) this.$router.push({ path: this.dest }); + else this.reload = true;renovate.json-14-14 (1)
14-14: Use array syntax for the schedule property.The cron expression
* * * * 0,6is a valid Renovate schedule for weekends. However, Renovate requires thescheduleproperty to be an array:- "schedule": "* * * * 0,6" + "schedule": ["* * * * 0,6"]frontend/src/i18n/hr.json-175-175 (1)
175-175: Untranslated string remains in English.The
aceEditorThemekey is not translated to Croatian.📝 Suggested fix
- "aceEditorTheme": "Ace editor theme", + "aceEditorTheme": "Tema Ace uređivača",frontend/src/i18n/hr.json-46-46 (1)
46-46: Untranslated string remains in English.The
stopSearchkey is not translated to Croatian.📝 Suggested fix
- "stopSearch": "Stop searching", + "stopSearch": "Zaustavi pretraživanje",frontend/src/i18n/hr.json-262-262 (1)
262-262: Untranslated string remains in English.The
currentPasswordkey is not translated to Croatian.📝 Suggested fix
- "currentPassword": "Your Current Password" + "currentPassword": "Vaša trenutna lozinka"frontend/src/api/utils.ts-95-111 (1)
95-111: Timeout cancellation won't work after the first chunk elapses.The function returns only the initial timeout ID, but subsequent recursive timeouts get new IDs. If
clearTimeout(id)is called after the first 24-hour chunk completes, it won't cancel the pending callback.If cancellation is required for your use case (e.g., user logs out manually before token expires), consider tracking the current timeout ID in a mutable reference or returning a cancel function.
🔧 Alternative approach with cancellation support
-export function setSafeTimeout(callback: () => void, delay: number): number { +export function setSafeTimeout(callback: () => void, delay: number): () => void { const MAX_DELAY = 86_400_000; let remaining = delay; + let currentTimeoutId: number; function scheduleNext(): number { if (remaining <= MAX_DELAY) { - return window.setTimeout(callback, remaining); + currentTimeoutId = window.setTimeout(callback, remaining); + return currentTimeoutId; } else { - return window.setTimeout(() => { + currentTimeoutId = window.setTimeout(() => { remaining -= MAX_DELAY; scheduleNext(); }, MAX_DELAY); + return currentTimeoutId; } } - return scheduleNext(); + scheduleNext(); + return () => window.clearTimeout(currentTimeoutId); }Note: If cancellation isn't needed for the auth timeout use case, this can be deferred.
www/docs/authentication.md-21-21 (1)
21-21: Minor grammar fix.Consider "another provider" instead of "other provider" for grammatical correctness.
📝 Suggested fix
-By default, we use [Google's reCAPTCHA](https://developers.google.com/recaptcha/docs/display) service. If you live in China, or want to use other provider, you can change the host with the following command: +By default, we use [Google's reCAPTCHA](https://developers.google.com/recaptcha/docs/display) service. If you live in China, or want to use another provider, you can change the host with the following command:www/docs/authentication.md-43-45 (1)
43-45: Heading hierarchy issue."No Authentication" is currently an H3 (
###) under the "Proxy Header" H2 section, which implies it's a subsection of Proxy Header. Since it's a separate authentication method, it should be an H2 (##) to maintain proper document structure.📝 Suggested fix
-### No Authentication +## No Authenticationfrontend/src/i18n/es_CO.json-200-208 (1)
200-208: Untranslated settings strings.
aceEditorTheme(Line 200) andhideLoginButton(Line 208) are in English instead of Spanish.frontend/src/i18n/es_CO.json-287-287 (1)
287-287: Untranslated string.
currentPasswordis in English instead of Spanish (e.g., "Tu contraseña actual").frontend/src/i18n/lv.json-262-262 (1)
262-262: Untranslated string in Latvian locale.
"currentPassword"is in English. This should be translated to Latvian (e.g.,"Jūsu pašreizējā parole").frontend/src/i18n/ru_RU.json-50-50 (1)
50-50: Untranslated string in Russian locale.
"stopSearch"is in English. This should be translated to Russian (e.g.,"Остановить поиск").frontend/src/i18n/es_CO.json-50-55 (1)
50-55: Multiple untranslated button strings.Several new button labels are in English instead of Spanish:
discardChanges: "Discard" → e.g., "Descartar"stopSearch: "Stop searching" → e.g., "Detener búsqueda"editAsText: "Edit as Text" → e.g., "Editar como texto"increaseFontSize: "Increase font size" → e.g., "Aumentar tamaño de fuente"decreaseFontSize: "Decrease font size" → e.g., "Disminuir tamaño de fuente"frontend/src/i18n/ru_RU.json-284-284 (1)
284-284: Untranslated string in Russian locale.
"currentPassword"is in English. This should be translated to Russian (e.g.,"Ваш текущий пароль").frontend/src/i18n/lv.json-46-46 (1)
46-46: Untranslated string in Latvian locale.
"stopSearch"is still in English. This should be translated to Latvian for consistency (e.g.,"Apturēt meklēšanu").frontend/src/i18n/es_CO.json-95-102 (1)
95-102: Untranslated CSV-related strings.The CSV preview strings are in English and should be translated to Spanish:
csvTooLarge,csvLoadFailed,showingRows,columnSeparator, andcsvSeparatorsvalues.frontend/src/i18n/es_CO.json-130-133 (1)
130-133: Untranslated login-related strings.
passwordTooShortandlogout_reasons.inactivityare in English instead of Spanish.README.md-2-2 (1)
2-2: Add alt text to the banner image.The image tag is missing alternate text, which affects accessibility for screen readers.
Proposed fix
- <img src="https://raw.githubusercontent.com/filebrowser/filebrowser/master/branding/banner.png" width="550"/> + <img src="https://raw.githubusercontent.com/filebrowser/filebrowser/master/branding/banner.png" width="550" alt="File Browser logo"/>cmd/utils.go-170-172 (1)
170-172: Warning message assumes hardcoded database filename.The warning message hardcodes
"filebrowser.db"but the actual database path could have a different filename. Consider usingfilepath.Base(path)for accuracy.Proposed fix
case !exists && !options.expectsNoDatabase: - log.Println("WARNING: filebrowser.db can't be found. Initialing in " + strings.TrimSuffix(path, "filebrowser.db")) + log.Printf("WARNING: %s can't be found. Initializing in %s", filepath.Base(path), filepath.Dir(path))frontend/src/components/files/CsvViewer.vue-82-96 (1)
82-96: Hardcoded error message should use i18n.The error message on line 93 is hardcoded in English while other messages in this component use
$t()for internationalization. For consistency, this should use the translation system.Proposed fix
const displayError = computed(() => { // External error takes priority (e.g., file too large) if (props.error) { return props.error; } // Check for parse errors if ( props.content && props.content.trim().length > 0 && data.value.headers.length === 0 ) { - return "Failed to parse CSV file"; + return t("files.csvParseFailed"); } return null; });You'll also need to add
useI18n:import { parseCSV, type CsvData } from "@/utils/csv"; -import { computed, ref } from "vue"; +import { computed, ref } from "vue"; +import { useI18n } from "vue-i18n"; +const { t } = useI18n();frontend/src/i18n/pt_PT.json-49-54 (1)
49-54: Missing Portuguese translations for new button keys.The newly added button labels are in English while this is the Portuguese (pt_PT) locale file. These should be translated for consistency with existing Portuguese translations in this file.
Suggested translations:
discardChanges: "Descartar"stopSearch: "Parar pesquisa"saveChanges: "Guardar alterações"editAsText: "Editar como texto"increaseFontSize: "Aumentar tamanho da fonte"decreaseFontSize: "Diminuir tamanho da fonte"frontend/src/i18n/pt_PT.json-87-96 (1)
87-96: Missing Portuguese translations for CSV preview strings.These new CSV-related strings are in English. Consider translating them:
noPreview: "A pré-visualização não está disponível para este ficheiro."csvTooLarge: "O ficheiro CSV é demasiado grande para pré-visualizar (>5MB). Por favor descarregue para ver."csvLoadFailed: "Falha ao carregar o ficheiro CSV."showingRows: "A mostrar {count} linha(s)"columnSeparator: "Separador de colunas"csvSeparators.comma: "Vírgula (,)"csvSeparators.semicolon: "Ponto e vírgula (;)"csvSeparators.both: "Ambos (,) e (;)"frontend/src/i18n/pt_PT.json-122-126 (1)
122-126: Additional untranslated strings in login and settings sections.Several more strings need Portuguese translations:
Login section:
passwordTooShort: "A palavra-passe deve ter pelo menos {min} caracteres"logout_reasons.inactivity: "Foi desligado devido a inatividade."Settings section:
aceEditorTheme: "Tema do editor Ace"hideLoginButton: "Ocultar o botão de login das páginas públicas"currentPassword: "A sua palavra-passe atual"Also applies to: 193-205, 280-280
www/docs/cli/filebrowser-config.md-11-13 (1)
11-13: Add language specifiers to fenced code blocks.Per static analysis (MD040), add language specifiers:
📝 Proposed fix
-``` +```text -h, --help help for config...
-
+text
-c, --config string config file path
-d, --database string database path (default "./filebrowser.db")Also applies to: 17-20
www/docs/cli/filebrowser-cmds-ls.md-9-11 (1)
9-11: Add language specifiers to fenced code blocks.Per static analysis (MD040), fenced code blocks should have a language specified for proper syntax highlighting and accessibility. Based on this linting feedback:
📝 Proposed fix
-``` +```shell filebrowser cmds ls [flags]...
-
+text
-e, --event string event name, without 'before' or 'after'
-h, --help help for ls... -``` +```text -c, --config string config file path -d, --database string database path (default "./filebrowser.db")</details> Also applies to: 15-18, 22-25 </blockquote></details> <details> <summary>www/docs/cli/filebrowser-cmds-ls.md-29-29 (1)</summary><blockquote> `29-29`: **Replace hard tab with spaces.** Per static analysis (MD010), this line contains a hard tab. Replace it with spaces for consistency. <details> <summary>📝 Proposed fix</summary> ```diff -* [filebrowser cmds](filebrowser-cmds.md) - Command runner management utility +* [filebrowser cmds](filebrowser-cmds.md) - Command runner management utilitywww/docs/cli/filebrowser-config.md-24-29 (1)
24-29: Replace hard tabs with spaces in See Also section.Per static analysis (MD010), lines 24-29 contain hard tabs. Replace with spaces for consistency.
📝 Proposed fix
-* [filebrowser](filebrowser.md) - A stylish web-based file browser -* [filebrowser config cat](filebrowser-config-cat.md) - Prints the configuration -* [filebrowser config export](filebrowser-config-export.md) - Export the configuration to a file -* [filebrowser config import](filebrowser-config-import.md) - Import a configuration file -* [filebrowser config init](filebrowser-config-init.md) - Initialize a new database -* [filebrowser config set](filebrowser-config-set.md) - Updates the configuration +* [filebrowser](filebrowser.md) - A stylish web-based file browser +* [filebrowser config cat](filebrowser-config-cat.md) - Prints the configuration +* [filebrowser config export](filebrowser-config-export.md) - Export the configuration to a file +* [filebrowser config import](filebrowser-config-import.md) - Import a configuration file +* [filebrowser config init](filebrowser-config-init.md) - Initialize a new database +* [filebrowser config set](filebrowser-config-set.md) - Updates the configurationfrontend/src/css/context-menu.css-11-17 (1)
11-17: Remove duplicatedisplayproperty.The
displayproperty is declared twice (lines 12 and 15). The second declaration (display: flex;) will override the first (display: block;), making line 12 dead code. Based on thealign-items: center;usage, the intent is clearly flexbox layout.🐛 Proposed fix
.context-menu .action { - display: block; width: 100%; border-radius: 0; display: flex; align-items: center; }.github/workflows/docs.yml-5-7 (1)
5-7: Path filter'www'may not match subdirectory changes.The path
'www'matches only the directory itself, not files within it. To trigger on changes to any file underwww/, use'www/**'.Suggested fix
pull_request: paths: - - 'www' + - 'www/**' - '*.md'.github/workflows/docs.yml-33-35 (1)
33-35: Missing stepidcauses invalid output reference.The
urlreferencessteps.deployment.outputs.page_url, but no step hasid: deployment. The deploy step at line 51-52 needs this ID for the output to be accessible.Suggested fix
- name: Deploy to GitHub Pages + id: deployment uses: actions/deploy-pages@v4frontend/src/i18n/bg.json-46-50 (1)
46-50: Several translation keys remain in English.The following keys are not yet translated to Bulgarian:
- Lines 46, 48-50:
stopSearch,editAsText,increaseFontSize,decreaseFontSize- Lines 83-91: CSV-related strings (
csvTooLarge,csvLoadFailed,showingRows,columnSeparator,csvSeparators.*)- Line 118:
passwordTooShort- Line 262:
currentPasswordConsider completing these translations or opening an issue to track the remaining work.
Would you like me to open an issue to track completing the Bulgarian translations?
Also applies to: 83-91, 118-118, 262-262
frontend/src/components/settings/AceEditorTheme.vue-14-14 (1)
14-14: UseHTMLSelectElementinstead ofSelectHTMLAttributesfor DOM element casting.
SelectHTMLAttributesis a Vue type for defining props on<select>elements, not for accessing DOM element properties. UseHTMLSelectElementfor proper typing when accessing native DOM properties likevalue.🐛 Suggested fix
-import { type SelectHTMLAttributes } from "vue"; import { themes } from "ace-builds/src-noconflict/ext-themelist"; // ... const change = (event: Event) => { - emit("update:aceEditorTheme", (event.target as SelectHTMLAttributes)?.value); + emit("update:aceEditorTheme", (event.target as HTMLSelectElement)?.value); };Also applies to: 25-27
frontend/src/components/ContextMenu.vue-33-42 (1)
33-42: Potential race condition: click listener may immediately close the menu.When the menu opens (triggered by a click), the watch immediately adds a click listener to the document. If the opening click event bubbles up to the document, it could trigger
hideContextMenuand close the menu immediately.Consider using
{ once: false }with anextTickdelay, or checking if the click target is outside the menu:🐛 Suggested fix using nextTick
+import { ref, watch, computed, onUnmounted, nextTick } from "vue"; -import { ref, watch, computed, onUnmounted } from "vue"; watch( () => props.show, (val) => { if (val) { - document.addEventListener("click", hideContextMenu); + nextTick(() => { + document.addEventListener("click", hideContextMenu); + }); } else { document.removeEventListener("click", hideContextMenu); } } );frontend/src/i18n/lv_LV.json-46-46 (1)
46-46: Untranslated string: "stopSearch" is in English.This key contains English text while the rest of the file is in Latvian. It should be translated for consistency.
Suggested fix
- "stopSearch": "Stop searching", + "stopSearch": "Pārtraukt meklēšanu",frontend/src/i18n/lv_LV.json-262-262 (1)
262-262: Untranslated string: "currentPassword" is in English.This key contains English text ("Your Current Password") while the rest of the file is in Latvian.
Suggested fix
- "currentPassword": "Your Current Password" + "currentPassword": "Jūsu pašreizējā parole".github/workflows/ci.yaml-37-37 (1)
37-37: Use consistent Go version format across jobs.The workflow uses inconsistent Go version formats:
"1.25.x"in lines 37 and 49, but'1.25'in lines 61 and 85. Use1.25.xconsistently for clarity and explicit patch-version matching.www/docs/command-execution.md-43-54 (1)
43-54: Docker example will not work with the Alpine-based image.The
filebrowser/filebrowserDocker image uses Alpine Linux, which usesapkas its package manager instead ofapt. Additionally,sudois not included in Alpine by default.Suggested fix
> If using Docker and you want to add a new command that is not in the base image then you will need to build a custom Docker image using `filebrowser/filebrowser` as a base image. For example to add 7z: > > ```docker > FROM filebrowser/filebrowser -> RUN sudo apt install p7zip-full +> RUN apk add --no-cache p7zip > ```cmd/docs.go-72-75 (1)
72-75: File permissions too permissive and output directory not created.Two issues:
0666grants world-writable permissions;0644is more appropriate for documentation files.- If the output directory doesn't exist,
WriteFilewill fail. Consider creating it first.Proposed fix
Add directory creation before the loop (after line 55):
fmt.Println("Generated Documents:") + if err := os.MkdirAll(outputDir, 0755); err != nil { + return err + } + for _, entry := range entries {And fix the file permissions:
- err = os.WriteFile(dstPath, data, 0666) + err = os.WriteFile(dstPath, data, 0644)frontend/src/views/files/Editor.vue-226-231 (1)
226-231: Empty catch block silently swallows save errors.If
save(true)fails, the error is caught and ignored, leaving the user on the editor without feedback about why the close didn't complete. Consider showing an error message or at minimum logging the error.🐛 Suggested fix
saveAction: async () => { try { await save(true); finishClose(); - } catch {} + } catch (e) { + // Error already shown to user via $showError in save() + // User stays on editor to retry or discard + } },At minimum, add a comment explaining the intentional behavior, or log the error for debugging purposes.
cmd/users.go-104-152 (1)
104-152: MissingdateFormatcase in the switch statement.The
dateFormatflag is defined inaddUserFlags(line 85) but there's no corresponding case in thegetUserDefaultsswitch statement. This means the flag won't populatedefaults.DateFormat, rendering it ineffective for user defaults.🐛 Proposed fix - add the missing case
case "hideDotfiles": defaults.HideDotfiles, err = flags.GetBool(flag.Name) + case "dateFormat": + defaults.DateFormat, err = flags.GetBool(flag.Name) }frontend/src/i18n/zh_CN.json-50-55 (1)
50-55: Missing Chinese translation forstopSearch.The key
stopSearchhas English text"Stop searching"instead of a Chinese translation. This should be translated for consistency with the rest of the locale file (e.g.,"停止搜索").frontend/src/i18n/zh_CN.json-296-298 (1)
296-298: Missing Chinese translation forcurrentPassword.The key
currentPasswordhas English text"Your Current Password"instead of a Chinese translation. This should be translated for consistency (e.g.,"您的当前密码").cmd/root.go-359-364 (1)
359-364: Incomplete warning message.The warning at line 361-362 reads "This feature has known security vulnerabilities and should not" but appears to be truncated. The sentence is incomplete.
📝 Suggested fix
if server.EnableExec { log.Println("WARNING: Command Runner feature enabled!") - log.Println("WARNING: This feature has known security vulnerabilities and should not") - log.Println("WARNING: you fully understand the risks involved. For more information") + log.Println("WARNING: This feature has known security vulnerabilities and should not be used unless") + log.Println("WARNING: you fully understand the risks involved. For more information,") log.Println("WARNING: read https://github.com/filebrowser/filebrowser/issues/5199") }
| } | ||
| } | ||
|
|
||
| s.Commands[evt] = append(s.Commands[evt][:i], s.Commands[evt][f+1:]...) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n cmd/cmds_rm.go | head -80 | tail -40Repository: hostinger/filebrowser
Length of output: 1082
🏁 Script executed:
cd cmd && rg -n "Args" cmds_rm.go -B5 -A5Repository: hostinger/filebrowser
Length of output: 462
🏁 Script executed:
# Check for bounds checking patterns in the file
rg -n "len\(" cmd/cmds_rm.go -B2 -A2Repository: hostinger/filebrowser
Length of output: 167
🏁 Script executed:
rg -n "withStore" cmd/cmds_rm.go -B2 -A10Repository: hostinger/filebrowser
Length of output: 348
🏁 Script executed:
# Search for withStore implementation to see if there's validation there
rg -n "func withStore" cmd/ -A15Repository: hostinger/filebrowser
Length of output: 832
🏁 Script executed:
# Check if s.Commands is initialized/guarded
rg -n "s.Commands\[evt\]" cmd/cmds_rm.go -B5 -A2Repository: hostinger/filebrowser
Length of output: 252
Add bounds validation before removing commands from slice.
Line 57 will panic if the provided indices exceed the command list length. The Args validator only ensures indices are valid integers, not that they're within bounds of s.Commands[evt]. Add a length check before the slice operation:
if i > len(s.Commands[evt]) || f >= len(s.Commands[evt]) {
return fmt.Errorf("index out of range")
}
🤖 Prompt for AI Agents
In `@cmd/cmds_rm.go` at line 57, The slice removal at s.Commands[evt] using
indices i and f can panic if i or f are out of bounds; before performing
s.Commands[evt] = append(s.Commands[evt][:i], s.Commands[evt][f+1:]...),
validate that evt exists and that 0 <= i <= f < len(s.Commands[evt]) (or similar
inclusive checks) and return an error (e.g., fmt.Errorf("index out of range"))
when the check fails to avoid a runtime panic. Ensure you reference s.Commands,
evt, i, and f when adding the conditional guard.
| user := func(u *users.User) error { | ||
| u.Rules = append(u.Rules[:i], u.Rules[f+1:]...) | ||
| return d.store.Users.Save(u) | ||
| return st.Users.Save(u) | ||
| } | ||
|
|
||
| global := func(s *settings.Settings) error { | ||
| s.Rules = append(s.Rules[:i], s.Rules[f+1:]...) | ||
| return d.store.Settings.Save(s) | ||
| return st.Settings.Save(s) | ||
| } |
There was a problem hiding this comment.
Potential panic on out-of-bounds slice access.
The slice operations u.Rules[:i] and u.Rules[f+1:] (and similarly for s.Rules) will panic if i or f+1 exceed the slice length. While the Args validation ensures the arguments are valid integers, there's no bounds check against the actual rules count.
Consider adding bounds validation:
🛡️ Suggested bounds check
user := func(u *users.User) error {
+ if i < 0 || f >= len(u.Rules) || i > f {
+ return fmt.Errorf("index out of range: rules count is %d", len(u.Rules))
+ }
u.Rules = append(u.Rules[:i], u.Rules[f+1:]...)
return st.Users.Save(u)
}
global := func(s *settings.Settings) error {
+ if i < 0 || f >= len(s.Rules) || i > f {
+ return fmt.Errorf("index out of range: rules count is %d", len(s.Rules))
+ }
s.Rules = append(s.Rules[:i], s.Rules[f+1:]...)
return st.Settings.Save(s)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| user := func(u *users.User) error { | |
| u.Rules = append(u.Rules[:i], u.Rules[f+1:]...) | |
| return d.store.Users.Save(u) | |
| return st.Users.Save(u) | |
| } | |
| global := func(s *settings.Settings) error { | |
| s.Rules = append(s.Rules[:i], s.Rules[f+1:]...) | |
| return d.store.Settings.Save(s) | |
| return st.Settings.Save(s) | |
| } | |
| user := func(u *users.User) error { | |
| if i < 0 || f >= len(u.Rules) || i > f { | |
| return fmt.Errorf("index out of range: rules count is %d", len(u.Rules)) | |
| } | |
| u.Rules = append(u.Rules[:i], u.Rules[f+1:]...) | |
| return st.Users.Save(u) | |
| } | |
| global := func(s *settings.Settings) error { | |
| if i < 0 || f >= len(s.Rules) || i > f { | |
| return fmt.Errorf("index out of range: rules count is %d", len(s.Rules)) | |
| } | |
| s.Rules = append(s.Rules[:i], s.Rules[f+1:]...) | |
| return st.Settings.Save(s) | |
| } |
| user.LockPassword, err = flags.GetBool("lockPassword") | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| hideDotfiles, err := getBool(cmd.Flags(), "hideDotfiles") | ||
| user.DateFormat, err = flags.GetBool("dateFormat") | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| user := &users.User{ | ||
| Username: args[0], | ||
| Password: password, | ||
| LockPassword: lockPassword, | ||
| DateFormat: dateFormat, | ||
| HideDotfiles: hideDotfiles, | ||
| user.HideDotfiles, err = flags.GetBool("hideDotfiles") | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| s.Defaults.Apply(user) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find Apply() method implementation - broader search with context
rg -A 20 "func.*Apply\(" cmd/users_add.goRepository: hostinger/filebrowser
Length of output: 47
🏁 Script executed:
# Also check UserDefaults struct definition and Apply method in other files
fd -e go | xargs rg -l "UserDefaults" | head -20Repository: hostinger/filebrowser
Length of output: 199
🏁 Script executed:
# Look for the Apply method in the codebase
ast-grep --pattern $'func ($_ $_) Apply($_ $_) {
$$$
}'Repository: hostinger/filebrowser
Length of output: 880
🏁 Script executed:
# Let's also look at the full context of the flag assignment section in users_add.go
cat -n cmd/users_add.go | sed -n '30,70p'Repository: hostinger/filebrowser
Length of output: 1247
Apply() unconditionally overwrites HideDotfiles and DateFormat, discarding flag values.
The code sets DateFormat and HideDotfiles from flags (lines 45, 50), but s.Defaults.Apply(user) (line 55) immediately overwrites both with hardcoded defaults. This silently discards user-provided flag values. The Apply() method in settings/defaults.go unconditionally assigns these fields regardless of whether they were already set:
u.HideDotfiles = d.HideDotfiles // overwrites line 50
u.DateFormat = d.DateFormat // overwrites line 45
Either conditionally apply defaults (only if not already set), reorder the calls so Apply() runs first, or remove the flag extraction for these fields if defaults should always be used.
| user.LockPassword, err = flags.GetBool("lockPassword") | ||
| if err != nil { | ||
| return err | ||
| } | ||
| user.DateFormat, err = getBool(flags, "dateFormat") | ||
|
|
||
| user.DateFormat, err = flags.GetBool("dateFormat") | ||
| if err != nil { | ||
| return err | ||
| } | ||
| user.HideDotfiles, err = getBool(flags, "hideDotfiles") | ||
|
|
||
| user.HideDotfiles, err = flags.GetBool("hideDotfiles") | ||
| if err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
Boolean flags unconditionally overwrite existing user values.
When a user runs filebrowser users update <user> without passing --lockPassword, --dateFormat, or --hideDotfiles, GetBool returns the default value (typically false), which overwrites the user's existing settings. This means updating any user attribute resets these boolean fields.
Proposed fix using flags.Changed()
- user.LockPassword, err = flags.GetBool("lockPassword")
- if err != nil {
- return err
+ if flags.Changed("lockPassword") {
+ user.LockPassword, err = flags.GetBool("lockPassword")
+ if err != nil {
+ return err
+ }
}
- user.DateFormat, err = flags.GetBool("dateFormat")
- if err != nil {
- return err
+ if flags.Changed("dateFormat") {
+ user.DateFormat, err = flags.GetBool("dateFormat")
+ if err != nil {
+ return err
+ }
}
- user.HideDotfiles, err = flags.GetBool("hideDotfiles")
- if err != nil {
- return err
+ if flags.Changed("hideDotfiles") {
+ user.HideDotfiles, err = flags.GetBool("hideDotfiles")
+ if err != nil {
+ return err
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| user.LockPassword, err = flags.GetBool("lockPassword") | |
| if err != nil { | |
| return err | |
| } | |
| user.DateFormat, err = getBool(flags, "dateFormat") | |
| user.DateFormat, err = flags.GetBool("dateFormat") | |
| if err != nil { | |
| return err | |
| } | |
| user.HideDotfiles, err = getBool(flags, "hideDotfiles") | |
| user.HideDotfiles, err = flags.GetBool("hideDotfiles") | |
| if err != nil { | |
| return err | |
| } | |
| if flags.Changed("lockPassword") { | |
| user.LockPassword, err = flags.GetBool("lockPassword") | |
| if err != nil { | |
| return err | |
| } | |
| } | |
| if flags.Changed("dateFormat") { | |
| user.DateFormat, err = flags.GetBool("dateFormat") | |
| if err != nil { | |
| return err | |
| } | |
| } | |
| if flags.Changed("hideDotfiles") { | |
| user.HideDotfiles, err = flags.GetBool("hideDotfiles") | |
| if err != nil { | |
| return err | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@cmd/users_update.go` around lines 82 - 95, The boolean flag assignments for
user.LockPassword, user.DateFormat, and user.HideDotfiles always overwrite
existing values because flags.GetBool returns a default when the flag isn't
provided; change the logic in the users update handler (where user.LockPassword,
user.DateFormat, user.HideDotfiles are set using flags.GetBool) to first check
flags.Changed("lockPassword"), flags.Changed("dateFormat"), and
flags.Changed("hideDotfiles") respectively and only call flags.GetBool and
assign when Changed() is true, preserving existing user fields otherwise and
still returning any errors from GetBool.
| // Read in configuration | ||
| if err := v.ReadInConfig(); err != nil { | ||
| if errors.Is(err, viper.ConfigParseError{}) { | ||
| return nil, err | ||
| } | ||
|
|
||
| log.Println("No config file used") | ||
| } else { | ||
| log.Printf("Using config file: %s", v.ConfigFileUsed()) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the actual code in cmd/utils.go around lines 120-129
cat -n cmd/utils.go | sed -n '115,135p'Repository: hostinger/filebrowser
Length of output: 587
🌐 Web query:
viper ConfigParseError error handling Go
💡 Result:
Use errors.As to detect a viper.ConfigParseError after calling ReadInConfig (or other Viper funcs that return an error). Example:
import (
"errors"
"fmt"
"github.com/spf13/viper"
)
err := viper.ReadInConfig()
if err != nil {
var perr viper.ConfigParseError
if errors.As(err, &perr) {
// config file was found but failed to parse
fmt.Printf("config parse error: %v\n", perr)
// handle accordingly (exit, default values, etc.)
return
}
var fnfe viper.ConfigFileNotFoundError
if errors.As(err, &fnfe) {
// config file not found — handle optional config case
fmt.Println("config file not found, continuing with defaults")
return
}
// other errors
fmt.Printf("unexpected error reading config: %v\n", err)
}
Sources: Viper docs/README and API docs showing ConfigParseError and examples of using errors.As. [1][2]
References
[1] spf13/viper README (error handling example).
[2] pkg.go.dev — github.com/spf13/viper (ConfigParseError type).
Bug: errors.Is used incorrectly with struct type.
errors.Is(err, viper.ConfigParseError{}) will never match because errors.Is compares error values using the Is method or direct comparison, not types. ConfigParseError is a struct type and requires type assertion via errors.As. This means config parse errors will be silently treated as "no config file" instead of being reported. This is a critical bug since parse errors go unhandled.
Correct error handling pattern
// Read in configuration
if err := v.ReadInConfig(); err != nil {
- if errors.Is(err, viper.ConfigParseError{}) {
+ var configParseErr viper.ConfigParseError
+ if errors.As(err, &configParseErr) {
return nil, err
}
log.Println("No config file used")
} else {
log.Printf("Using config file: %s", v.ConfigFileUsed())
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Read in configuration | |
| if err := v.ReadInConfig(); err != nil { | |
| if errors.Is(err, viper.ConfigParseError{}) { | |
| return nil, err | |
| } | |
| log.Println("No config file used") | |
| } else { | |
| log.Printf("Using config file: %s", v.ConfigFileUsed()) | |
| } | |
| // Read in configuration | |
| if err := v.ReadInConfig(); err != nil { | |
| var configParseErr viper.ConfigParseError | |
| if errors.As(err, &configParseErr) { | |
| return nil, err | |
| } | |
| log.Println("No config file used") | |
| } else { | |
| log.Printf("Using config file: %s", v.ConfigFileUsed()) | |
| } |
🤖 Prompt for AI Agents
In `@cmd/utils.go` around lines 120 - 129, The code incorrectly uses
errors.Is(err, viper.ConfigParseError{}) which compares values not types, so
parse errors slip through; replace that check with errors.As to detect a
*viper.ConfigParseError (or viper.ConfigParseError) via var cfgErr
viper.ConfigParseError; if errors.As(err, &cfgErr) return nil, err so parse
errors are returned, otherwise treat missing config as before and log "No config
file used"; keep the existing log of v.ConfigFileUsed() in the else branch.
| buildDownloadLink(share) { | ||
| return api.pub.getDownloadURL( | ||
| { | ||
| hash: share.hash, | ||
| path: "", | ||
| }, | ||
| true | ||
| ); | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find Share interface/type definition
rg -n "interface.*Share|type.*Share" --type ts frontend/src
# Find links structure
rg -n "links:" --type vue --type ts frontend/src
# Find the pub.ts file and check getDownloadURL implementation
fd "pub.ts" --type tsRepository: hostinger/filebrowser
Length of output: 328
🏁 Script executed:
#!/bin/bash
# Read the Share interface definition
cat -n frontend/src/types/api.d.ts | head -50
# Find pub.ts file
fd "pub.ts" --type f
# Look at Share.vue around the buildDownloadLink method
grep -n "buildDownloadLink\|share\." frontend/src/components/prompts/Share.vue | head -20Repository: hostinger/filebrowser
Length of output: 1293
🏁 Script executed:
#!/bin/bash
# Read pub.ts file - find and examine getDownloadURL
cat -n frontend/src/api/pub.ts | grep -A 20 "getDownloadURL"Repository: hostinger/filebrowser
Length of output: 362
🏁 Script executed:
#!/bin/bash
# Find Resource type definition
rg -n "interface Resource|type Resource" --type-list
# Check what getDownloadURL expects
grep -B 5 "export function getDownloadURL" frontend/src/api/pub.ts
# Find other uses of getDownloadURL to see if token is passed
rg "getDownloadURL" frontend/src --context 3Repository: hostinger/filebrowser
Length of output: 4245
Pass the share token to getDownloadURL if available.
The share object contains an optional token property, and getDownloadURL explicitly handles it:
...(res.token && { token: res.token })
The current implementation should pass the token:
buildDownloadLink(share) {
return api.pub.getDownloadURL(
{
hash: share.hash,
path: "",
...(share.token && { token: share.token })
},
true
);
}Without the token, authenticated download links may fail or be inaccessible.
🤖 Prompt for AI Agents
In `@frontend/src/components/prompts/Share.vue` around lines 267 - 275, The
buildDownloadLink function currently omits the optional share.token when calling
api.pub.getDownloadURL; update buildDownloadLink to include token in the payload
when present (e.g., include ...(share.token && { token: share.token }) alongside
hash and path) so getDownloadURL receives the token and returns authenticated
download URLs correctly.
| let lastThrottleTime = 0; | ||
| const throttledCalculateSpeed = (sentBytes: number, oldSentBytes: number) => { | ||
| const now = Date.now(); | ||
| if (now - lastThrottleTime < 100) { | ||
| return; | ||
| } | ||
| lastThrottleTime = now; | ||
| calculateSpeed(sentBytes, oldSentBytes); | ||
| }; |
There was a problem hiding this comment.
Throttling causes incorrect speed calculation due to value/time mismatch.
When throttled calls are skipped, oldSentBytes (from Vue's watcher) reflects the immediately previous reactive value, but lastSpeedUpdate was set during the last non-throttled execution. This mismatch causes calculateSpeed to compute bytes delta from recent value but time delta from much earlier, resulting in underestimated speed.
Track the last processed bytes value instead of relying on Vue's oldSentBytes:
🐛 Proposed fix
let lastThrottleTime = 0;
+let lastProcessedSentBytes = 0;
-const throttledCalculateSpeed = (sentBytes: number, oldSentBytes: number) => {
+const throttledCalculateSpeed = (sentBytes: number) => {
const now = Date.now();
if (now - lastThrottleTime < 100) {
return;
}
lastThrottleTime = now;
- calculateSpeed(sentBytes, oldSentBytes);
+ calculateSpeed(sentBytes, lastProcessedSentBytes);
+ lastProcessedSentBytes = sentBytes;
};Also reset lastProcessedSentBytes in the calculateSpeed reset block:
const calculateSpeed = (sentBytes: number, oldSentBytes: number) => {
// Reset the state when the uploads batch is complete
if (sentBytes === 0) {
lastSpeedUpdate = 0;
recentSpeeds = [];
+ lastProcessedSentBytes = 0;
eta.value = Infinity;
speed.value = 0;
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let lastThrottleTime = 0; | |
| const throttledCalculateSpeed = (sentBytes: number, oldSentBytes: number) => { | |
| const now = Date.now(); | |
| if (now - lastThrottleTime < 100) { | |
| return; | |
| } | |
| lastThrottleTime = now; | |
| calculateSpeed(sentBytes, oldSentBytes); | |
| }; | |
| let lastThrottleTime = 0; | |
| let lastProcessedSentBytes = 0; | |
| const throttledCalculateSpeed = (sentBytes: number) => { | |
| const now = Date.now(); | |
| if (now - lastThrottleTime < 100) { | |
| return; | |
| } | |
| lastThrottleTime = now; | |
| calculateSpeed(sentBytes, lastProcessedSentBytes); | |
| lastProcessedSentBytes = sentBytes; | |
| }; |
🤖 Prompt for AI Agents
In `@frontend/src/components/prompts/UploadFiles.vue` around lines 114 - 124,
Throttling in throttledCalculateSpeed causes a mismatch between the bytes delta
(oldSentBytes from the Vue watcher) and the time delta (lastSpeedUpdate from the
last non-throttled run), underestimating speed; fix by introducing and
maintaining a lastProcessedSentBytes variable that is updated whenever
throttledCalculateSpeed actually calls calculateSpeed and use
lastProcessedSentBytes instead of oldSentBytes when computing the delta inside
calculateSpeed (also reset lastProcessedSentBytes in the calculateSpeed reset
block alongside whatever resets lastSpeedUpdate).
| clearUser() { | ||
| this.$reset(); | ||
| }, | ||
| setLogoutTimer(logoutTimer: number | null) { | ||
| this.logoutTimer = logoutTimer; | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find and read the auth store file to see full context
find . -name "auth.ts" -path "*/stores/*" | head -5Repository: hostinger/filebrowser
Length of output: 94
🏁 Script executed:
#!/bin/bash
# Search for logoutTimer usage and clearTimeout calls
rg -n "logoutTimer|clearTimeout" --type=ts -g '!node_modules/**' -B 3 -A 3Repository: hostinger/filebrowser
Length of output: 1409
🏁 Script executed:
#!/bin/bash
# Check if frontend/src/utils/auth.ts exists and examine timer-related code
if [ -f "frontend/src/utils/auth.ts" ]; then
echo "=== frontend/src/utils/auth.ts ==="
cat -n "frontend/src/utils/auth.ts"
fiRepository: hostinger/filebrowser
Length of output: 4452
🏁 Script executed:
#!/bin/bash
# Check if there are other places where clearUser() is called
rg -n "clearUser\(\)" --type=ts -g '!node_modules/**'Repository: hostinger/filebrowser
Length of output: 169
🏁 Script executed:
#!/bin/bash
# Also check the logout function and how it's called
rg -n "logout\(" --type=ts -g '!node_modules/**' -B 2 -A 2 | head -60Repository: hostinger/filebrowser
Length of output: 797
Clear the active logout timer before resetting state in logout() function.
The clearUser() action calls $reset() which nullifies the logoutTimer reference, but does not clear the actual timer via clearTimeout(). If a logout timer is active and logout() is invoked, the timer callback will still fire and trigger logout("inactivity") again, causing duplicate logout attempts.
Clear the timer before calling clearUser() in the logout() function (frontend/src/utils/auth.ts:118), similar to how parseToken() handles it (lines 27-29). Move the timer cleanup logic to occur before authStore.clearUser() is called.
🤖 Prompt for AI Agents
In `@frontend/src/stores/auth.ts` around lines 39 - 44, The logout flow can clear
the store (authStore.clearUser() / clearUser()) which $reset() nulls out
logoutTimer but does not clear the underlying timeout, so an active timer can
still fire and call logout("inactivity") again; modify the logout()
implementation in frontend/src/utils/auth.ts to call
clearTimeout(authStore.logoutTimer) (or the equivalent stored timer ID) and set
authStore.setLogoutTimer(null) before calling authStore.clearUser(), mirroring
the order used in parseToken() (lines 27-29), to ensure the timer is cancelled
prior to resetting state.
| const expiresAt = new Date(data.exp! * 1000); | ||
| const timeout = expiresAt.getTime() - Date.now(); | ||
| authStore.setLogoutTimer( | ||
| setSafeTimeout(() => { | ||
| logout("inactivity"); | ||
| }, timeout) | ||
| ); |
There was a problem hiding this comment.
Non-null assertion on data.exp may cause runtime errors.
JWT tokens don't always include an exp claim. If a token without expiry is parsed, data.exp! will be undefined, causing expiresAt to become an invalid date and timeout to be NaN.
Suggested fix
+ if (!data.exp) {
+ console.warn("Token has no expiry, skipping logout timer");
+ return;
+ }
+
const expiresAt = new Date(data.exp! * 1000);
const timeout = expiresAt.getTime() - Date.now();
+ if (timeout <= 0) {
+ logout("inactivity");
+ return;
+ }
authStore.setLogoutTimer(
setSafeTimeout(() => {
logout("inactivity");
}, timeout)
);🤖 Prompt for AI Agents
In `@frontend/src/utils/auth.ts` around lines 31 - 37, The code uses a non-null
assertion on data.exp which can be undefined; update the logic around
expiresAt/timeout so you first check whether data.exp is present and numeric
(e.g., typeof data.exp === "number"), compute expiresAt and timeout only when
valid, and only call authStore.setLogoutTimer with setSafeTimeout if timeout is
finite and > 0; if exp is missing or timeout is invalid, avoid scheduling the
logout (or clear any existing timer via authStore) and handle that branch
explicitly instead of relying on data.exp!. Reference the symbols data.exp,
expiresAt, timeout, authStore.setLogoutTimer, setSafeTimeout, and logout when
implementing the guard and fallback behavior.
| timeout := time.NewTimer(searchPingInterval * time.Second) | ||
| defer timeout.Stop() | ||
| for { | ||
| var err error | ||
| var infoBytes []byte | ||
| select { | ||
| case info := <-response: | ||
| if info == nil { | ||
| return | ||
| } | ||
| infoBytes, err = json.Marshal(info) | ||
| case <-timeout.C: | ||
| // Send a heartbeat packet | ||
| infoBytes = nil | ||
| case <-ctx.Done(): | ||
| return | ||
| } | ||
| if err != nil { | ||
| cancel(err) | ||
| return | ||
| } | ||
| _, err = w.Write(infoBytes) | ||
| if err == nil { | ||
| _, err = w.Write([]byte("\n")) | ||
| } | ||
| if err != nil { | ||
| cancel(err) | ||
| return | ||
| } | ||
| if flusher, ok := w.(http.Flusher); ok { | ||
| flusher.Flush() | ||
| } | ||
| } |
There was a problem hiding this comment.
Timer never resets — heartbeat fires only once.
time.Timer fires exactly once. After the first <-timeout.C receives, subsequent loop iterations will never match that case again, so no further heartbeats are sent. The timer must be reset after each iteration to maintain the keep-alive mechanism.
🐛 Proposed fix to reset the timer
for {
var err error
var infoBytes []byte
select {
case info := <-response:
if info == nil {
return
}
infoBytes, err = json.Marshal(info)
case <-timeout.C:
// Send a heartbeat packet
infoBytes = nil
case <-ctx.Done():
return
}
+ // Reset timer for next heartbeat window
+ if !timeout.Stop() {
+ select {
+ case <-timeout.C:
+ default:
+ }
+ }
+ timeout.Reset(searchPingInterval * time.Second)
if err != nil {
cancel(err)
return
}Alternatively, use time.Ticker instead of time.Timer for periodic heartbeats:
♻️ Alternative: Use time.Ticker for periodic heartbeats
- timeout := time.NewTimer(searchPingInterval * time.Second)
- defer timeout.Stop()
+ ticker := time.NewTicker(searchPingInterval * time.Second)
+ defer ticker.Stop()
for {
var err error
var infoBytes []byte
select {
case info := <-response:
if info == nil {
return
}
infoBytes, err = json.Marshal(info)
- case <-timeout.C:
+ case <-ticker.C:
// Send a heartbeat packet
infoBytes = nil
case <-ctx.Done():
return
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| timeout := time.NewTimer(searchPingInterval * time.Second) | |
| defer timeout.Stop() | |
| for { | |
| var err error | |
| var infoBytes []byte | |
| select { | |
| case info := <-response: | |
| if info == nil { | |
| return | |
| } | |
| infoBytes, err = json.Marshal(info) | |
| case <-timeout.C: | |
| // Send a heartbeat packet | |
| infoBytes = nil | |
| case <-ctx.Done(): | |
| return | |
| } | |
| if err != nil { | |
| cancel(err) | |
| return | |
| } | |
| _, err = w.Write(infoBytes) | |
| if err == nil { | |
| _, err = w.Write([]byte("\n")) | |
| } | |
| if err != nil { | |
| cancel(err) | |
| return | |
| } | |
| if flusher, ok := w.(http.Flusher); ok { | |
| flusher.Flush() | |
| } | |
| } | |
| timeout := time.NewTimer(searchPingInterval * time.Second) | |
| defer timeout.Stop() | |
| for { | |
| var err error | |
| var infoBytes []byte | |
| select { | |
| case info := <-response: | |
| if info == nil { | |
| return | |
| } | |
| infoBytes, err = json.Marshal(info) | |
| case <-timeout.C: | |
| // Send a heartbeat packet | |
| infoBytes = nil | |
| case <-ctx.Done(): | |
| return | |
| } | |
| // Reset timer for next heartbeat window | |
| if !timeout.Stop() { | |
| select { | |
| case <-timeout.C: | |
| default: | |
| } | |
| } | |
| timeout.Reset(searchPingInterval * time.Second) | |
| if err != nil { | |
| cancel(err) | |
| return | |
| } | |
| _, err = w.Write(infoBytes) | |
| if err == nil { | |
| _, err = w.Write([]byte("\n")) | |
| } | |
| if err != nil { | |
| cancel(err) | |
| return | |
| } | |
| if flusher, ok := w.(http.Flusher); ok { | |
| flusher.Flush() | |
| } | |
| } |
| timeout := time.NewTimer(searchPingInterval * time.Second) | |
| defer timeout.Stop() | |
| for { | |
| var err error | |
| var infoBytes []byte | |
| select { | |
| case info := <-response: | |
| if info == nil { | |
| return | |
| } | |
| infoBytes, err = json.Marshal(info) | |
| case <-timeout.C: | |
| // Send a heartbeat packet | |
| infoBytes = nil | |
| case <-ctx.Done(): | |
| return | |
| } | |
| if err != nil { | |
| cancel(err) | |
| return | |
| } | |
| _, err = w.Write(infoBytes) | |
| if err == nil { | |
| _, err = w.Write([]byte("\n")) | |
| } | |
| if err != nil { | |
| cancel(err) | |
| return | |
| } | |
| if flusher, ok := w.(http.Flusher); ok { | |
| flusher.Flush() | |
| } | |
| } | |
| ticker := time.NewTicker(searchPingInterval * time.Second) | |
| defer ticker.Stop() | |
| for { | |
| var err error | |
| var infoBytes []byte | |
| select { | |
| case info := <-response: | |
| if info == nil { | |
| return | |
| } | |
| infoBytes, err = json.Marshal(info) | |
| case <-ticker.C: | |
| // Send a heartbeat packet | |
| infoBytes = nil | |
| case <-ctx.Done(): | |
| return | |
| } | |
| if err != nil { | |
| cancel(err) | |
| return | |
| } | |
| _, err = w.Write(infoBytes) | |
| if err == nil { | |
| _, err = w.Write([]byte("\n")) | |
| } | |
| if err != nil { | |
| cancel(err) | |
| return | |
| } | |
| if flusher, ok := w.(http.Flusher); ok { | |
| flusher.Flush() | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@http/search.go` around lines 25 - 57, The loop uses time.NewTimer (timeout)
which fires once, so heartbeats stop after the first tick; replace the timer
with a time.Ticker (e.g., ticker := time.NewTicker(searchPingInterval *
time.Second), defer ticker.Stop()) and read from ticker.C in the select instead
of timeout.C so the heartbeat case fires repeatedly, leaving the rest of the
logic (reading from response, json.Marshal, writes, cancel on error,
flusher.Flush) unchanged; ensure you rename references from timeout to ticker
and stop the ticker in defer to avoid leaks.
20 participants
Summary by CodeRabbit
Release Notes
New Features
Improvements
Security
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.