Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bad encoding of end of line characters #2938

Closed
aarranz opened this issue Jun 12, 2017 · 9 comments
Closed

Bad encoding of end of line characters #2938

aarranz opened this issue Jun 12, 2017 · 9 comments
Labels
Milestone

Comments

@aarranz
Copy link
Contributor

aarranz commented Jun 12, 2017

Orion allows to insert end of line characters inside string values of attributes, and that is working as expected. But the context broker is not correctly encoding back that EOL characters, that results in invalid JSON response from the context broker.

Example:

$ curl -X POST http://orion.server:1026/v2/entities -d '{"id": "deleteme", "type": "deleteme", "attribute": {"type": "text", "value": "a\nb"}}' -H "Content-Type: application/json"

$ curl -X GET http://orion.server:1026/v2/entities/deleteme                                                                                                                                            
{"id":"deleteme","type":"deleteme","attribute":{"type":"text","value":"a
b","metadata":{}}}

The same happens to the ' \r' character used also as EOL character.

Tested with:

{
"orion" : {
  "version" : "1.7.0-next",
  "uptime" : "16 d, 16 h, 45 m, 13 s",
  "git_hash" : "7ac993b3c00b3b1dccd629c1ced380623d487b16",
  "compile_time" : "Fri May 26 10:13:54 UTC 2017",
  "compiled_by" : "root",
  "compiled_in" : "b99744612d0b"
}
@kzangeli
Copy link
Member

I'm not so sure this is an error ...
\n is one character, with the value 10 (0xA) and when printed on a screen it provokes a newline. Similar with \r.
I think that if you want two characters: a backslash and an n, you need to escape the backslash: \\n.

This needs some thorough thought though :-)

@fgalan
Copy link
Member

fgalan commented Jun 12, 2017

I understand the problem is in the round trip. I mean, if a given attribute at CB has been set to:

"value": "foo \n bar"

what the user wants to be retrieved is

"value": "foo \n bar"

and not

"value": "foo 
 bar"

which, in addition, is ilegal JSON (have a look with https://jsonlint.com)

@aarranz
Copy link
Contributor Author

aarranz commented Jun 12, 2017

Thanks @fgalan, that's the point. In JSON the character with value 10 (0xA, line feed) is encoded as \n and the character with value 13 (0xD, carriage return) is encoded as \r. Using a line feed or a carriage return character directly inside an string is illegal in JSON.

@kzangeli
Copy link
Member

Reading a little about this, i.e.
https://stackoverflow.com/questions/2392766/multiline-strings-in-json

it seems that orion should modify the incoming one byte \n into two bytes \ and n.

A bit strange to modify the value that the user has given ...
However, it seems to be the right thing to do. After all, JSON isn't a binary format, but text based.

Same same with the other 8 permitted escape sequences, presented here: http://json.org/

@fgalan
Copy link
Member

fgalan commented Jun 12, 2017

it seems that orion should modify the incoming one byte \n into two bytes \ and n.

Not sure... In fact, given that one-byte-\n is illegal JSON, CB should return a "Illegal JSON" error. In fact, it is not working right now in that way? Is not rapidjson detecting that as illegal JSON?

I think the fix should be in the other direction. I mean, assuming that we have (in the DB) two bytes \ and n what we should return is two bytes \ and n, not one byte newline (as I understand is happening right now).

@aarranz
Copy link
Contributor Author

aarranz commented Jun 12, 2017

I don't know the internals of Orion, but in the \n in the request is made up of two characters. Usually, JSON parsers converts those two characters into the real character, in that case a line feed character of one byte:

$ curl -v http://orion.server:1026/v2/entities -d '{"id": "deleteme", "type": "deleteme", "attribute": {"type": "text", "value": "ab"}}' -H "Content-Type: application/json"
*   Trying X.X.X.X...
* TCP_NODELAY set
* Connected to orion.server (X.X.X.X) port 1026 (#0)
> POST /v2/entities HTTP/1.1
> Host: ficodes.com:1026
> User-Agent: curl/7.51.0
> Accept: */*
> Content-Type: application/json
> Content-Length: 84
>
* upload completely sent off: 84 out of 84 bytes
< HTTP/1.1 201 Created
< Connection: Keep-Alive
< Content-Length: 0
< Location: /v2/entities/deleteme?type=deleteme
< Fiware-Correlator: c63796bc-4f62-11e7-8550-0242ac120003
< Date: Mon, 12 Jun 2017 11:32:10 GMT
<
* Curl_http_done: called premature == 0
* Connection #0 to host ficodes.com left intact
% curl -v http://orion.server:1026/v2/entities -d '{"id": "deleteme", "type": "deleteme", "attribute": {"type": "text", "value": "a\nb"}}' -H "Content-Type: application/json"
*   Trying X.X.X.X...
* TCP_NODELAY set
* Connected to ficodes.com (X.X.X.X) port 1026 (#0)
> POST /v2/entities HTTP/1.1
> Host: orion.server:1026
> User-Agent: curl/7.51.0
> Accept: */*
> Content-Type: application/json
> Content-Length: 86
>
* upload completely sent off: 86 out of 86 bytes
< HTTP/1.1 201 Created
< Connection: Keep-Alive
< Content-Length: 0
< Location: /v2/entities/deleteme?type=deleteme
< Fiware-Correlator: 6c4da448-4f62-11e7-bacf-0242ac120003
< Date: Mon, 12 Jun 2017 11:29:40 GMT
<
* Curl_http_done: called premature == 0
* Connection #0 to host ficodes.com left intact

As you can see, there is a difference of two bytes in the Content-Length header

@kzangeli
Copy link
Member

ok, makes sense, \nas one character should be prohibited, only the sequence of two chars should be permitted.
About rapidjson ... yes, rapidjson should detect this and flag an error.

We need to look into this to understand what is really happening.

@fgalan
Copy link
Member

fgalan commented Jun 12, 2017

@aarranz , how does the entity look like in the DB in both cases? I mean, db.entities.find({"_id.id": "deleteme"})

@fgalan
Copy link
Member

fgalan commented Sep 27, 2018

This issues seems to be duplicate #3280 (it seems we created #3280 without remembering this one was here :)

PR #3285 fixed #3280 and I understand that also this one. Thus, I'm closing it.

If I'm wrong, please don't hesitate to reopen the issue or comment.

@fgalan fgalan closed this as completed Sep 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants