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

Fixed the STD Lib monkey patch #1324

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

Conversation

crimson-knight
Copy link
Member

While trying to create a client to send POST requests with type multipart/form-data, my requests were being caught and messed up somehow.

This is infuriating to track down because the monkey patch was not at all obvious so I went ahead and removed this.

Overall the behavior is kind of weird. Changing form submissions from POST to other request types feels a bit dangerous.

There are a few tests that are failing for io reasons now. I'm running this branch in prod on my own project to see what kind of side effects it might have (yolo!)

Anyway, this was a huge blocker. Anyone using or trying to create an HTTP client of any kind would be affected.

@crimson-knight
Copy link
Member Author

The monkey patch in the STD lib is to handle HTTP Tunneling from HTML forms that are using the regular form submission behavior but are being used for PATCH requests (ie the 'edit' view).

But because HTTP::Request from the STD lib is used for both incoming requests and outgoing requests, anyone who uses an HTTP::Client to try and send certain types of requests would encounter interference in how the requests are formed (which is how I found this).

Standard HTML forms only submit with GET and POST. So this means DELETE and PATCH requests will need some form of Javascript to be properly formed and sent.

I'm still working through a way to solve this. Obviously there are existing JS libraries out there and writing your own fetch is easy, which is how I'm working around the removal of this monkey patch for now.

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.

1 participant