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

Make the core ResourceService a bit more reusable for subclasses #12281

Draft
wants to merge 1 commit into
base: jetty-12.0.x
Choose a base branch
from

Conversation

garydgregory
Copy link
Contributor

@garydgregory garydgregory commented Sep 17, 2024

Make the core ResourceService a bit more reusable for subclasses

  • The goal is to allow a subclass to look at incoming Request and HttpContent objects and customize processing
  • Refactor sendDirectory() to call a new getSendDirectoryResource() method
  • Make sendData() protected

- The goal is to allow a subclass to look at incoming Request and
HttpContent objects and customize processing
- Refactor sendDirectory() to call a new getDirectoryResource() method
- Make sendData() protected
@garydgregory
Copy link
Contributor Author

CC: @lorban

@lorban
Copy link
Contributor

lorban commented Sep 18, 2024

Can you share an example of subclass you'd like to create? Feel free to modify ResourceService as you wish, we'll consider the best way to achieve your goal.

Thanks!

@garydgregory
Copy link
Contributor Author

garydgregory commented Sep 18, 2024

Here is what I am after and I can update this PR is the idea is acceptable.

The code I currently have is hard-coded to Jetty EE9 classes, but there is nothing really EE9 about it. I would prefer to configure Jetty using Core classes. I've recently refactored it to make it work with Jetty Core and EE9 without too much duplication, but I can't quite get there cleanly with the way Jetty Core classes are written today.

I have a class FilteredResourceService that extends org.eclipse.jetty.ee9.nested.ResourceService that you configure with a regular expression and a flag (the arguments to Pattern.compile(String, int)) such that when a directory is listed, only resources that match the regex are returned. This lets us "safely" serve Swagger files (like .*\.openapi|.*\.swagger), for example, without worrying that users can download whatever else might be laying around.

I do not want/cannot use a Jetty resource Factory because of the use case I'll outline later.

I believe the above could be folder into the Core ResourceService if the Core ResourceHandler adds a setter and/or constructor that takes a Core ResourceService. Right now I subclass ResourceService and override newResourceService() to provide a different ResourceService. This would let me configure a ResourceService with a ResourceService which itself defines a regex.

I am not suggesting the following is appropriate for Jetty but this is what we need: A subclass of EE9 FilteredResourceService that looks at each request and request content to perform string substitution on the contents of the files. This is done using Apache Commons Text's StringSubstitutor. The string replacement needs data from the request (for example the host name, and so on). The only to do what we need is to override org.eclipse.jetty.ee9.nested.ResourceService.sendData(HttpServletRequest, HttpServletResponse, boolean, HttpContent, Enumeration<String>) and do our business there. We basically build a new HttpContent and pass it to super.

The above all works as Jetty EE9 code. I think this would be better as Jetty Core code.

The scope of the PR would be to edit Jetty Core and Jetty EE9 such that each ResourceService can use a regex for directory listings.

@joakime
Copy link
Contributor

joakime commented Sep 18, 2024

I do not want/cannot use a Jetty resource Factory because of the use case I'll outline later.

You can wrap Resource, and that can provide the regex based filter in Resource.list() as well.
No need to be messing with ResourceFactory.

@joakime
Copy link
Contributor

joakime commented Sep 18, 2024

The scope of the PR would be to edit Jetty Core and Jetty EE9 such that each ResourceService can use a regex for directory listings.

Having ResourceService support regex on directory listings seems like a natural evolution of the API.

But I think having it as ResourceService.setDirectoryListingPredicate(Predicate<Resource> listingPredicate) would be a more versatile approach.

You can do it via regex, someone else could do it via filenames, someone else could do it via java.nio.file.PathMatcher, etc ...

@garydgregory garydgregory marked this pull request as draft September 19, 2024 13:14
@lorban
Copy link
Contributor

lorban commented Sep 20, 2024

Opening up the internals of ResourceService as an API looks sketchy to me, so I agree with @joakime that adding a method to add a Predicate<Resource> filter that would be passed on to ResourceListing.getAsXHTML() would be more natural and versatile.

@garydgregory
Copy link
Contributor Author

Thank you all for the suggestions. I'll update this PR over the next couple of days.

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