Fix json decoding

[warmwaffles]

We have been running into issues where the decoding was broken.

The issue was that you are trying to fetch Content-Type from the headers when it was being returned as content-type. Rather than trying to figure out the content type to be parsed. We should just try to decode it as json, and then if it fails, just return the body.

[techgaun]

I wonder if we should try to retrieve both versions because Content-Type was probably working in the past.

[warmwaffles]

We upgraded from v0.6.0 to v0.8.0 and the issue occurred.

Also, can you start tagging the release commits git tag v0.8.0 etc... would make for an easier to time diff ranges of commits.

[benkimpel]

Hi @techgaun and @warmwaffles

I have a similar issue after upgrade to 0.8.0, but debugging showed that headers are coming in as {"Content-Type", "application/json; charset=utf-8"} which fails the equality check here.

The Content-Type header stores a media type which contains the type/subtype followed by optional parameters (I'm seeing charset in this case).

What if we do something like...

  defp is_json_content_type?(headers) do
    get_media_type(headers) == "application/json"
  end

  defp get_media_type(headers) do
    case List.keyfind(headers, "Content-Type", 0) do
      {"Content-Type", media_type} when is_binary(media_type) ->
        # Split off any parameters leaving just type/subtype.
        # Trim any extras spaces just in case.
        # Downcase because type/subtype are case insensitive.
        media_type
        |> String.split(";")
        |> List.first()
        |> String.trim()
        |> String.downcase()

      _ ->
        # This media type may be assumed if there is no content type sent
        "application/octet-stream"
    end
  end

If headers are coming in as lowercase as noted by @warmwaffles then we could modify the search in get_media_type/1 a bit to cover both cases, but so far I'm only seeing the header come in as title cased.

[benkimpel]

If headers are coming in as lowercase as noted by @warmwaffles then we could modify the search in get_media_type/1 a bit to cover both cases, but so far I'm only seeing the header come in as title cased.

Something like this would do to cover both cases...

defp get_media_type(headers) do
  Enum.find_value(headers, "application/octet-stream", fn
    {header, media_type} when header in ["Content-Type", "content-type"] ->
      # ... the split, downcase, etc.

    _ ->
      false
  end)
end

[warmwaffles]

Or hear me out, we just decode it always as json and disregard the content type. 😉

[benkimpel]

Or hear me out, we just decode it always as json and disregard the content type. 😉

I assumed there was some reason that there was a JSON check in the first place. If there is only one API response type (json) then sure that solution makes sense, but if there are other content types from Auth0 API then why bother parsing HTML as JSON just to fail and return the original HTML? Seems expensive.

Anyhow, just trying to help since I saw this was not merged and it's causing me trouble. Good luck to you!

[techgaun]

@warmwaffles I think that's fair. I can't recall why we had a check but seems like its json always anyway.

[warmwaffles]

I haven't run into a case yet where it wasn't returning json. At least in our production instance of this.

[techgaun]

Sounds like that check was added for sending password change e-mail. So looks like we do need it. I however wanted to support both versions of content-type. I did a PR https://github.com/techgaun/auth0_ex/pull/44/files so maybe you guys can chime in if it looks good to you all.

[techgaun]

closing this in favor of what was merged previously