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

Bug report for trillium-method-override: Not converting from method field in request body #207

Closed
jalcine opened this issue Apr 29, 2022 · 4 comments

Comments

@jalcine
Copy link
Contributor

jalcine commented Apr 29, 2022

Describe the bug
While using htmx.org, namely hx-post with <input type="hidden" name="_method" value="put"> in a form, I noticed that the request wasn't being translated into an PUT request on the server side.

To Reproduce
Reproduction of this could work by copying one of the tests at trillium-rs/trillium#main/method-override/tests/tests.rs:L16-L29 but instead of checking the query string , putting the test body as the request body, like:

assert_ok!(post("/").with_request_body("_method=put").on(&amp;app), "it was a PUT");

This way, it'll show that the use of only query string extraction wouldn't be enough. However, it looks like reading the body of the request will mutate the connection itself at https://docs.rs/trillium/latest/src/trillium/conn.rs.html#257-274. Perhaps an updated test will show otherwise.

Expected behavior
Proper conversion of a POST request with a hint to the expected HTTP Verb.

Additional context
I figure that the code of method-override is a bit older than the tests and perhaps led to this not being covered.

(Originally published at: https://jacky.wtf/2022/4/W1/W1YTNu6QaaLQ33o0zcbWFhRr)

@jbr
Copy link
Contributor

jbr commented May 14, 2022

Hey, sorry for the delay on this!

To make sure I understand your suggestion, the goal is to parse the body and check for a given attribute that modifies the http method along the lines of trillium-method-override, which currently only uses a url param? Let me know if I misunderstood the intent

I don't think there's currently a good way to provide a library that does this that can support arbitrary body encodings (json, xml, x-www-urlecoded, etc). I think this is the sort of thing that would be best to handle in an application, by parsing the body from whatever format and putting that into conn state, as well as modifying the method.

@jalcine
Copy link
Contributor Author

jalcine commented May 14, 2022

Hm, that makes sense! I think I can add this on my side, but I'd need to figure out how to change the method while a request is being processed. It looks simple enough to do reading https://docs.trillium.rs/src/trillium_method_override/lib.rs.html#94-108

(Originally published at: https://jacky.wtf/2022/5/r-Er)

@jbr
Copy link
Contributor

jbr commented May 14, 2022

Yep, it's really just conn.inner_mut().set_method(method); (using trillium_http::Conn::set_method) — And Method has a try_from(&str)

@jbr
Copy link
Contributor

jbr commented May 18, 2022

I'm going to tentatively close this because it seems like this needs to be application-specific, since there are any number of body encodings that the trillium-method-override library doesn't/shouldn't know about

@jbr jbr closed this as completed May 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants