Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add new rule for flask-route-decorator #3244

Closed
wants to merge 7 commits into from

Conversation

khanhldt
Copy link

Wahoo! New published rules with khanhldt.pnbj from @khanhldt.

See semgrep.dev/s/2ZJ3Y for more details.

Thanks for your contribution! ❤️

@CLAassistant
Copy link

CLAassistant commented Dec 13, 2023

CLA assistant check
All committers have signed the CLA.

@CLAassistant

This comment was marked as outdated.

@khanhldt
Copy link
Author

I signed the CLA but that message doesn't get resolved for some reason.

@0xDC0DE
Copy link
Contributor

0xDC0DE commented Jan 9, 2024

It looks like Semgrep does not take the order of decorators into account currently: https://semgrep.dev/playground/s/pKLAZ

While your regex is a nice workaround, I am reporting this as a bug/feature request so that we can avoid complex regexes for this.

There are other route decorators in Flask besides the route decorator, e.g. delete, patch, put, post, get. These are basically aliases for the route decorator that use a different default value for the optional methods argument. I think the rule should probably include those as well.

I've annotated a few more things you should change for the CI checks to pass.

python/flask/correctness/flask-route-decorator.py Outdated Show resolved Hide resolved
def f():
pass

# ok: Correct order
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

def f():
pass

# ok: "func" not "route"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

def f():
pass

# ok: Correct order with app
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

def f():
pass

# ok: 2 routes are independent
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

@0xDC0DE 0xDC0DE closed this Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants