Bug #17582

Cannot assign names that start with '['

Added by Lucas Di Pentima 9 months ago. Updated 9 months 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.2Resolved05/03/2021

Associated revisions

Revision 34ed87de
Added by Lucas Di Pentima 9 months 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 9 months 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 9 months ago

  • Description updated (diff)

#2 Updated by Lucas Di Pentima 9 months ago

  • Status changed from New to In Progress

#3 Updated by Lucas Di Pentima 9 months 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 9 months 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 9 months 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 9 months ago

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

#7 Updated by Peter Amstutz 9 months ago

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

#8 Updated by Peter Amstutz 9 months ago

#9 Updated by Tom Clegg 9 months 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 9 months 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 9 months 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 9 months ago

This LGTM, thanks!

#15 Updated by Lucas Di Pentima 9 months ago

  • Status changed from In Progress to Resolved

Also available in: Atom PDF