Skip to content

(DNM) update WASM plugin to support http calls and return value of TSHttpTxnIsInternal() calls - #10359

Closed
thebadpete wants to merge 8 commits into
apache:masterfrom
thebadpete:wasm-txn-is-internal
Closed

(DNM) update WASM plugin to support http calls and return value of TSHttpTxnIsInternal() calls#10359
thebadpete wants to merge 8 commits into
apache:masterfrom
thebadpete:wasm-txn-is-internal

Conversation

@thebadpete

Copy link
Copy Markdown
Contributor

No description provided.

@shukitchan
shukitchan self-requested a review September 1, 2023 21:40
@shukitchan shukitchan self-assigned this Sep 1, 2023
@shukitchan shukitchan added this to the 10.0.0 milestone Sep 1, 2023

@shukitchan shukitchan 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.

Please take a look at my comments. Thanks.

path = value;
} else if (key == ":authority") {
authority = value;
} else if (key == ":protocol") {

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.

It should be ":scheme" instead according to
https://portswigger.net/burp/documentation/desktop/http2/http2-basics-for-burp-users
Please check out the section in pseudo-headers.

} else if (path.substr(0, p_txn_is_internal.size()) == p_txn_is_internal) {
int internal = TSHttpTxnIsInternal(txnp_);
result->assign(reinterpret_cast<const char *>(&internal), sizeof(int));
return WasmResult::Ok;

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.

I am actually thinking of using foreign function instead for this. e.g. in envoy it has "compress and "uncompress" foreign function exposed to wasm modules.

This is how it is defined -
https://github.com/envoyproxy/envoy/blob/d12d47b05a832ce04d9130a869fa72f7ddbcef28/source/extensions/common/wasm/foreign.cc#L43

This is how it is used -
https://github.com/envoyproxy/envoy/blob/d12d47b05a832ce04d9130a869fa72f7ddbcef28/test/extensions/common/wasm/test_data/test_cpp.cc#L158

Please take a look and see if we can implement this using foreign function

@shukitchan

Copy link
Copy Markdown
Contributor

And let's move this PR to draft for now till it is ready.

@thebadpete

Copy link
Copy Markdown
Contributor Author

closing this pr for now, will refile after addressing the feeedback

@thebadpete thebadpete closed this Sep 9, 2023
@zwoop zwoop removed this from the 10.0.0 milestone Feb 29, 2024
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.

3 participants