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.

Sign up for GitHub

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

Jump to bottom

feat!: invoke actions on POST only #149

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat!: invoke actions on POST only #149

wants to merge 1 commit into from

Conversation

wlatanowicz
Copy link

@wlatanowicz wlatanowicz commented Nov 21, 2022 •
edited by crccheck
Loading

closes #16

@pfouque
Copy link

pfouque commented Nov 28, 2022

I like the idea of restricting to POST only, in order to avoid XSS.
Is there any reason not to use <input type="hidden" name="{{ k }}" value="{{ v }}" /> (and a classic submit button)?

@wlatanowicz
Copy link
Author

Is there any reason not to use <input type="hidden" name="{{ k }}" value="{{ v }}" /> (and a classic submit button)?

I decided to use <a> element as an action button, to avoid potentially complex changes to css styles.

@crccheck
Copy link
Owner

crccheck commented Jan 3, 2023

sorry for dragging my feet on this. I keep thinking I'll have time the next holiday to go over this, but real life keeps getting in the way. I like what I see so far; switching over to POST keeps things simple.

@crccheck crccheck changed the title Invoke actions on POST only feat!: invoke actions on POST only Jan 3, 2023
@SupraSummus
Copy link
Contributor

@crccheck have you went over this PR? I'd really appreciate merging this in. Cheers!

@crccheck
Copy link
Owner

I think we need to phase in this feature more gradually. Like:

  1. Add a toggle to use POST instead of GET (default to GET). Perhaps make the options: GET, POST, and ONLY_POST
  2. Make a "breaking" change to the default from GET to POST

I've been meaning to do it for awhile but real life has gotten pretty busy

@SupraSummus SupraSummus mentioned this pull request Mar 5, 2024
Merged
crccheck pushed a commit that referenced this pull request Sep 9, 2024
@SupraSummus
wip to select GET or POST for actions (#168)
1274ae7 
Another try at enforcing POST actions. This change is more gradual than
#149 - when library user doesn't change default options the behavior is
exactly the same as before the change, that is:

1. Action buttons send GET requests
2. Action handlers accept GET and POST requests

However, user can change this behavior using `methods` and `button_type`
kwargs. For example `@action(methods=['POST'], button_type='form')`
results in

1. Action button sends POST requests
2. Action handler accepts only POST request

Unfortunately I have this tested only within my project. Also the docs
are missing.

And one more thing - I think it is better to use `<input type="submit">`
instead of js to submit the form. This js is need to make the buttons
look the same in both versions. With proper CSS (that is beyond my
ability to write ;) ) js is avoidable and we could be using pretty
semantic html submit button. I took the form button template from #149.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

POST actions
4 participants
@wlatanowicz @pfouque @crccheck @SupraSummus