On Mon, 16 Feb 2026 08:02:09 GMT, Erik Gahlin <[email protected]> wrote:

> For more information. see:
> 
> https://bugs.openjdk.org/browse/JDK-8372760
> 
> Testing: tier1 + test/jdk/jdk/jfr + manual testing
> 
> - [x] I confirm that I make this contribution in accordance with the [OpenJDK 
> Interim AI Policy](https://openjdk.org/legal/ai).

src/hotspot/share/jfr/periodic/jfrPeriodic.cpp line 103:

> 101: 
> 102: TRACE_REQUEST_FUNC(JVMInformation) {
> 103:   ResourceMark rm;

I see we had a ResourceMark in previous implementation, do we still need this 
or none of the new function calls use this type of allocation ?

src/hotspot/share/jfr/periodic/jfrPeriodic.cpp line 259:

> 257:     while (processes != nullptr) {
> 258:       SystemProcess* tmp = processes;
> 259:       const char* info = processes->command_line();

The intent of the change here is to remove command line arguments for other 
processes.
I understand that these are likely to contain sensitive data, though I did not 
see a mention of this change in the 
[issue](https://bugs.openjdk.org/browse/JDK-8372760)

src/hotspot/share/jfr/periodic/jfrRedactedEvents.cpp line 118:

> 116: }
> 117: 
> 118: void JfrRedactedEvents::add_default_filters(StringArray* target, bool 
> argument) {

I do not think there is a risk in adding more considering you added a mechanism 
to override these defaults.
- certificates or even paths to certificates are not something we want to 
capture: `cert`
- signing keys or anything containing signing: `signing`
- encryption strings: `encryption`
- access is also used to describe some secrets: `access` 
- Jason web tokens: `jwt`
- <optional> bearer might be interesting to add, though it is associated to 
token, so we should be already covered.

src/hotspot/share/jfr/periodic/jfrRedactedEvents.cpp line 376:

> 374:   delete flags_args;
> 375: 
> 376:   _initialized = true;

💭 does the `_initialized` flag need to be an atomic ?

src/hotspot/share/jfr/periodic/jfrRedactedEvents.cpp line 589:

> 587:     return false;
> 588:   }
> 589:   char buffer[512];

💭 We could add a log if we exceed the size of the buffer

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/29736#discussion_r2846480314
PR Review Comment: https://git.openjdk.org/jdk/pull/29736#discussion_r2846537606
PR Review Comment: https://git.openjdk.org/jdk/pull/29736#discussion_r2859338731
PR Review Comment: https://git.openjdk.org/jdk/pull/29736#discussion_r2846423024
PR Review Comment: https://git.openjdk.org/jdk/pull/29736#discussion_r2846568438

Reply via email to