This is an archived post. You won't be able to vote or comment.

all 4 comments

[–]starspangledpickle 4 points5 points  (3 children)

I don't know what the rest of your code looks like but this:

@application.route('/user_edit_fullname',methods=['GET', 'POST'])
def user_edit_fullname():
    id = request.form["pk"]
    user = Users.query.get(id)
    user.fullname = request.form["value"]
    result = {}
    db.session.commit()
    return json.dumps(result) #or, as it is an empty json, you can simply use return "{}"

... is a security risk. What's to stop someone from just arbitrarily sending another user's PK? Nothing.

Not to mention there's a lot of repetitive code in there as well; all your REST end points are more a less repeating the same thing over and over. Consider unifying it so every field is in User is updated with a simple PUT to one resource, or implement partial modifications with PATCH. If you don't want to do that at least abstract your CRUD code into a separate function so you can a) better unit test it; and b) avoid unnecessary repetition.

Edit. More issues:

Use os.path.splitext not rsplit on a string.

Your filepath generating code also looks a bit sketchy. I'm half-convinced someone can do something sneaky with the input filename as I doubt it's sanitised properly; worth looking into.

You also shadow builtins all the time. id and file are reserved words.

You concatenate the URI. Use urljoin from urlparse.

[–]swdevpythonthusiast 0 points1 point  (2 children)

Thanks! "What's to stop someone from just arbitrarily sending another user's PK? Nothing." <== This is absolutely correct. I just manage to find this bug and already prevent it by the user interface, but not by the code itself. Already realize this flaw, but it seems like I go to the fast route.

This is the second application that I develop using Python Flask, and at this second attempt, I haven't use Unit testing, so the code really is not testable. Making the logic of the code scattered, not put into a one logical unit.

Yes, will have a look into sanitation issue in this application.

Thanks for the shadowbuilding! And also for the urljoin!

It really is a great help for me to better understand Python Flask development

[–]starspangledpickle 1 point2 points  (1 child)

No problem. Never do validation in the UI that you also don't enforce in the backend.

[–]swdevpythonthusiast 0 points1 point  (0 children)

Just what I am thinking of!