fix: return error instead of panicking on invalid date input in parse_date()#7070
Closed
mango766 wants to merge 2 commits intodani-garcia:mainfrom
Closed
fix: return error instead of panicking on invalid date input in parse_date()#7070mango766 wants to merge 2 commits intodani-garcia:mainfrom
mango766 wants to merge 2 commits intodani-garcia:mainfrom
Conversation
`Cipher::to_json()` returns `Result<Value, Error>` but its match arm for
unknown `atype` values called `panic!("Wrong type")` instead of
propagating an error. This means if a cipher with an invalid/unknown type
ends up in the database (via direct DB edits, data migration issues, or
future type additions in the upstream Bitwarden protocol), the entire
server process would crash on the next sync request.
Replace the `panic!` with `err!()` so callers receive a proper `Err` and
can handle or log it gracefully without taking down the server.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The parse_date() utility function called .unwrap() directly on user-
controlled date strings from HTTP query parameters, allowing any
authenticated user to crash the server with a malformed RFC3339 date.
Change parse_date() to return Result<NaiveDateTime, Error> and update
all callers in events.rs to propagate the error with ?, returning a
400 Bad Request instead of panicking. The one call-site using a
hardcoded literal (sends.rs) uses .expect() with an explanatory message.
Affected endpoints:
- GET /organizations/{id}/events?start=...&end=...
- GET /ciphers/{id}/events?start=...&end=...
- GET /organizations/{id}/users/{id}/events?start=...&end=...
- POST /collect (event collection endpoint)
Collaborator
|
I'm not sure this really really is needed, though it might be a bit nicer. |
BlackDex
approved these changes
Apr 18, 2026
Collaborator
|
Going to close this, it's not a real issue, and no response regarding the mixing of PR's |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #7069
What's happening
parse_date()insrc/util.rscalls.unwrap()on aDateTime::parse_from_rfc3339()result, where the input comes directly from HTTP query parameters or JSON body fields. Sending a malformed RFC3339 date string to any of the event endpoints panics the request handler thread.Affected endpoints:
GET /api/organizations/{id}/events?start=...&end=...GET /api/ciphers/{id}/events?start=...&end=...GET /api/organizations/{id}/users/{id}/events?start=...&end=...POST /events/collectFix
Changed
parse_date()to returnResult<NaiveDateTime, Error>and updated all callers inevents.rsto propagate the error with?, which results in a400 Bad Requestresponse instead of a panic.The one call site with a hardcoded literal (
sends.rs, epoch timestamp used for an anonymous push device) now uses.expect()with an explanatory message since that string is always valid.Changes
src/util.rs:parse_date()now returnsResult<NaiveDateTime, crate::Error>and uses pattern matching instead of.unwrap()src/api/core/events.rs: allparse_date()calls use?to propagate errorssrc/api/core/sends.rs: hardcoded constant call-site uses.expect()with explanation