-
Notifications
You must be signed in to change notification settings - Fork 94
Fix security problems in mapper-attachments. #185
Conversation
Many of the tests were not running, or did not check the exceptions. I renamed all tests to meet *Tests* so they run, and assert exception messages. Also because we must (currently) invoke tika with additional privileges, I added the security logic, and fixed unit testing to call our static method directly. This must be package private for security reasons, i simply put everything in org.elasticsearch.mapper.attachments package. I upgraded tika to the latest, so we are up to date, and removed logic around tika == null and old locale issues.
@@ -10,5 +10,5 @@ | |||
- do: | |||
nodes.info: {} | |||
|
|||
- match: { nodes.$master.plugins.0.name: mapper-attachments } | |||
- match: { nodes.$master.plugins.0.name: elasticsearch-mapper-attachments } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this should be necessary. Esplugin defaults to the project name, and gradle defaults the project name to the directory name. Instead of changing this, if you add the name explicitly in build.gradle under the esplugin block, then it wont matter what directory the repo is checked out into.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is really trappy? We should just fail if this is not set.
Lgtm, one small comment about forcing the plugin name. |
Fix security problems in mapper-attachments.
Is it possible to backport most of those changes in other branches so the next releases will have those enhancements? |
We can backport all the commits once its stable? I want to get some good tests for each document we support. That means for complex formats like PDF, good mix with embedded/embedded-subset/external fonts, the whole shebang. I think we can get these test docs from tika itself or other apache licensed projects and then we know stuff works. |
Many of the tests were not running, or did not check the exceptions.
I renamed all tests to meet Tests so they run, and assert exception messages.
Also because we must (currently) invoke tika with additional privileges, I added
the security logic, and fixed unit testing to call our static method directly.
This must be package private for security reasons, i simply put everything in
org.elasticsearch.mapper.attachments package.
I upgraded tika to the latest, so we are up to date, and removed logic around
tika == null and old locale issues.