search

evilGmonkey / opendatakit

Automatically exported from code.google.com/p/opendatakit
0 stars 0 forks source link

Couple of ODK Collect Fixes #324

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
from nic pottier of nyaruka.com fame...

I recently updated the RapidSMS-XForms app to work with the new authentication 
features in ODK Collect 1.1.7:
  https://github.com/nyaruka/rapidsms-xforms

Found a couple little niggles in the process:
 1) The parser for formList was never saving the form id passed in response, this was just a simple variable scoping issue.  Fixed in this commit:
 https://github.com/nicpottier/opendatakit.collect/commit/ba619f808967c9998781fba64e465b2d9e5defaf

 2) According to the HTTP spec, HEAD requests should return the same return code as a normal POST or GET.  Right now, the ODK Collect client only moves on if you return a 204 to a form submission.  I changed this to also allow 201 and 202, since those are the return codes for successful form submissions:
 https://github.com/nicpottier/opendatakit.collect/commit/c0fe78255d35aebbfbf47bfe11a2e7f10ae8ad38

I noticed one other thing but I'm not sure whether it is expected or not.  To 
me it seems like there is a ton of HTTP traffic taking place needlessly when 
using authentication.  For example, submitting a form submission looks like 
this in my HTTP logger:
         HEAD /submission (no nonce)  -- server returns 401
         HEAD /submission (w/nonce) -- server returns 204
         POST /submission (no nonce) -- server returns 401
         POST /submission (w/nonce) -- server return 201

Shouldn't we be sending the nonce right from the getgo?  Four round trips for a 
single submission seems like a lot, especially given the latencies in the 
countries where we are using this.

Original issue reported on code.google.com by yanokwa on 17 Sep 2011 at 8:28

GoogleCodeExporter commented 9 years ago 
w.r.t. #2: Since we are using digest authentication, the server must initiate 
the interaction by specifying the realm and a server nonce, after which the 
client can formulate a cnonce and an appropriate authenticated reply.  The HEAD 
requests, while they do incur a network round-trip, have minimal size and are 
used to negotiate the channel security (http/https) and authentication (if any) 
prior to sending the much larger POST.  When you are submitting multiple forms, 
Collect is sending the HEAD request only once for a particular server, so the 
bandwidth/latency costs are negligible.

W.r.t. return code -- a HEAD request CAN return a different response code from 
a GET - the HTTP spec requires that the header information be identical, but 
the response status line (and status code) is not a header.  In this case, 
since the two OpenRosa return codes are 201 (created) and 202 (accepted), and 
the appropriate choice is entirely based upon the POST-body missing from the 
HEAD request, it would be inappropriate for the server to return either value. 
201 means the server fully processed the submission. 202 means it is being held 
but is in limbo. ( 
https://bitbucket.org/javarosa/javarosa/wiki/FormSubmissionAPI ).  Hence the 
appropriate return of 204 (no content).

Original comment by mitchellsundt@gmail.com on 21 Sep 2011 at 5:28

GoogleCodeExporter commented 9 years ago 
Just an update on #1 here.. it turns out this actually prevents you from seeing 
more than one form when using the old format, as because the formId is always 
null, every form past the first overwrites the previous in the map.  The key in 
the map is the formId, so it is stuff 'null' as the key over and over.

That really should be a critical bug for any release of the new version.

Original comment by nicpott...@gmail.com on 6 Oct 2011 at 10:13

GoogleCodeExporter commented 9 years ago 
Oh and on the return code, I don't much care, but I do think you are misusing 
HEAD here..  By contract HEAD is already idempotent, ie, it is already known 
that it will not actually DO anything, but rather is a 'ping' to see if it 
tried to do something whether it would work.

http://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html

I think it is definitely more in the spirit of the spec to have it return the 
exact same status code as actual submission.  Changing the status code is a 
really odd use case in combination with HEAD.

Original comment by nicpott...@gmail.com on 6 Oct 2011 at 10:19

GoogleCodeExporter commented 9 years ago

Original comment by yanokwa on 25 May 2012 at 3:46

GoogleCodeExporter commented 9 years ago

Original comment by yanokwa on 25 May 2012 at 8:14

GoogleCodeExporter commented 9 years ago 
Fixed the missing formId issue in 1.2 codebase, where it is informational. The 
map was changed in 1.1.7 to resolve this bug differently.

Confirmed that in 1.2 (and 1.1.7, as this code hasn't changed) that the 
transmission is:

HEAD (fail - server supplies NONCE)
[user confirms username and password]
HEAD (fail - server supplies NONCE)
HEAD (success - using NONCE)
POST (success)

i.e., no extra POST.

Original comment by mitchellsundt@gmail.com on 30 Jun 2012 at 1:01