Skip to content

Loading…

Update to use latest cowboy API #9

Merged
merged 1 commit into from

2 participants

@yrashk

Conversation starter — the patch passes[1] the tests and generally seems to work, but it probably needs improvements.

[1] Until ninenines/cowboy#247 is merged in, it will not pass one test

@josevalim josevalim commented on the diff
lib/dynamo/http.ex
@@ -38,7 +38,7 @@ defmodule Dynamo.HTTP do
## Examples
- conn.method #=> :GET
+ conn.method #=> "GET"

We need to change our routes generator to pattern match on binaries too. A test should fail but it doesn't, so we need to write one first.

@yrashk
yrashk added a note

I kind of hacked around that in Dynamo.Router.Base.service(conn), by converting binaries to atoms. But whatever, I am okay with either approach.

I think the reason extend converted those atoms to binaries was to avoid atom injection attacks. so we probably shouldn't simply convert to atom too.

@yrashk
yrashk added a note

Fair enough

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@josevalim

Btw, do you know if cowboy/ranch is normalizing the headers? For example, if we request "content-length", it automatically searches for "Content-Length"?

@yrashk

I think it downcases all headers when it parses them

@josevalim josevalim merged commit f1d41f6 into dynamo:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 26, 2012
  1. @yrashk

    Update to use latest cowboy API

    yrashk committed
