Skip to content

Conversation

@jgresty
Copy link
Contributor

@jgresty jgresty commented Oct 17, 2024

This patch adds support for any arbitrary http method. I discovered this while trying to implement a server that would respond to the QUERY (draft) method, which caused a crash when I tried it. Since this functionality is supported in the upstream rust http libraries, I can't see any reason why it cannot be supported here too.

The canonical form for http method names is upper case according to
rfc7231, this matches the format used in the upstream reqwest library.
Http allows any arbitrary methods defined in rfc2774 and implemented by
convention. These are implemented by the upstream reqwest library so
there is no reason why we can't also implement it.

This removes a source of a potential crash, which is on the path of user
submitted data (a http request).
@lukewilliamboswell lukewilliamboswell self-assigned this Oct 17, 2024
@lukewilliamboswell
Copy link
Collaborator

I'm going to have a crack at using a tag union instead of a RocStr to cross the host boundary. I'll hand roll some glue for the method.

@bhansconnect
Copy link
Member

I'm going to have a crack at using a tag union instead of a RocStr to cross the host boundary. I'll hand roll some glue for the method.

If you run into issues, feel free to ping me on zulip. I would guess that any issues will come from https://roc.zulipchat.com/#narrow/channel/395097-compiler-development/topic/tags.20and.20c.20abi

Copy link
Collaborator

@lukewilliamboswell lukewilliamboswell left a comment

Choose a reason for hiding this comment

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

I attempted to avoid the RocStr, but that turned out to be more fragile than I hoped. This is good.

@lukewilliamboswell lukewilliamboswell merged commit 86b22ca into roc-lang:main Oct 18, 2024
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

Successfully merging this pull request may close these issues.

3 participants