404 if I leave a trailing slash in the URI #96

Closed
benjamin-hodgson opened this Issue Mar 3, 2014 · 9 comments

Comments

Projects
None yet
3 participants
@benjamin-hodgson

GET /streams/mystream/ returns a 404, whereas /streams/mystream returns a 200 with the correct content. I was not expecting this! 馃槈

@gregoryyoung

This comment has been minimized.

Show comment
Hide comment
@gregoryyoung

gregoryyoung Mar 3, 2014

Member

is mystream a directory or a resource?

On Mon, Mar 3, 2014 at 11:45 PM, Benjamin Hodgson
notifications@github.comwrote:

GET /streams/mystream/ returns a 404, whereas /streams/mystream returns a
200 with the correct content. I was not expecting this! [image: 馃槈]

Reply to this email directly or view it on GitHubhttps://github.com/EventStore/EventStore/issues/96
.

Le doute n'est pas une condition agr茅able, mais la certitude est absurde.

Member

gregoryyoung commented Mar 3, 2014

is mystream a directory or a resource?

On Mon, Mar 3, 2014 at 11:45 PM, Benjamin Hodgson
notifications@github.comwrote:

GET /streams/mystream/ returns a 404, whereas /streams/mystream returns a
200 with the correct content. I was not expecting this! [image: 馃槈]

Reply to this email directly or view it on GitHubhttps://github.com/EventStore/EventStore/issues/96
.

Le doute n'est pas une condition agr茅able, mais la certitude est absurde.

@benjamin-hodgson

This comment has been minimized.

Show comment
Hide comment
@benjamin-hodgson

benjamin-hodgson Mar 3, 2014

Apologies, I should have been more specific in my reproduction steps. It's a resource identifying an event stream.

Steps to reproduce

  1. POST an event to the Event Store to create a stream:
POST http://127.0.0.1:2113/streams/mystream HTTP/1.1
Host: http://127.0.0.1:2113
Content-Type: application/json

[{"eventType": "myEvent", "data": {}, "eventId": "d266c2c4-4afc-429c-a1bd-4f84946541b6"}]
  1. Try and GET your new stream with a trailing slash in the URI:
GET http://127.0.0.1:2113/streams/mystream/ HTTP/1.1
Host: http://127.0.0.1:2113
Accept: application/atom+xml

Expected result
A 200 response with the same body as when you GET the resource without the trailing slash

Actual result
A 404 response with no body. You also get a 404 if you attempt to POST to that resource.

Hope this is helpful 馃槂

Apologies, I should have been more specific in my reproduction steps. It's a resource identifying an event stream.

Steps to reproduce

  1. POST an event to the Event Store to create a stream:
POST http://127.0.0.1:2113/streams/mystream HTTP/1.1
Host: http://127.0.0.1:2113
Content-Type: application/json

[{"eventType": "myEvent", "data": {}, "eventId": "d266c2c4-4afc-429c-a1bd-4f84946541b6"}]
  1. Try and GET your new stream with a trailing slash in the URI:
GET http://127.0.0.1:2113/streams/mystream/ HTTP/1.1
Host: http://127.0.0.1:2113
Accept: application/atom+xml

Expected result
A 200 response with the same body as when you GET the resource without the trailing slash

Actual result
A 404 response with no body. You also get a 404 if you attempt to POST to that resource.

Hope this is helpful 馃槂

@gregoryyoung

This comment has been minimized.

Show comment
Hide comment
@gregoryyoung

gregoryyoung Mar 3, 2014

Member

What I meant by that is mystream/ represents a directory mystream
represents a document

We have talked about this with quite a few people and have basically come
to the point that putting a trailing / is wrong.

eg: DELETE mystream/ says to delete all the resources under mystream/ eg if
it were dogs/1 dogs/2 dogs/3 etc (where 1,2,3 are resources and dogs is a
directory of dog resources).

an atom feed is a resource itself so the trailing / doesn't make sense. For
the POST case I can make a better argument for supporting it.

We are open to reopening this discussion but its definitely by design.

On Tue, Mar 4, 2014 at 12:34 AM, Benjamin Hodgson
notifications@github.comwrote:

Apologies, I should have been more specific in my reproduction steps. It's
a resource.

Steps to reproduce

  1. POST an event to the Event Store to create a stream:

POST http://127.0.0.1:2113/streams/mystreamContent-Type: application/json
[{"eventType": "myEvent", "data": {}, "eventId": "d266c2c4-4afc-429c-a1bd-4f84946541b6"}]

  1. Try and GET your new stream with a trailing slash in the URI

