Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

send local response: clarify call sequence #73

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion abi-versions/v0.2.1/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1156,7 +1156,8 @@ Plugin must return one of the following values:

Sends HTTP response with body (`body_data`, `body_size`) and
[serialized] headers (`serialized_headers_data`,
`serialized_headers_size`).
`serialized_headers_size`). The response is sent after the current
callback completes and triggers response callbacks.
Copy link
Member

Choose a reason for hiding this comment

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

This is the opposite of the intended behavior (which is that proxy_send_local_response() is always a final call).

We want plugins to do something like this:

proxy_on_request_headers() {
   ...
   return proxy_send_local_response(...);
}

and not something like this:

proxy_on_request_headers() {
   ...
   proxy_send_local_response(...);
   do_something_very_important();
   return Pause;
}

The reason this is deferred in Envoy is because the response callbacks (i.e. proxy_on_response_headers() and friends) are called immediately upon sending local response, which leads to re-entrancy problems, but that's another issue (I don't think that plugins should "receive" the local responses they generated themselves).

Copy link
Author

Choose a reason for hiding this comment

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

@mpwarres I think we have to specify it one way or another. How does WaaS behave here?


This can be used as long as HTTP response headers were not sent downstream.

Expand Down