Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Make API case-insensitive #1648

Closed
patcon opened this issue Nov 7, 2013 · 11 comments
Closed

Make API case-insensitive #1648

patcon opened this issue Nov 7, 2013 · 11 comments

Comments

@patcon
Copy link
Contributor

patcon commented Nov 7, 2013

Forked from #1563 (comment)

Mis-cased usernames in the API are 302 redirected to the correct url when accessed via browser, but some request libs (particularly that used by hubot) don't support redirects.

Thinking might be best to handle this unexpected behaviour server-side. For the record, this is what github does.

cc: @balupton

@patcon
Copy link
Contributor Author

patcon commented Nov 8, 2013

Have never really used python before, but this would seem to be where the username in the slug were theoretically converted to lowercase to match the username_lower db column:
https://github.com/gittip/www.gittip.com/blob/master/gittip/utils/__init__.py#L314-L318

Could it be that we're not using slug.lower().first() when passing in params, as we were did previously the codebase? Or perhaps the empty space in the dict is confusing things? Just guessing here. Not really set up for local py dev, although I should get on that soon :)

DB docs ref:

EDIT: Ok, seems my second suggestion was baloney.

@patcon
Copy link
Contributor Author

patcon commented Nov 8, 2013

Ah, I'm an idiot. It's because it redirects before it checks against lower...

https://github.com/gittip/www.gittip.com/blob/master/gittip/utils/__init__.py#L310-L312

Maybe the redirect isn't needed?

@chadwhitacre
Copy link
Contributor

The canonicalize function is where the 302 is raised:

https://github.com/gittip/www.gittip.com/blob/master/gittip/utils/__init__.py#L284-293

@chadwhitacre
Copy link
Contributor

A robust HTTP client library should follow redirects, so technoweenie/node-scoped-http-client#7 should be fixed in any case, and that would take care of the presenting issue here.

Whether or not to canonicalize URIs for API requests is complicated by the fact that we run both /foo/ and /foo/public.json through the same code path. We would need to differentiate the two one way or another.

Why not just fix technoweenie/node-scoped-http-client#7, though?

@patcon
Copy link
Contributor Author

patcon commented Nov 8, 2013

Totally. We can do that. But then every client lib implementation in every language will have to reimplement. I've never run into a 302 in regular API use, except in one really crufty API from some obscure service, and I don't think they're even in the same league as us. My experience is that 302's are usually used to get people onto https (in the context of API's :)

Random selection of API's for popular services (and using non-numeric IDs):
https://api.github.com/users/PatCon
http://api.dribbble.com/players/MyPlanet
https://coderwall.com/PatCon.json
http://api.reddit.com/user/PatCon/about.json

Anyhow, case-sensitivity is fine for us for now, but I'll scratch the itch when the time comes :) Thanks for the pointer on where to look!

@patcon
Copy link
Contributor Author

patcon commented Nov 11, 2013

Copying convo from other thread (#1563):

@clone1018:

Hrm, I think we should still redirect the user to the proper URL. Serving actual results on a url that's not 1:1 with the database is :(

@balupton:

@clone1018 definitely, the proper URL should always be redirected to. I think the argument here though, is that by making the API case insensitive, like the way GitHub does, then both https://api.github.com/users/PatCon and https://api.github.com/users/patcon are both the proper URLs. In exactly the same way case-insensitive file systems work, as well as the GitHub API. For instance, if the database is case insensitive, like most file systems, then we wouldn't even be having this issue.

@patcon:

Serving actual results on a url that's not 1:1 with the database is :(

@clone1018 I'm genuinely curious, why is that? It would strike me as a pattern that only contributes rigidity. I haven't noticed that pattern in any respected services that I use. Sorry, I'm not trying to be confrontational, but I am curious :)

A singular example: https://github.com/PatCon

@clone1018:

I'm not sure I have an actual argument for or against, but it feels wrong. The only potential reason I can think of would be if a system used "username" as a unique, case-sensitive ID but that would be the failure of that system.

@patcon:

Here was a relevant convo in rails. People raised the same thought about urls being intentionally case-sensitive, but supposing there are cases where it isn't wanted, the solution there was to just tackle it in the router middleware and not bother redirecting:

http://stackoverflow.com/questions/2291907/rails-routes-how-to-make-them-case-insensitive

@chadwhitacre
Copy link
Contributor

+1 from @espadrine in working with gh-badges (node badge server we're using for Shields). IRC

@chadwhitacre
Copy link
Contributor

This SO answer points to https://github.com/mikeal/request :

Request is designed to be the simplest way possible to make http calls. It supports HTTPS and follows redirects by default.

@chadwhitacre
Copy link
Contributor

@chadwhitacre
Copy link
Contributor

I'm closing this as won't fix. HTTP redirects are not something esoteric. A proper client library should support them.

@zbynekwinkler
Copy link
Contributor

@patcon As you say, for the record, this is github's disclaimer: http://developer.github.com/v3/#http-redirects

API v3 uses HTTP redirection where appropriate. Clients should assume that any request may result in a redirection. Receiving an HTTP redirection is not an error and clients should follow that redirect. Redirect responses will have a Location header field which contains the URI of the resource to which the client should repeat the requests.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants