624714 - J-PAKE client: Prevent double PUTs by supplying an If-Match header

Status: RESOLVED WONTFIX

(Firefox :: Sync, defect)

Description

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

This is a fix for the first problem described in https://bugzilla.mozilla.org/show_bug.cgi?id=618969#c0

By supplying an If-Match header for the PUT requests, we can prevent messages being pushed to the server multiple times and potentially out of order.

The idea is that we set the If-Match header to the MD5 hash of the last message that the client succesfully GET. This means that the new message will not be POST if it is already on the server OR if the other side has already uploaded the next message in sequence.

Comment 1

[Philipp von Weitershausen [:philikon]]

I'm guess all the handling is done on the server, so nothing will change in terms of the HTTP responses to those PUT requests, right?

Comment 2

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

If you PUT a message, and it fails because of a timeout of network error, retry the PUT with the If-Match header. (It is probably safe to always set the If-Match)

Set the If-Match header to the MD5 hash of the last message that you GET before that.

If you receive a 412 on a retry then it means that the message was PUT successfully and the other side has already received it and uploaded the next response message. In that case just continue with the flow of the protocol.

Comment 3

[Tarek Ziadé (:tarek)]

Yeah what Stefan says. You just need to handle 412 codes. I'll update the server API doc accordingly once we're all on the same page.

Comment 4

[Philipp von Weitershausen [:philikon]]

(In reply to comment #2)
> If you PUT a message, and it fails because of a timeout of network error, retry
> the PUT with the If-Match header. (It is probably safe to always set the
> If-Match)

So we'll always set If-Match to the latest Etag.

> Set the If-Match header to the MD5 hash of the last message that you GET before
> that.

Your repeated mentioning of MD5 worries me :). I hope you're talking about the Etag that the server sends us here

> If you receive a 412 on a retry then it means that the message was PUT
> successfully and the other side has already received it and uploaded the next
> response message. In that case just continue with the flow of the protocol.

Ok. That makes sense.

Comment 5

[Philipp von Weitershausen [:philikon]]

(In reply to comment #4)
> So we'll always set If-Match to the latest Etag.

By that I meant the Etag of the last GET.

Comment 6

[Philipp von Weitershausen [:philikon]]

In bug 689428 I've added If-Match / If-None-Match headers to PUT request. I believe that takes care of just half of this bug, though, as Firefox doesn't actually attempt to PUT the record again if it encounters a network error. Once it does that, it should accept a 412 Precondition Failed response on the 2nd attempt.

Comment 7

[Chris Karlof [:ckarlof]]

This bug only likely affects Sync 1.1, and it was shut down in the fall of 2015 (https://blog.mozilla.org/services/2015/07/31/shutting-down-the-legacy-sync-service/).