Skip to content

jax_fingerprint: Emit type-default for custom log fields with no transaction#13243

Merged
maskit merged 1 commit into
apache:masterfrom
maskit:jax_no_txn
Jun 8, 2026
Merged

jax_fingerprint: Emit type-default for custom log fields with no transaction#13243
maskit merged 1 commit into
apache:masterfrom
maskit:jax_no_txn

Conversation

@maskit

@maskit maskit commented Jun 6, 2026

Copy link
Copy Markdown
Member

When a log entry has no backing HttpSM (e.g. a malformed H2/H3 request rejected before transaction creation), passing nullptr to a TSLogFieldRegister'd callback dereferences null inside the plugin. Skip the callback in that case and emit the type-appropriate default the way built-in fields do: 0 for ints, "-" for strings, AF_UNSPEC for IPs.

When a log entry has no backing HttpSM (e.g. a malformed H2/H3
request rejected before transaction creation), passing
nullptr to a TSLogFieldRegister'd callback dereferences null
inside the plugin.  Skip the callback in that case and emit the
type-appropriate default the way built-in fields do: 0 for ints,
"-" for strings, AF_UNSPEC for IPs.
@maskit maskit added this to the 11.0.0 milestone Jun 6, 2026
@maskit maskit self-assigned this Jun 6, 2026
Copilot AI review requested due to automatic review settings June 6, 2026 01:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 hardens ATS custom log field marshalling for cases where a log entry is generated without an HttpSM (e.g., malformed H2/H3 requests rejected before transaction creation). Instead of calling plugin-provided TSLogFieldRegister callbacks with a null transaction pointer (which can crash plugins), it emits the same type-appropriate defaults used by built-in fields.

Changes:

  • Plumbs LogField::Type into LogAccess::marshal_custom_field() so it can select a default value when there’s no backing transaction.
  • Skips the plugin callback when http_sm_for_plugins() is null and marshals a default value: 0 (ints), "-" (strings), AF_UNSPEC (IPs).
  • Updates LogField call sites and the LogAccess declaration to match the new signature.

Reviewed changes

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

File Description
src/proxy/logging/LogField.cc Passes the field type into custom-field marshalling for both sizing and data writes.
src/proxy/logging/LogAccess.cc Implements type-based default marshalling when no transaction is available, avoiding null plugin callbacks.
include/proxy/logging/LogAccess.h Updates the marshal_custom_field API to accept LogField::Type.

@amirnoshian

Copy link
Copy Markdown

I’m trying to build the project using version 8, but I’m encountering some issues.

When building traffic_opc with the Dockerfile, there seems to be a problem with pdgp:13. It looks like this dependency or image might need to be changed.

Additionally, I’m also having issues with the Traffic Ops Dockerfile during the build process.

Does anyone have any suggestions on how to resolve these problems?

@JosiahWI

JosiahWI commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

@amirnoshian If you could open an issue that would help us track it. This is not the right thread for your comment. Please ask if you would like help opening an issue or have questions about it.

@amirnoshian

amirnoshian commented Jun 8, 2026 via email

Copy link
Copy Markdown

@bneradt

bneradt commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

[approve ci osx freebsd]

@maskit maskit merged commit e23068d into apache:master Jun 8, 2026
15 checks passed
@github-project-automation github-project-automation Bot moved this to For v10.2.0 in ATS v10.2.x Jun 8, 2026
@maskit maskit deleted the jax_no_txn branch June 8, 2026 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: For v10.2.0

Development

Successfully merging this pull request may close these issues.

5 participants