japacible / commission-me

Platform for commissioners and buyers to connect and finalize sales.
http://commissionme.herokuapp.com/
4 stars 3 forks source link

Commission Request Revisions does not handle illegal prices #228

Open japacible opened 10 years ago

japacible commented 10 years ago

/commissions/review/{number}

Assume illegal numbers in prices include:

Steps:

This is on the wip-commissions branch

japacible commented 10 years ago

@Bejoty @jbrodhacker can you guys add in this check? I don't want to just go poke around in case I break what you guys are planning on doing.

jbrodhacker commented 10 years ago

@Bejoty is this check possible in javascript? If so, it'd be better to implement there since the server doesn't get ahold of the number until post, which would mean a loss of data (the revision text).

Bejoty commented 10 years ago

Definitely possible, but generally, validation is ALWAYS done on the server end, because until it reaches the server, the information can be manipulated. I can build a JS check as well, which would mitigate the info loss if the user is just being an idiot.

Bejoty commented 10 years ago

A good analogy I read somewhere is that client-side validation is like a big "DO NOT ENTER" sign on the door, but server-side validation is actually locking the door.

japacible commented 10 years ago

Sorry to jump in - based on time constraints, just hack in a js check if you can. :)

acouch00 commented 10 years ago

This might be useful for your hack.

jbrodhacker commented 10 years ago

The server does not have to validate what the front end can prevent; like disabling the user from typing in characters that are not relevant, or checking for empty fields. This is one of those things that the front end would be better at checking than the server for the reason I listed above.

Bejoty commented 10 years ago

Well similarly to all the other pages with user input, server validation is essential, and client validation is a preferred addition. However, hacking it together in JS as a bandage I wouldn't think would be worth the limited time.

Binding event handlers to each form upon blur (losing focus), checking for even basic validation, injecting a useful error message, style updates, and a submission flag.... It just seems like it will take quite a while for the time we don't have.

Bejoty commented 10 years ago

The issue is that the front end can't prevent it, only mitigate the risk. If someone wants to inject malicious input, the front end is very easy to get past.

Bejoty commented 10 years ago

Oh, and I have the current list of things needing validation here: #234

quanc commented 10 years ago

It looks like this issue is in https://github.com/japacible/commission-me/issues/234. Closing this.

quanc commented 10 years ago

This problem is still there, except I noticed that non-numeric values can be entered as well (for example "abc").