feat: allow users to restrict http requests to certain paths#285
feat: allow users to restrict http requests to certain paths#285
Conversation
* This commit creates the AllowHttpRequestPaths type, which is then plugged into the AllowCertainHttpRequests. Users of AllowCertainHttpRequests can now pass in a set of 'allowed paths' which will restrict which HTTP request paths the host allows the guest to access.
| /// Set of all allowed paths. | ||
| /// | ||
| /// The request path must start with one of these paths to be allowed. | ||
| allowed_paths: AllowHttpRequestPath, |
There was a problem hiding this comment.
I think the paths should be bound to the matcher since different hosts may have different path filters, i.e. it's likely matchers: HasMap<HttpRequestMatcher, AllowedHttpPath>, although that interface becomes a bit of a mess. I suggest the following: the public interface is based on HttpRequestMatcher and you add the add the list of allowed prefixes to that struct. For fast internal filtering I think you have two options, since you now need to replace the HashSet in AllowCertainHttpRequests:
- hash map: Model the lookup table as
HashMap<MatcherWithoutPrefixes, Prefixes> - binary search: Encode each prefix in each matcher as something like
method\0host\0port\0prefixland perform a binary search on that data - tree: Same as the binary search but as some kind of search tree.
| &StringArray::from_iter([Some("hello world!".to_owned()),]) as &dyn Array, | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
After you've changed it to a "prefixes are matcher-specific", you should probably also harded test_integration to only accept the paths that are part of the specific test cases.
|
I'm dubious about the utility of this configuration. HTTP proxies already exists which provide much more sophisticated request filtering, I'm not sure the reproducing a subset of that functionality is more useful than just forcing the HTTP request through a configured proxy and have that do filtering. |
I think this depends on where you want all this logic to live and how to deploy it. "HTTP proxies already exists" is correct, but for deployment that means that you need yet-another-service running somewhere. And if that's so easy or if you want to do that kinda depends on where you are in the monolith<>micro-service spectrum. |
One thing I'd like to add is that although this is basically duplicated work, (that is, a solution already exists) doing it this way gives us:
Now both of the above do increase maintenance burden, but I think that's an acceptable tradeoff as there would still be a maintenance burden when using a 3rd party. |
Helps #227