Invalid JSON (quotes not escaped)

[Hawkings]

In some cases, Orion returns an HTTP response with Content-Type: application/json but with the body being invalid json.

For example, the response for the query /v2/entities?foo" (note the " at the end) is:

HTTP/1.1 400 Bad Request
Connection: Keep-Alive
Content-Length: 81
Content-Type: application/json
Fiware-Correlator: 2a1d9306-6332-11e7-a995-080027f6529d
Date: Fri, 07 Jul 2017 16:34:36 GMT

{"error":"BadRequest","description":"Empty right-hand-side for URI param /foo"/"}

Whose body is not valid JSON. I think this problem could be solved by escaping the quotes.

[fgalan]

Thanks for the feedback! It seems that the logic that avoids rendering fordidden chars (note that " is one of them, see https://fiware-orion.readthedocs.io/en/master/user/forbidden_characters/index.html) is failing in this particular case.

[fgalan]

@arigliano , the function htmlEscape() is the one that translates forbidden chars into allowed chars. Have a look around the code to see how it is being used.

In this issue it has to be found why this is not being applied to the "description" field in the case of the response shown above. And fix it.

• Impact on documentation: none
• Impact of unittest: almost sure none, but not fully sure. I don't remember any unit test related with this, but let's see...
• Impact of functional tests:
  • Existing .test should work without any expected change. If the modification break some existing test, please comment on this issue (in that case we would probably need to reevaluate the solution approach)
  • A new test mirroring the use case described by @Hawkings in his original post should be added using (for instance) the directory cases/2855_quote_in_url_not_escaped. Only one step reuired in that .test. You could start with some existng .test (the case is so simple that any one including a GET request should work).

Don't hesitate to ask if you have some doubt regarding implementation.

[arigliano]

@fgalan Thank you for the suggestion. I've found the problem in the uriArgumentGet method.
In particular, it appends "ckey" to "Empty right-hand-side for URI param..." string, in order to pass it as "details" in the OrionError constructor. The real problem is in OrionError::toJson() method, because, unlike the render(), it prints out JSON_STR(details) , without perfoming any check about forbidden chars.
I propose two ways to fix this issue:

• The immediate solution would be to wrap the "ckey", when creating the details string, with either jsonInvalidCharsTransformation() or better JSONHelper::toJsonString() (rest.cpp:114)

• A more generic fix, in order to avoid such misbehavior in other parts using OrionError::toJson(), should be to fix this method, by introducing a filtering with either jsonInvalidCharsTransformation() or better JSONHelper::toJsonString()

[fgalan]

Thank you for so precise and illustrative analysis of the cause of the problem :)

I think the second option is the best one. That is, fixing the OrionError::toJson() to apply invalid chars transformation to its parameters. Among the two sub-options (jsonInvalidCharsTransformation or JSONHelper::toJsonString) maybe the best one is better as it doesn't requires to create a short-lived object (the JSONHelper) for just a invalidad char transformation, but I don't have an strong opinion on that.

[fgalan]

On a second thought, I think the point to apply the fix is correct (OrionError::toJson) but the function to use is not jsonInvalidCharsTransformation/JSONHelper::toJsonString.

Note these functions do the transformation json_ivalid -> \xx. However, what it should be applied is the function that transform orion_invalid -> &xx as, by design, Orion shouldn't response with any forbidden char and

"description": "Empty right-hand-side for URI param /foo\"/", 

uses "

In fact, I think is should be something like

"description": "Empty right-hand-side for URI param /foo&xx;/", 

@kzangeli will elaborate later today on which exact function to use to implment the orion_invalid -> &xx transformation.

[kzangeli]

We have a function called htmlEscape that does this 'uriencode'.
Keep in mind that this function allocates a buffer for the new, escaped string, and this string (which is returned by the function) needs to be freed after usage.

[fgalan]

Fixed by PR #3012