Repository navigation
Conversation
roger2hk
left a comment
roger2hk
left a comment
There was a problem hiding this comment.
I think Gemini reads the spec comment in the code which is misleading.
| name: "200 ok (uploadEnd reached before pendingSize)", | ||
| origin: testOrigin, | ||
| mockTarget: &mockTarget{ | ||
| addEntriesFunc: func(ctx context.Context, uploadStart, uploadEnd uint64, ticket []byte, next func() (*tessera.MirrorPackage, error)) (uint64, uint64, []byte, []byte, error) { | ||
| return testUploadEnd, testPendingSize, []byte(testNewTicket), nil, nil | ||
| }, | ||
| }, | ||
| wantStatus: http.StatusOK, | ||
| wantBody: "", |
There was a problem hiding this comment.
This case expects 200 OK with an empty body, which the tlog-mirror spec doesn't allow. A successful add-entries response "MUST be … a sequence of one or more note signature lines", and "the mirror MUST persist the new checkpoint before responding".
Also, the MirrorTarget.AddEntries can't return this combination (nextEntry == uploadEnd, pendingSize != uploadEnd, no cosigs, no error). It takes the pending checkpoint from the ticket and returns ErrConflict unless that checkpoint's size equals uploadEnd.
There was a problem hiding this comment.
The spec here does not align with the tlog-mirror specification which doesn't say returning an empty response body in 200 OK.
The implementation here is correct to return 200 OK with the cosignatures.
Per C2SP tlog-mirror, an add-entries request completing up to upload_end must receive a 200 OK status code even if upload_end < pendingSize.
…rror spec Audit all SPEC comments in the mirror HTTP handler to include exact URLs and quotes from c2sp.org/tlog-mirror and c2sp.org/tlog-cosignature. Specifically, fix a misleading SPEC comment that suggested a 200 OK response on add-entries could return an empty response body without cosignatures; in c2sp.org/tlog-mirror, a 200 Success response on add-entries must always return the mirror's cosignatures over the newly published checkpoint. Also remove the corresponding test expecting an empty body on 200 OK.
phbnf
changed the title
|
Well spotted @roger2hk! I have therefore repurposed this PR to remove the erroneous spec reference, and to better format all other references in this file. |
This branch has not been deployed
3 participants
Towards #945.
Per C2SP tlog-mirror, an add-entries request completing up to upload_end must receive a 200 OK status code even if upload_end < pendingSize.
Courtesy of Gemini, and it looks legit to me.