View
6 lib/dynamo/cowboy.ex
@@ -17,6 +17,7 @@ defmodule Dynamo.Cowboy do
"""
def run(app, options // []) do
+ :application.start(:ranch)
:application.start(:cowboy)
port = options[:port] || 4000
@@ -30,10 +31,7 @@ defmodule Dynamo.Cowboy do
IO.puts "Running #{inspect app} at localhost:#{port} with Cowboy on #{Dynamo.env}"
end
- :cowboy.start_listener(app, acceptors,
- :cowboy_tcp_transport, [port: port],
- :cowboy_http_protocol, [dispatch: dispatch]
- )
+ :cowboy.start_http(app, acceptors, [port: port], [dispatch: dispatch])
end
def shutdown(app) do
View
12 lib/dynamo/cowboy/body_parser.ex
@@ -1,10 +1,10 @@
defmodule Dynamo.Cowboy.BodyParser do
- require :cowboy_http_req, as: R
+ require :cowboy_req, as: R
@moduledoc false
def parse(dict, req) do
- { type, req } = R.parse_header(:"Content-Type", req)
+ { :ok, type, req } = R.parse_header("content-type", req)
parse_body(type, dict, req)
end
@@ -27,11 +27,11 @@ defmodule Dynamo.Cowboy.BodyParser do
{ acc, req }
end
- defp parse_multipart({ { :headers, headers }, req }, nil, nil, acc) do
+ defp parse_multipart({ :headers, headers, req }, nil, nil, acc) do
parse_multipart(R.multipart_data(req), headers, "", acc)
end
- defp parse_multipart({ { :body, tail }, req }, headers, body, acc) do
+ defp parse_multipart({ :body, tail, req }, headers, body, acc) do
parse_multipart(R.multipart_data(req), headers, body <> tail, acc)
end
@@ -41,7 +41,7 @@ defmodule Dynamo.Cowboy.BodyParser do
end
defp multipart_entry(headers, body, acc) do
- case List.keyfind(headers, "Content-Disposition", 0) do
+ case List.keyfind(headers, "content-disposition", 0) do
{ _, value } ->
[_|parts] = String.split(value, ";", global: true)
parts = lc part inlist parts, do: to_multipart_kv(part)
@@ -51,7 +51,7 @@ defmodule Dynamo.Cowboy.BodyParser do
entry =
case List.keyfind(parts, "filename", 0) do
{ "filename", filename } ->
- { _, type } = List.keyfind(headers, :"Content-Type", 0) || { :"Content-Type", nil }
+ { _, type } = List.keyfind(headers, "content-type", 0) || { "content-type", nil }
{ name, Dynamo.HTTP.File.new(name: name, filename: filename, content_type: type, body: body) }
_ ->
{ name, body }
View
2 lib/dynamo/cowboy/handler.ex
@@ -4,7 +4,7 @@ defmodule Dynamo.Cowboy.Handler do
to respond to http and websockets requests.
"""
- require :cowboy_http_req, as: R
+ require :cowboy_req, as: R
@behaviour :cowboy_http_handler
def init({ :tcp, :http }, req, app) do
View
24 lib/dynamo/cowboy/http.ex
@@ -7,7 +7,7 @@ defmodule Dynamo.Cowboy.HTTP do
"""
@behaviour Dynamo.HTTP
- require :cowboy_http_req, as: R
+ require :cowboy_req, as: R
Record.defmacros __ENV__, :connection,
[ :req, :path_info_segments, :script_name_segments, :req_headers,
@@ -23,9 +23,12 @@ defmodule Dynamo.Cowboy.HTTP do
@doc false
def new(req) do
- { segments, req } = R.path(req)
{ verb, req } = R.method(req)
+ { binary, _ } = R.path req
+ { segments, _, _} = :cowboy_dispatcher.split_path(binary,
+ fn(bin) -> :cowboy_http.urldecode(bin, :crash) end)
+
connection(
req: req,
original_method: verb,
@@ -51,17 +54,19 @@ defmodule Dynamo.Cowboy.HTTP do
## Request API
def query_string(connection(req: req)) do
- { query_string, _ } = R.raw_qs req
+ { query_string, _ } = R.qs req
query_string
end
def path_segments(connection(req: req)) do
- { segments, _ } = R.path req
+ { binary, _ } = R.path req
+ { segments, _, _} = :cowboy_dispatcher.split_path(binary,
+ fn(bin) -> :cowboy_http.urldecode(bin, :crash) end)
segments
end
def path(connection(req: req)) do
- { binary, _ } = R.raw_path req
+ { binary, _ } = R.path req
binary
end
@@ -73,7 +78,7 @@ defmodule Dynamo.Cowboy.HTTP do
## Response API
def send(_status, body,
- connection(original_method: :HEAD)) when body != "" do
+ connection(original_method: "HEAD")) when body != "" do
raise Dynamo.HTTP.InvalidSendOnHeadError
end
@@ -92,7 +97,7 @@ defmodule Dynamo.Cowboy.HTTP do
def sendfile(path, connection(req: req) = conn) do
File.Stat[type: :regular, size: size] = File.stat!(path)
- { :ok, :cowboy_tcp_transport, socket } = :cowboy_http_req.transport(req)
+ { :ok, :ranch_tcp, socket } = R.transport(req)
send(200, { size, fn -> :file.sendfile(path, socket) end }, conn)
end
@@ -106,7 +111,7 @@ defmodule Dynamo.Cowboy.HTTP do
## Misc
def fetch(:params, connection(req: req, params: nil) = conn) do
- { query_string, req } = R.raw_qs req
+ { query_string, req } = R.qs req
params = Dynamo.HTTP.QueryParser.parse(query_string)
{ params, req } = Dynamo.Cowboy.BodyParser.parse(params, req)
connection(conn, req: req, params: params)
@@ -131,7 +136,6 @@ defmodule Dynamo.Cowboy.HTTP do
defp write_cookie({ key, value, opts }, req) do
opts = Keyword.update(opts, :http_only, true, fn(x) -> x end)
- { :ok, req } = R.set_resp_cookie(key, value, opts, req)
- req
+ R.set_resp_cookie(key, value, opts, req)
end
end
View
4 lib/dynamo/http.ex
@@ -38,7 +38,7 @@ defmodule Dynamo.HTTP do
## Examples
- conn.method #=> :GET
+ conn.method #=> "GET"

We need to change our routes generator to pattern match on binaries too. A test should fail but it doesn't, so we need to write one first.

@yrashk
yrashk added a note

I kind of hacked around that in Dynamo.Router.Base.service(conn), by converting binaries to atoms. But whatever, I am okay with either approach.

I think the reason extend converted those atoms to binaries was to avoid atom injection attacks. so we probably shouldn't simply convert to atom too.

@yrashk
yrashk added a note

Fair enough

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
"""
defcallback method(conn)
@@ -51,7 +51,7 @@ defmodule Dynamo.HTTP do
## Examples
- conn.original_method #=> :GET
+ conn.original_method #=> "GET"
"""
defcallback original_method(conn)
View
1 lib/dynamo/router/base.ex
@@ -24,6 +24,7 @@ defmodule Dynamo.Router.Base do
@doc false
def service(conn) do
+ if is_binary(conn.method), do: conn = conn.method(binary_to_atom(conn.method))
dispatch(conn.method, conn.path_info_segments, conn)
end
View
3 lib/mix/tasks/dynamo.ex
@@ -124,7 +124,8 @@ defmodule Mix.Tasks.Dynamo do
end
defp deps do
- [ { :cowboy, "0.6.1", github: "josevalim/cowboy" },
+ [ { :cowboy, %r(.*), github: "extend/cowboy" },
+ { :ranch, %r(.*), github: "extend/ranch" },
{ :dynamo, "<%= @version %>", <%= @dynamo %> } ]
end
end
View
3 mix.exs
@@ -11,7 +11,8 @@ defmodule Dynamo.Mixfile do
def deps do
[ { :mimetypes, github: "spawngrid/mimetypes" },
- { :cowboy, github: "josevalim/cowboy" } ]
+ { :ranch, github: "extend/ranch" },
+ { :cowboy, github: "extend/cowboy" } ]
end
def test_deps do
View
7 mix.lock
@@ -1,4 +1,5 @@
-[ "cowboy": "9ec5626171e0fb4b1e4bbb7a98f501866d004794",
+[ "cowboy": "981ea359ba23564cb9bc3056a56dfcd1a98aa362",
"edown": "35f8296a25d14c5c1e3209399a2c1856700d9d5b",
- "hackney": "041fdd3af5f2b9b5c2dbf687871def7e9f88cf0c",
- "mimetypes": "e9dfab5aec98963589ecf13bdfcf0490667a730d" ]
+ "hackney": "77977858bf833e80abf16a276175320305108eb9",
+ "mimetypes": "e9dfab5aec98963589ecf13bdfcf0490667a730d",
+ "ranch": "cd099983b1b807b87fa050d1e4ff0a13aba25b49" ]
View
17 test/dynamo/cowboy/http_test.exs
@@ -31,12 +31,12 @@ defmodule Dynamo.Cowboy.HTTPTest do
end
def method(conn) do
- assert conn.method == :GET
- assert conn.original_method == :GET
+ assert conn.method == "GET"
+ assert conn.original_method == "GET"
- conn = conn.method(:POST)
- assert conn.method == :POST
- assert conn.original_method == :GET
+ conn = conn.method("POST")
+ assert conn.method == "POST"
+ assert conn.original_method == "GET"
conn
end
@@ -181,6 +181,7 @@ defmodule Dynamo.Cowboy.HTTPTest do
headers = List.keydelete(headers, "Set-Cookie", 0)
assert List.keyfind(headers, "Set-Cookie", 0) == { "Set-Cookie","bar=baz; Version=1" }
+ # FIXME: Will fail until https://github.com/extend/cowboy/pull/247 is merged in
end
test :req_resp_cookies do
@@ -217,8 +218,8 @@ defmodule Dynamo.Cowboy.HTTPTest do
def req_headers(conn) do
conn = conn.fetch(:headers)
- assert conn.req_headers["Host"] == "127.0.0.1:8011"
- assert conn.req_headers["X-Special"] == "foo"
+ assert conn.req_headers["host"] == "127.0.0.1:8011"
+ assert conn.req_headers["x-special"] == "foo"
conn
end
@@ -291,7 +292,7 @@ defmodule Dynamo.Cowboy.HTTPTest do
test :sendfile do
{ 200, headers, "HELLO" } = request :get, "/sendfile"
- assert List.keyfind(headers, "Content-Length", 0) == { "Content-Length", "5" }
+ assert List.keyfind(headers, "content-length", 0) == { "content-length", "5" }
assert { 500, _, _ } = request :get, "/invalid_sendfile"
end
View
1 test/test_helper.exs
@@ -3,6 +3,7 @@ Mix.env(:dev)
Mix.shell(Mix.Shell.Process)
System.put_env("MIX_ENV", "dev")
+:application.start(:ranch)
Dynamo.start(:dev, __FILE__)
ExUnit.start
Something went wrong with that request. Please try again.