Skip to content

Conversation

lorban
Copy link
Contributor

@lorban lorban commented Sep 8, 2025

BufferUtil.toMappedBuffer(Path) was meant to catch UnsupportedOperationException but it mistakenly was coded to catch UnsupportedEncodingException instead.

Fix the typo and add related tests.

Fixes #13563

…ionException and add related tests

Signed-off-by: Ludovic Orban <[email protected]>
@lorban lorban requested review from gregw and joakime September 8, 2025 09:01
@lorban lorban self-assigned this Sep 8, 2025
@lorban lorban added the Bug For general bugs on Jetty side label Sep 8, 2025
@lorban lorban moved this to 👀 In review in Jetty 12.1.2 - FROZEN Sep 8, 2025
gregw
gregw previously approved these changes Sep 8, 2025
Copy link
Contributor

@gregw gregw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good fix.

However, I'm also wondering if we should maintain a set of FileSystems from the passed in Path filePath that throw UOE and if a path is from such a file system, we just return null without the expense of creating an exception on every lookup.

For the most common case, the cost of looking at an empty set should be trivial.

On the other hand, perhaps the content cache makes this unnecessary?

Either way, if you think we should consider this, then merge this PR and open an enhancement issue for later consideration.

Signed-off-by: Ludovic Orban <[email protected]>
@lorban
Copy link
Contributor Author

lorban commented Sep 9, 2025

I'm of the opinion that the content cache makes any form of FileSystems caching unneeded.

@lorban lorban merged commit c3bd8e6 into jetty-12.1.x Sep 9, 2025
10 checks passed
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in Jetty 12.1.2 - FROZEN Sep 9, 2025
@lorban lorban deleted the fix/jetty-12.1.x/13563-mapping-buffers-util branch September 9, 2025 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

Jetty 12.1.0 fails to serve big (> 1MiB) static web resources located inside a jar file
3 participants