GET http://127.0.0.1:2113/streams/mystream/Accept: application/atom+xml

Expected results
A 200 response with the same body as when you GET the resource without the
trailing slash

Actual result
A 404 response with no body. You also get a 404 if you attempt to POST to
that resource.

Hope this is helpful [image: 馃槂]

Reply to this email directly or view it on GitHubhttps://github.com/EventStore/EventStore/issues/96#issuecomment-36569260
.

Le doute n'est pas une condition agr茅able, mais la certitude est absurde.

Member

gregoryyoung commented Mar 3, 2014

What I meant by that is mystream/ represents a directory mystream
represents a document

We have talked about this with quite a few people and have basically come
to the point that putting a trailing / is wrong.

eg: DELETE mystream/ says to delete all the resources under mystream/ eg if
it were dogs/1 dogs/2 dogs/3 etc (where 1,2,3 are resources and dogs is a
directory of dog resources).

an atom feed is a resource itself so the trailing / doesn't make sense. For
the POST case I can make a better argument for supporting it.

We are open to reopening this discussion but its definitely by design.

On Tue, Mar 4, 2014 at 12:34 AM, Benjamin Hodgson
notifications@github.comwrote:

Apologies, I should have been more specific in my reproduction steps. It's
a resource.

Steps to reproduce

  1. POST an event to the Event Store to create a stream:

POST http://127.0.0.1:2113/streams/mystreamContent-Type: application/json
[{"eventType": "myEvent", "data": {}, "eventId": "d266c2c4-4afc-429c-a1bd-4f84946541b6"}]

  1. Try and GET your new stream with a trailing slash in the URI

GET http://127.0.0.1:2113/streams/mystream/Accept: application/atom+xml

Expected results
A 200 response with the same body as when you GET the resource without the
trailing slash

Actual result
A 404 response with no body. You also get a 404 if you attempt to POST to
that resource.

Hope this is helpful [image: 馃槂]

Reply to this email directly or view it on GitHubhttps://github.com/EventStore/EventStore/issues/96#issuecomment-36569260
.

Le doute n'est pas une condition agr茅able, mais la certitude est absurde.

@jnardiello

This comment has been minimized.

Show comment
Hide comment
@jnardiello

jnardiello Mar 3, 2014

Quite an interesting topic. I believe that ES is returning 404 because it expects something connected to a specific event in the stream while you are passing null. I agree that it is confusing but not formally wrong.

Quite an interesting topic. I believe that ES is returning 404 because it expects something connected to a specific event in the stream while you are passing null. I agree that it is confusing but not formally wrong.

@gregoryyoung

This comment has been minimized.

Show comment
Hide comment
@gregoryyoung

gregoryyoung Mar 4, 2014

Member

What if we were to HTTP 308 on the slash? This would prevent us from
polluting caches.

On Tue, Mar 4, 2014 at 12:47 AM, Jacopo Nardiello
notifications@github.comwrote:

Quite an interesting topic. I believe that ES is returning 404 because it
expects something connected to a specific event in the stream while you are
passing null. I agree that it is confusing but not formally wrong.

Reply to this email directly or view it on GitHubhttps://github.com/EventStore/EventStore/issues/96#issuecomment-36570713
.

Le doute n'est pas une condition agr茅able, mais la certitude est absurde.

Member

gregoryyoung commented Mar 4, 2014

What if we were to HTTP 308 on the slash? This would prevent us from
polluting caches.

On Tue, Mar 4, 2014 at 12:47 AM, Jacopo Nardiello
notifications@github.comwrote:

Quite an interesting topic. I believe that ES is returning 404 because it
expects something connected to a specific event in the stream while you are
passing null. I agree that it is confusing but not formally wrong.

Reply to this email directly or view it on GitHubhttps://github.com/EventStore/EventStore/issues/96#issuecomment-36570713
.

Le doute n'est pas une condition agr茅able, mais la certitude est absurde.

@gregoryyoung

This comment has been minimized.

Show comment
Hide comment
@gregoryyoung

gregoryyoung Mar 4, 2014

Member

To further discussion https://github.com/nicolaiarocci/eve/issues/118

On Tue, Mar 4, 2014 at 2:07 AM, Greg Young gregoryyoung1@gmail.com wrote:

What if we were to HTTP 308 on the slash? This would prevent us from
polluting caches.

On Tue, Mar 4, 2014 at 12:47 AM, Jacopo Nardiello <
notifications@github.com> wrote:

Quite an interesting topic. I believe that ES is returning 404 because it
expects something connected to a specific event in the stream while you are
passing null. I agree that it is confusing but not formally wrong.

