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

New line in string raises decode error #28

Closed
jaimeiniesta opened this issue Feb 27, 2018 · 2 comments
Closed

New line in string raises decode error #28

jaimeiniesta opened this issue Feb 27, 2018 · 2 comments

Comments

@jaimeiniesta
Copy link

jaimeiniesta commented Feb 27, 2018

Hello, we're replacing Poison by Jason in a project and it all works great but we found a test case where Poison didn't complain, but Jason does, which involves new lines inside JSON strings.

I've added a test to reproduce it here:

jaimeiniesta@0e03d04

Now, for a little more context, we use helpers like that in out tests:

defp unauthorized_response_body do
  """
  {
      "error": "invalid_client",
      "error_description": "Client authentication failed due to unknown client, no client
                            authentication included, or unsupported authentication method."
  }
  """
end

The newline there is just for readability purposes, but it's what raises the decoding error.

I think that JSON allows to include \n in strings, and Poison allows that:

iex(4)> json =    """
...(4)>     {
...(4)>         "error": "invalid_client",
...(4)>         "error_description": "Client authentication failed due to unknown client, no client
...(4)>                               authentication included, or unsupported authentication method."
...(4)>     }
...(4)>     """
"{\n    \"error\": \"invalid_client\",\n    \"error_description\": \"Client authentication failed due to unknown client, no client\n                          authentication included, or unsupported authentication method.\"\n}\n"
iex(5)> Poison.decode(json)
{:ok,
 %{"error" => "invalid_client",
   "error_description" => "Client authentication failed due to unknown client, no client\n                          authentication included, or unsupported authentication method."}}
@michalmuskala
Copy link
Owner

michalmuskala commented Feb 27, 2018

According to the JSON spec (RFC 8259) control characters (including newline character - U+000A) must be escaped.

The representation of strings is similar to conventions used in the C
family of programming languages. A string begins and ends with
quotation marks. All Unicode characters may be placed within the
quotation marks, except for the characters that MUST be escaped:
quotation mark, reverse solidus, and the control characters (U+0000
through U+001F).

The current implementation strictly follows the spec and does not allow unescaped newlines inside strings. It's up to debate if we should follow the spec here, I think we should. But we also should at least mention this in the "Differences to Poison" section of documentation.

There are also explicit tests to refuse such behaviour in the JSON Test Suite.

@jaimeiniesta
Copy link
Author

Thanks for the explanation! Yes, I also think that we should follow the spec strictly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants