Conversation
Import github.com/markmnl/fmsgd/pkg/fmsg and use it in the Send handler to build the wire-format fmsg.Header from in-memory fields, then call GetMessageHash() to obtain the SHA-256 over the encoded header + decompressed body + attachment data. The hash is written to msg.sha256 in the same UPDATE that sets time_sent. Also update fetchMessage to SELECT add_to_from so the Send handler has the correct AddToFrom value when building the header for add-to messages. This fixes the production error where POST /fmsg with a pid failed because the parent message had sha256 = NULL and the DB trigger populate_msg_psha256_from_pid requires a non-null sha256 on any sent parent message. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the message send flow to compute and persist a deterministic SHA-256 hash for a message at send-time, using the canonical fmsg wire-format header/body/attachments encoding, and ensures add_to_from is available when constructing the header.
Changes:
- Import
github.com/markmnl/fmsgd/pkg/fmsgand computemsg.sha256duringPOST /fmsg/:id/sendin the same DB update that setstime_sent. - Load additional DB fields needed for hash computation (
add_to_from,psha256, and attachment header metadata). - Add the
github.com/markmnl/fmsgdmodule dependency.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/handlers/messages.go |
Compute and store message sha256 on send; extend fetchMessage to select add_to_from. |
src/go.mod |
Adds the github.com/markmnl/fmsgd module requirement. |
src/go.sum |
Adds checksums for the newly required github.com/markmnl/fmsgd module. |
Comments suppressed due to low confidence (2)
src/handlers/messages.go:857
- Attachment metadata is loaded with errors silently ignored (
attErrignored, per-rowScanerrors ignored, andattRows.Err()is never checked). If any scan/query issue occurs, the hash may be computed without some attachments and then written tomsg.sha256, permanently diverging from the real wire-format hash. Please fail the request if attachment metadata cannot be read completely/correctly.
attRows, attErr := h.DB.Pool.Query(ctx,
"SELECT flags, type, filename, filesize, filepath FROM msg_attachment WHERE msg_id = $1 ORDER BY position",
msgID,
)
if attErr == nil {
for attRows.Next() {
var a attachMeta
if scanErr := attRows.Scan(&a.flags, &a.typ, &a.filename, &a.filesize, &a.path); scanErr == nil {
attachments = append(attachments, a)
}
}
attRows.Close()
}
src/handlers/messages.go:843
attachMeta.filesizeis declared asint32, but attachments are written with anint64byte count (io.Copyresult) and DBfilesizeis likely a 64-bit integer. If a value is out ofint32range, pgx will return a scan error (currently ignored) and the attachment will be omitted from the hash. Useint64for scanning and validate it fits into the wire/header size type before casting.
type attachMeta struct {
flags int16
typ string
filename string
filesize int32
path string
}
Comment on lines
+830
to
+834
| // Fetch psha256 (the parent message's sha256 hash used as the pid wire field). | ||
| var psha256 []byte | ||
| if existing.HasPid { | ||
| _ = h.DB.Pool.QueryRow(ctx, "SELECT psha256 FROM msg WHERE id = $1", msgID).Scan(&psha256) | ||
| } |
Contributor
There was a problem hiding this comment.
Fixed in commit 18ccd92:
psha256lookup now useserr =instead of_ =, returning 5xx on any DB error- Added a length check: if
psha256is not exactly 32 bytes, the handler returns 5xx with a clear error message - Attachment query errors now return 5xx instead of silently treating it as "no attachments"
- Per-row scan errors during attachment iteration now close the rows and return 5xx
- Added
attRows.Err()check after the iteration loop
…ndler Agent-Logs-Url: https://github.com/markmnl/fmsg-webapi/sessions/a9b68060-a134-43ee-a020-1ac1016d5e81 Co-authored-by: markmnl <2630321+markmnl@users.noreply.github.com>
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.
Import github.com/markmnl/fmsgd/pkg/fmsg and use it in the Send handler to build the wire-format fmsg.Header from in-memory fields, then call GetMessageHash() to obtain the SHA-256 over the encoded header + decompressed body + attachment data. The hash is written to msg.sha256 in the same UPDATE that sets time_sent.
Also update fetchMessage to SELECT add_to_from so the Send handler has the correct AddToFrom value when building the header for add-to messages.
This fixes the production error where POST /fmsg with a pid failed because the parent message had sha256 = NULL and the DB trigger populate_msg_psha256_from_pid requires a non-null sha256 on any sent parent message.