Take advantage of improvement in the http crate for cloning#7559
Take advantage of improvement in the http crate for cloning#7559
Conversation
In an older version of the crate, `http::Extensions` were not `Clone`. This had the consequence that `http::Request` and `http::Response` were also not clone. To work-around this limitation, we had functions in our http_ext module which would "clone" a `Request` or a `Response`, but they'd omit `Extensions`. Generally speaking, this wasn't a problem for most use cases. However, over time we started to add functionality, such as file uploads, to the router which relied on the correct behaviour of cloning and expected Extensions to be preserved across a clone. This finally resulted in an issue for a customer who could not understand why modifying a request in a Rhai script would cause a File Upload to fail. It transpires that at version 1.0.0 of the http crate, an incompatible change was made which forces `http::Extensions` to be `Clone` which means both `http::Request` and `http::Response` are now `Clone`. I've replaced our `http::Extensions` ignoring implementation with the provided `Clone` behaviour and this means that Request and Response Extensions are now correctly cloned. I've added tests for this as well.
This comment has been minimized.
This comment has been minimized.
✅ Docs preview has no changesThe preview was not built because there were no changes. Build ID: 9fca2c2268b1f88159a19fe1 |
goto-bus-stop
left a comment
goto-bus-stop
left a comment
There was a problem hiding this comment.
I'd like to see an integration test that uses file upload + rhai...
That's a great suggestion. I've got no idea how I'd do that right now, but I'll take a look through the existing File Upload Integration Tests and see if I can figure something out. |
It was surprisingly easy: 15bc639. Well done to the author of the original File Upload Integration Tests! |
3 participants
In an older version of the crate,
http::Extensionswere notClone. This had the consequence thathttp::Requestandhttp::Responsewere also not clone. To work-around this limitation, we had functions in our http_ext module which would "clone" aRequestor aResponse, but they'd omitExtensions.Generally speaking, this wasn't a problem for most use cases. However, over time we started to add functionality, such as file uploads, to the router which relied on the correct behaviour of cloning and expected Extensions to be preserved across a clone.
This finally resulted in an issue for a customer who could not understand why modifying a request in a Rhai script would cause a File Upload to fail.
It transpires that at version 1.0.0 of the http crate, an incompatible change was made which forces
http::Extensionsto beClonewhich means bothhttp::Requestandhttp::Responseare nowClone.I've replaced our
http::Extensionsignoring implementation with the providedClonebehaviour and this means that Request and Response Extensions are now correctly cloned. I've added tests for this as well.Note: This can't be easily back-ported to 1.x.
httpis used all over the place and a test compile results in:I think that updating the code would be very intrusive.
Checklist
Complete the checklist (and note appropriate exceptions) before the PR is marked ready-for-review.
Exceptions
Note any exceptions here
Notes
Footnotes
It may be appropriate to bring upcoming changes to the attention of other (impacted) groups. Please endeavour to do this before seeking PR approval. The mechanism for doing this will vary considerably, so use your judgement as to how and when to do this. ↩
Configuration is an important part of many changes. Where applicable please try to document configuration examples. ↩
Tick whichever testing boxes are applicable. If you are adding Manual Tests, please document the manual testing (extensively) in the Exceptions. ↩