Skip to content

Traffic Dump: Use the correct transaction user index#8548

Merged
bneradt merged 1 commit intoapache:masterfrom
bneradt:fix_traffic_dump_txn_user_arg
Dec 8, 2021
Merged

Traffic Dump: Use the correct transaction user index#8548
bneradt merged 1 commit intoapache:masterfrom
bneradt:fix_traffic_dump_txn_user_arg

Conversation

@bneradt
Copy link
Contributor

@bneradt bneradt commented Dec 8, 2021

Traffic Dump has both session and transaction user data. In one of the
calls to TSUserArgGet, it accidentally used the session index to access
the transaction data. This could result in the corruption of another
plugin's session data. This patch fixes this so the correct index is
used.


In parallel I'm working on a core change that will detect incorrect indexing rather than allow the accidental corrupting of another plugin's data. See: #8550.

Traffic Dump has both session and transaction user data. In one of the
calls to TSUserArgGet, it accidentally used the session index to access
the transaction data. This could result in the corruption of another
plugin's session data. This patch fixes this so the correct index is
used.
@bneradt bneradt added the Plugins label Dec 8, 2021
@bneradt bneradt added this to the 10.0.0 milestone Dec 8, 2021
@bneradt bneradt requested a review from randall December 8, 2021 00:23
@bneradt bneradt self-assigned this Dec 8, 2021
@bneradt bneradt requested a review from bryancall as a code owner December 8, 2021 00:23
Copy link
Contributor

@brbzull0 brbzull0 left a comment

Choose a reason for hiding this comment

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

Good catch!

@bneradt bneradt merged commit 42ad946 into apache:master Dec 8, 2021
@bneradt bneradt deleted the fix_traffic_dump_txn_user_arg branch December 8, 2021 19:16
@ywkaras
Copy link
Contributor

ywkaras commented Dec 9, 2021

Does this need to go into 9.1.x and any 8.x maintenance releases?

@bneradt
Copy link
Contributor Author

bneradt commented Dec 9, 2021

Does this need to go into 9.1.x and any 8.x maintenance releases?

I'll add it to 9.1.x. The 8.1.x traffic_dump plugin is pretty minimal and does not have this bug.

Thanks.

zwoop pushed a commit that referenced this pull request Jan 5, 2022
Traffic Dump has both session and transaction user data. In one of the
calls to TSUserArgGet, it accidentally used the session index to access
the transaction data. This could result in the corruption of another
plugin's session data. This patch fixes this so the correct index is
used.

(cherry picked from commit 42ad946)
zwoop pushed a commit that referenced this pull request Jan 5, 2022
Traffic Dump has both session and transaction user data. In one of the
calls to TSUserArgGet, it accidentally used the session index to access
the transaction data. This could result in the corruption of another
plugin's session data. This patch fixes this so the correct index is
used.

(cherry picked from commit 42ad946)
@zwoop
Copy link
Contributor

zwoop commented Jan 5, 2022

Cherry-picked to v9.1.x branch.
Cherry-picked to v9.2.x

@zwoop zwoop modified the milestones: 10.0.0, 9.1.2 Jan 5, 2022
@zwoop zwoop added the 9.2.0 label Jan 5, 2022
bneradt added a commit to bneradt/trafficserver that referenced this pull request Jan 19, 2022
This reverts apache#8548 and instead directly calls
HttpTransact::need_to_revalidate in TSHttpTxnCacheLookupStatusGet to
ensure that an object which is a cache hit is indeed something ATS can
return to the client.

Fixes apache#8616
bneradt added a commit that referenced this pull request Jan 19, 2022
This reverts #8548 and instead directly calls
HttpTransact::need_to_revalidate in TSHttpTxnCacheLookupStatusGet to
ensure that an object which is a cache hit is indeed something ATS can
return to the client.

Fixes #8616
zwoop pushed a commit that referenced this pull request Jan 25, 2022
This reverts #8548 and instead directly calls
HttpTransact::need_to_revalidate in TSHttpTxnCacheLookupStatusGet to
ensure that an object which is a cache hit is indeed something ATS can
return to the client.

Fixes #8616

(cherry picked from commit 235c44a)
moonchen pushed a commit to moonchen/trafficserver that referenced this pull request Mar 17, 2022
* asf/9.2.x:
  Updated ChangeLog
  docs: fix fedora install notes and spelling issues (apache#8537)
  Docs: Fix default value of proxy.config.ssl.handshake_timeout_in (apache#8574)
  Partial of revert "Cleanup generated LDFLAGS for jemalloc (apache#8285)" (apache#8533)
  TSUserArg: add value type checking (apache#8550)
  Relax key validation of sni.yaml (apache#8549)
  Clear random header value by AIO read error (apache#8559)
  Fixes macOS arm64 builds (again) (apache#8556)
  Traffic Dump: Use the correct transaction user index (apache#8548)
  combo_handler: Initialize User Arg Index in TSRemapInit (apache#8551)
  backout down parent retry limiting in parent selection and nexthop (apache#8546)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants