[IFT] switch to uri_template_system crate for URI template expansion. #1299
Conversation
The uri-template crate panics when encountering errors so switch to the uri-template-system crate which has error handling built into the API. Fixes fuzzer crashes from #1280.
| @@ -153,7 +153,8 @@ impl PatchGroup<'_> { | |||
| partial_invalidation_iftx, | |||
| mut no_invalidation_ift, | |||
| mut no_invalidation_iftx, | |||
| } = GroupingByInvalidation::group_patches(candidates, ift_compat_id, iftx_compat_id); | |||
| } = GroupingByInvalidation::group_patches(candidates, ift_compat_id, iftx_compat_id) | |||
| .map_err(|_| ReadError::MalformedData("Malformed URI templates."))?; | |||
There was a problem hiding this comment.
we have an inner error here that should provide more detail, why not preserve that in a BadUri error variant?
There was a problem hiding this comment.
I'm not sure I want to add a ReadError type that's specific to IFT since ReadError is more general than IFT. MalformedData does capture the essence of the error and in this case the inner type doesn't have any additional information that the error string doesn't already capture.
|
In relation to your earlier comment on the issue? Do we benefit a lot from the dependency? What is the main functionality we use and could we do without the dependency, what's the tradeoff? |
This dep is an implementation of https://datatracker.ietf.org/doc/html/rfc6570 which is needed for the IFT mechanism. Writing our own would be a sizable task so I decided to stick with an existing crate in order to get this fuzzer issue sorted out in a timely fashion. I haven't ruled out replacing it with our own at some future point if we're looking to trim off dependencies. |
Thanks for the clarifications. Which parts, as in: replacement rule/format, of the RFC are we using and expecting to use in the future? Is the IFT spec allowing multiple types of replacements, or only few of them? Ok with me to land this for now, but I would like to understand if we can strip a dependency. |
We are using nearly the entirety of that spec. Specifically in the IFT client we do expansion of templates utilizing the syntax from the RFC and as the IFT spec is currently written all four operator levels are supported. The only part we're not using is expansion of composite values. For reference here's the relevant section from IFT. |
Makes sense, thanks. |
The uri-template crate panics when encountering errors so switch to the uri-template-system crate which has error handling built into the API. Fixes fuzzer crashes from #1280.