Bug #17582

Cannot assign names that start with '['

Added by Lucas Di Pentima 20 days ago. Updated about 17 hours ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
API
Target version:
Start date:
05/11/2021
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
-
Release relationship:
Auto

Description

When trying to assign a name that starts with opening square brackets, the API responds with the following error:

invalid character 'X' looking for beginning of value

This seems to be related to JSON lists, so the solution probably should be that wb2 needs to properly encode the [ char.

Workbench1 and the CLI tools don't have this issue.


Subtasks

Task #17592: Review 17582-square-brackets-on-names-fixResolvedLucas Di Pentima


Related issues

Blocks Arvados - Story #17512: Release Arvados 2.2New05/03/2021

Associated revisions

Revision 34ed87de
Added by Lucas Di Pentima 1 day ago

Merge branch '17582-square-brackets-on-names-fix'
Closes #17582

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <>

Revision 70b1654a
Added by Lucas Di Pentima about 16 hours ago

Merge branch '17582-cypress-test-fix'
Refs #17582

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <>

History

#1 Updated by Lucas Di Pentima 20 days ago

  • Description updated (diff)

#2 Updated by Lucas Di Pentima 16 days ago

  • Status changed from New to In Progress

#3 Updated by Lucas Di Pentima 16 days ago

Sending JSON data via curl is working correctly:

$ curl -v -X PUT --data '{"collection":{"name":"[Deprecated]"}}' -H "Authorization: OAuth2 xxxtokenxxx" -H "Content-Type: application/json" https://ce8i5.arvadosapi.com/arvados/v1/collections/ce8i5-4zz18-9m0e1jso8boqqxy

OTOH, Chrome's inspector shows what seems to be a correct request body, for example while creating a new collection

{"name":"{A new coll","description":null,"preserve_version":true}

...so I'm not sure what's going on yet.

#4 Updated by Lucas Di Pentima 16 days ago

I've found different behaviors depending on the shape of the passed JSON payload.

This is what wb2 is trying to pass (failing request):

{"name":"[Some name]"}

...but the following does work ok:

{"collection":{"name":"[Some name]"}}

...so I'm wondering now whether this is a client or a backend bug.

#5 Updated by Lucas Di Pentima 15 days ago

From chat:

Tom: I suspect it's either controller or rails that needs fixing. foo=[true] and foo="bar" are ambiguous in a form-encoded body, but {"foo":"[bar]"} is not ambiguous in a json-encoded body, so we should be able to handle that correctly.
Suspect part/all of the fix will be making controller use a JSON request body instead of formencoded. I'm not sure why we ever would have used formencoded. I vaguely recall it could be that _method=GET didn't work in a JSON body... if that's the case, maybe we can use the X-Http-Method-Override header instead?

#6 Updated by Lucas Di Pentima 15 days ago

  • Target version changed from 2021-04-28 bughunt sprint to 2021-05-12 sprint

#7 Updated by Peter Amstutz 15 days ago

  • Category changed from Workbench2 to API
  • Project changed from Arvados Workbench 2 to Arvados

#8 Updated by Peter Amstutz 9 days ago

#9 Updated by Tom Clegg 2 days ago

in (*lib/controller/router.router)loadRequestParams():

                                // The Ruby "arv" cli tool sends a                                                                                     
                                // JSON-encode params map with                                                                                         
                                // JSON-encoded values.                                                                                                

It seems that in order for "arv" to work, we double-decode a top level params, so this works:

{"properties":"{\"foo\":\"bar\"}"}

...but we don't double-decode lower levels, so this also works:

{"collection":{"name":"{\"foo\":\"bar\"}"}}

fwiw the examples at https://doc.arvados.org/main/api/requests.html put updated fields at lower levels.

Is it really annoying to change workbench2 to send a body like {"collection":{"name":"[foo]"}} instead of {"name":"foo"}?

Other possibilities:
  • detect afflicted versions of Ruby CLI/SDK (via user-agent?), fix current Ruby CLI/SDK, and turn off this weird behavior for normal clients
  • treat malformed JSON as a string, which would probably solve most real-world cases (like "[foo]") but would still break confusingly on other cases (like "[]").

#11 Updated by Lucas Di Pentima 1 day ago

Ok, so as suggested by Tom, I fixed it on the wb2 side: arvados-workbench2|c0d7c33 - branch: 17582-square-brackets-on-names-fix
Test run: https://ci.arvados.org/view/Developer/job/developer-tests-workbench2/407/

  • Updated the common resource service to send payloads of the form {rsc_name: { data }} instead of just a JSON object with the plain data.
  • Updated tests.

#13 Updated by Lucas Di Pentima 1 day ago

Fixes cypress test to support testing names with {curly braces} at arvados-workbench2|52517c0
Test run: https://ci.arvados.org/view/Developer/job/developer-tests-workbench2/412/

#14 Updated by Tom Clegg 1 day ago

This LGTM, thanks!

#15 Updated by Lucas Di Pentima 1 day ago

  • Status changed from In Progress to Resolved

Also available in: Atom PDF