Skip to content

Loading…

cowboy_rest chokes on missing Accept: header #310

Closed
dvv opened this Issue · 13 comments

2 participants

@dvv
dvv commented

https://github.com/extend/cowboy/blob/master/src/cowboy_rest.erl#L213 -- the pattern matching deadly fails here unless Accept: is provided, causing tough 500 (Server Error) instead of imho better softer not_acceptable() or even implying a certain convenient (and configurable) default Accept:.
i do believe missing data is not that server error.
please, explain how to cope? tia

@essen
Nine Nines member

Good point. I'm sick at the moment so I'm not doing any development, will fix when I can.

@dvv
dvv commented

My, I'll quench filing issues.
Please, get well!

@essen
Nine Nines member

Yeah there's definitely something wrong there. I'll take note of it for the new and improved diagram and code.

@essen
Nine Nines member

Wait, it shouldn't fail. Missing header should return {ok, undefined, Req}.

@essen
Nine Nines member

You can test this on the rest_hello_world example.

@essen
Nine Nines member

I'm closing it, please reopen with an example code if you run into the problem again. Thanks!

@essen essen closed this
@dvv

Am not having an erlang machine within my grasp, but purely speculatively, if you meant https://github.com/extend/cowboy/blob/master/examples/rest_hello_world/start.sh#L3 doesn't send Accept: header, it does. The rest of evals there do that as well evidently.
Will report asap.

@essen
Nine Nines member

You are of course right.

However:

% telnet localhost 8080        
Trying ::1...
Connection failed: Connection refused
Trying 127.0.0.1...
Connected to localhost.
Escape character is '^]'.
GET / HTTP/1.1
Host: localhost:8080

HTTP/1.1 200 OK
connection: keep-alive
server: Cowboy
date: Thu, 29 Nov 2012 10:50:32 GMT
content-length: 136
content-type: text/html
vary: accept

<html>
<head>
    <meta charset="utf-8">
    <title>REST Hello World!</title>
</head>
<body>
    <p>REST Hello World as HTML!</p>
</body>
</html>Connection closed by foreign host.
@dvv

Confirmed.
Now it chokes with 500 on eg Accept: 1. Bug or feature?

@essen
Nine Nines member

That's not a valid Accept header. :)

Feature!

@essen
Nine Nines member

Though it should probably be 400...

@dvv

^^^

@essen
Nine Nines member

Fixed in 5c315ab. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.