Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

404 if username contains / #116

Closed
raulsebastianmihaila opened this issue May 24, 2017 · 8 comments
Closed

404 if username contains / #116

raulsebastianmihaila opened this issue May 24, 2017 · 8 comments

Comments

@raulsebastianmihaila
Copy link

When I make a call to https://conduit.productionready.io/api/profiles/:username, it returns 404 if the username contains a forward slash (even though I'm encoding it before doing the call). Other characters like % (if encoded properly) are allowed.

@Cameron-C-Chapman
Copy link
Member

From what I see if you have an un-encoded forward slash you get a 404 with an html body indicating an error matching the route but with an encoded forward slash you get a 404 with a json body indicating the user was not found.

If you added a user with an encoded forward slash and get a 404 when looking for them with the encoded forward slash then it's probably an issue storing the data in that format.

@BRWR do you know how the rails implementation would handle that? If I have time tomorrow I can take a closer look but that feels like something most api's/databases could have issues with.

@jamesbrewerdev
Copy link

@raulsebastianmihaila Nice catch! Thanks for the issue. This is definitely a mistake on our end.

Yeah. I know what's going on here.

For a couple of reasons, we're running a modified version of the Rails backend for the demo. Unfortunately, that means the two can get out of sync. That's what happened here.

If you look at app/models/user.rb#15 from the Rails backend, you'll see that there is a validation that blocks usernames with non-alphanumeric characters. This validation isn't present in the demo backend.

@apai4 Mind fixing this up? I'm at work and can't get to it today.

NOTE: This will also require a data migration to handle usernames that were registered with non-alphanumeric characters. Not sure what the best way to communicate this is or whether we need to communicated it at all.

@EricSimons
Copy link
Member

@BRWR thanks for the detail here — @apai4 is currently in Chicago visiting fam, but I'm sure he'll be able to get in a fix in soon :)

@devadvance
Copy link
Contributor

This leads to a larger question though: should things like full support for things like non-alphanumeric characters and internationalization be included in the core spec? In #90 internationalization is raised as a possible extra credit item, but if this is meant to reflect real apps, then it should be part of the base spec.

@jamesbrewerdev
Copy link

I18n has already been discussed in #90. Best to leave that there.

Regarding alphanumeric-only usernames: This should be a constraint for all RealWorld projects, specifically for the reason this issue was opened.

@EricSimons
Copy link
Member

EricSimons commented May 31, 2017

@devadvance @BRWR Yup! Validations for all user, article, etc fields are actually a part of the new spec that @SandeeshS created — he's currently working on the formatting but it will be updated on the repo soon :)

Also, please see #115 re: validations

@devadvance
Copy link
Contributor

Glad to see validations being added/formalized, but for clarity: the alphanumeric limitation for usernames only, correct? Making sure this doesn't impact email/password/etc.!

@sandeesh
Copy link
Member

sandeesh commented Jun 1, 2017

@devadvance we're gonna add validation for all form fields. This is what we have for registration

username 	- required, max length:50, alphanumeric characters only, unique in database
email 		- required, max length:255, valid email format, unique in database
password 	- required, min length:6

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

7 participants