lucky_cli icon indicating copy to clipboard operation
lucky_cli copied to clipboard

Add `set_default_content_type` to `ApiAction`

Open paulcsmith opened this issue 6 years ago • 6 comments

If a client forgets to set a content type, we should set a default for them so that Lucky knows what kind of params to expect.

I think we can add a before set_default_content_type to ApiAction and the method would look something like this:

def set_default_content_type
  response.headers["Content-Type"] ||= "application/json"
  continue
end

@jwoertink could you try this in your app and leave feedback on if it works/suggested improvements?

paulcsmith avatar Jan 31 '19 16:01 paulcsmith

This doesn't work for me. I tried both response.headers and request.headers, but if I don't set them in the curl request, they just come through as "application/x-www-form-urlencoded". I'm guessing curl sets that internally.

This original issue came up from trying to use the javascript fetch api. Setting the header properly still seems to send the wrong content type which causes it to blow up. Because of a CORS issue, the Content-Type is still being set, but maybe by the browser?

#fails
curl -XPOST "http://localhost:5000/thing" -d '{"namespace":{"one":"1","two":"2"}}'

#works
curl -XPOST "http://localhost:5000/thing" -d "namespace:one=1&namespace:two=2"

#works
curl -XPOST -H "Content-Type: application/json" "http://localhost:5000/thing" -d '{"namespace":{"one":"1","two":"2"}}'

EDIT: this may have been related to a different issue. Now that I know what's going on, I need to try again and report back.

jwoertink avatar Jan 31 '19 17:01 jwoertink

Did a bit of investigations here. Curl sends */* by default. What other frameworks do is treat */* as empty and choose the first accepted format: https://github.com/phoenixframework/phoenix/blob/05f288f978e1f200a187192a7e66d7414063bc81/lib/phoenix/controller.ex#L1140-L1144

This is what Lucky should do. I'm working on a method that you'd use like this:

class Api::MyAction < ApiAction
  include Lucky::VerifyContentType

  def accepted_content_types
    {"application/json", "multipart/form-data", "application/x-www-form-urlencoded"}
  end

  # For BrowserAction
  # "text/html", "multipart/form-data", "application/x-www-form-urlencoded"
end

So if header is */* it will treat it as application/json

This can be done now with a method like this in ApiAction

class ApiAction
  before verify_content_type

  def accepted_content_types
    {"application/json", "multipart/form-data", "application/x-www-form-urlencoded"}
  end

  def verify_content_type
    content_type = request.headers["Content-Type"]?

    if content_type.nil? || content_type == "*/*"
      content_type = accepted_content_types.first
      context.request.headers["Content-Type"] = content_type
    end

    if accepted_content_types.includes?(content_type)
      continue
    else
      puts "#{content_type} is not accepted. Here are the accepted formats: #{accepted_content_types}"
      head :unsupported_media_type
    end
  end
end

Some resources I found

  • https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Type
  • https://johnnycode.com/2013/10/30/the-differences-between-html-form-content-types-enctype/
  • https://hexdocs.pm/phoenix/Phoenix.Controller.html#accepts/2
  • https://dev.to/sidthesloth92/understanding-html-form-encoding-url-encoded-and-multipart-forms-3lpa

paulcsmith avatar Mar 12 '19 21:03 paulcsmith

ah nice find! Yeah, that makes more sense. I think that will be a good update.

jwoertink avatar Mar 12 '19 21:03 jwoertink

Updated with a snippet I just tried in an app and seems to work just fine

paulcsmith avatar Mar 12 '19 21:03 paulcsmith

Also I was wrong about Curl. It uses */* when you don't post data. When you do it uses form encoded so you have to set the content_type if you are passing JSON. It looks like there is no way around that with CURL

paulcsmith avatar Mar 12 '19 21:03 paulcsmith

Or use https://httpie.org which is sooo much nicer than curl (IMO)

paulcsmith avatar Mar 12 '19 22:03 paulcsmith