Skip to content

Conversation

simonresch
Copy link
Contributor

@simonresch simonresch commented Oct 10, 2025

Adds a test to check that MethodHook annotations in sanitizers reference existing classes & methods.

@simonresch simonresch marked this pull request as ready for review October 10, 2025 06:05
@simonresch simonresch marked this pull request as draft October 10, 2025 06:23
@simonresch simonresch force-pushed the fix-hooked-method-descriptors branch 2 times, most recently from c7f252d to 2ccf382 Compare October 10, 2025 12:27
@simonresch simonresch marked this pull request as ready for review October 10, 2025 13:14
@simonresch simonresch requested review from a team and Copilot October 10, 2025 13:15
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a comprehensive test to validate that all @MethodHook annotations in sanitizers reference existing classes and methods, while also fixing several typos and descriptor errors discovered by this validation.

  • Introduces HookBindingSanityTest to verify method hook annotations reference valid targets
  • Fixes multiple typos in method descriptors across sanitizer classes
  • Corrects class name references and removes duplicate/invalid hooks

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
HookBindingSanityTest.java New test class that validates method hook annotations against actual class/method existence
BUILD.bazel (test) Adds build configuration for the new test with required dependencies
RegexRoadblocks.java Fixes return type in method descriptor from CharPredicate to BmpCharPredicate
LdapInjection.kt Corrects typos in method descriptors (namespace separator and parameter type)
FilePathTraversal.java Fixes class name typo and removes duplicate method hook
BUILD.bazel (main) Updates visibility to allow test access to sanitizer classes
maven_install.json Adds new Maven dependencies required for testing
MODULE.bazel Declares new Maven artifacts for Jakarta EL and JPA APIs

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@simonresch simonresch force-pushed the fix-hooked-method-descriptors branch 2 times, most recently from 8511bfa to 909680c Compare October 15, 2025 13:23
Copy link
Contributor

@oetr oetr left a comment

Choose a reason for hiding this comment

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

Awesome work, especially with the tests!

@simonresch simonresch force-pushed the fix-hooked-method-descriptors branch from 909680c to b45d2eb Compare October 15, 2025 16:52
@simonresch simonresch enabled auto-merge (rebase) October 15, 2025 16:53
@simonresch simonresch merged commit d88ad22 into main Oct 15, 2025
10 checks passed
@simonresch simonresch deleted the fix-hooked-method-descriptors branch October 15, 2025 17:03
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