Reply to this email directly or view it on GitHubhttps://github.com/EventStore/EventStore/issues/96#issuecomment-36570713
.

Le doute n'est pas une condition agr茅able, mais la certitude est absurde.

Le doute n'est pas une condition agr茅able, mais la certitude est absurde.

Member

gregoryyoung commented Mar 4, 2014

To further discussion https://github.com/nicolaiarocci/eve/issues/118

On Tue, Mar 4, 2014 at 2:07 AM, Greg Young gregoryyoung1@gmail.com wrote:

What if we were to HTTP 308 on the slash? This would prevent us from
polluting caches.

On Tue, Mar 4, 2014 at 12:47 AM, Jacopo Nardiello <
notifications@github.com> wrote:

Quite an interesting topic. I believe that ES is returning 404 because it
expects something connected to a specific event in the stream while you are
passing null. I agree that it is confusing but not formally wrong.

Reply to this email directly or view it on GitHubhttps://github.com/EventStore/EventStore/issues/96#issuecomment-36570713
.

Le doute n'est pas une condition agr茅able, mais la certitude est absurde.

Le doute n'est pas une condition agr茅able, mais la certitude est absurde.

@benjamin-hodgson

This comment has been minimized.

Show comment
Hide comment
@benjamin-hodgson

benjamin-hodgson Mar 4, 2014

OK, I understand what's going on, thanks. It was just a surprise to see it!

OK, I understand what's going on, thanks. It was just a surprise to see it!

@gregoryyoung

This comment has been minimized.

Show comment
Hide comment
@gregoryyoung

gregoryyoung Mar 6, 2014

Member

Benjamin.

I have implemented 308 permanent redirects on the uris with trailing
slashes, These are committed with tests to dev branch now. You can see
commits here:

d36291d
ba65eca
ba0705c

I wanted to add a few paragraphs about what I did and why I chose to do it
this way if for no other reason than people will be able to see this in the
future (and can serve as some documentation).

I have made all common Uris that are normally say /streams/foo do a 308
permanent redirect if you type them as /streams/foo/. Now 308 is
experimental but almost everyone supports it at this point. If people have
issues I would be happy to switch to a 301 but that would require the
caller to then query what methods were available on the redirect (or change
verb which 308 says you must use same verb).

The reason I chose to 308 as opposed to just making it work has to do with
intermediaries and cache pollution. /streams/foo and /streams/foo/ are two
different uris. If we were to support requests from both an intermediary
cache would need to cache both uris as being unique. That said I can
understand the wanting to be able to type it either way.

Hope this makes sense.

Cheers,

Greg

On Tue, Mar 4, 2014 at 11:41 AM, Benjamin Hodgson
notifications@github.comwrote:

OK, I understand what's going on. It was just a surprise to see it!

Reply to this email directly or view it on GitHubhttps://github.com/EventStore/EventStore/issues/96#issuecomment-36606959
.

Le doute n'est pas une condition agr茅able, mais la certitude est absurde.

Member

gregoryyoung commented Mar 6, 2014

Benjamin.

I have implemented 308 permanent redirects on the uris with trailing
slashes, These are committed with tests to dev branch now. You can see
commits here:

d36291d
ba65eca
ba0705c

I wanted to add a few paragraphs about what I did and why I chose to do it
this way if for no other reason than people will be able to see this in the
future (and can serve as some documentation).

I have made all common Uris that are normally say /streams/foo do a 308
permanent redirect if you type them as /streams/foo/. Now 308 is
experimental but almost everyone supports it at this point. If people have
issues I would be happy to switch to a 301 but that would require the
caller to then query what methods were available on the redirect (or change
verb which 308 says you must use same verb).

The reason I chose to 308 as opposed to just making it work has to do with
intermediaries and cache pollution. /streams/foo and /streams/foo/ are two
different uris. If we were to support requests from both an intermediary
cache would need to cache both uris as being unique. That said I can
understand the wanting to be able to type it either way.

Hope this makes sense.

Cheers,

Greg

On Tue, Mar 4, 2014 at 11:41 AM, Benjamin Hodgson
notifications@github.comwrote:

OK, I understand what's going on. It was just a surprise to see it!

Reply to this email directly or view it on GitHubhttps://github.com/EventStore/EventStore/issues/96#issuecomment-36606959
.

Le doute n'est pas une condition agr茅able, mais la certitude est absurde.

@gregoryyoung

This comment has been minimized.

Show comment
Hide comment
@gregoryyoung

gregoryyoung Mar 17, 2014

Member

resolved.

Member

gregoryyoung commented Mar 17, 2014

resolved.

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