Skip to content

Conversation

@XiaoYingGee
Copy link

Problem

When using ParallelChannel, HTTP headers set on the parent controller (e.g., Authorization headers) are lost during sub-calls because they are not propagated to the sub-controllers.

Solution

  • Propagate all HTTP headers from parent controller to each sub-controller
  • Preserve application-set headers (like Authorization) on each sub-call
  • Maintain backward compatibility

mswuying and others added 2 commits August 11, 2025 12:03
Add judgement to avoid create new object when cntl dont have http_request
@wwbmmm wwbmmm changed the title Fix: Preserve HTTP headers in ParallelChannel sub-calls Fix: Preserve HTTP headers in ParallelChannel sub-calls Aug 12, 2025
@wwbmmm wwbmmm requested a review from Copilot August 12, 2025 04:51
Copy link

Copilot AI left a comment

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 fixes an issue where HTTP headers set on parent controllers are lost when making sub-calls through ParallelChannel. The fix ensures that application-set headers like Authorization are properly propagated to each sub-controller.

  • Adds header propagation logic to preserve parent controller headers in sub-calls
  • Maintains backward compatibility by only copying headers when they exist
  • Improves functionality for applications that rely on HTTP headers for authentication/authorization

Comment on lines +147 to +151
if (parent_hdr.HeaderBegin() != parent_hdr.HeaderEnd()) {
auto& sub_hdr = d->sub_done(i)->cntl.http_request();
for (auto it = parent_hdr.HeaderBegin(); it != parent_hdr.HeaderEnd(); ++it) {
sub_hdr.AppendHeader(it->first, it->second);
}
Copy link

Copilot AI Aug 12, 2025

Choose a reason for hiding this comment

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

[nitpick] The check for empty headers is redundant since the for loop will naturally handle empty header collections. This condition can be removed to simplify the code.

Suggested change
if (parent_hdr.HeaderBegin() != parent_hdr.HeaderEnd()) {
auto& sub_hdr = d->sub_done(i)->cntl.http_request();
for (auto it = parent_hdr.HeaderBegin(); it != parent_hdr.HeaderEnd(); ++it) {
sub_hdr.AppendHeader(it->first, it->second);
}
auto& sub_hdr = d->sub_done(i)->cntl.http_request();
for (auto it = parent_hdr.HeaderBegin(); it != parent_hdr.HeaderEnd(); ++it) {
sub_hdr.AppendHeader(it->first, it->second);

Copilot uses AI. Check for mistakes.
if (parent_hdr.HeaderBegin() != parent_hdr.HeaderEnd()) {
auto& sub_hdr = d->sub_done(i)->cntl.http_request();
for (auto it = parent_hdr.HeaderBegin(); it != parent_hdr.HeaderEnd(); ++it) {
sub_hdr.AppendHeader(it->first, it->second);
Copy link

Copilot AI Aug 12, 2025

Choose a reason for hiding this comment

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

Using AppendHeader may result in duplicate headers if the sub-controller already has headers with the same name. Consider using SetHeader instead to avoid potential header duplication, or check if the header already exists before appending.

Suggested change
sub_hdr.AppendHeader(it->first, it->second);
sub_hdr.SetHeader(it->first, it->second);

Copilot uses AI. Check for mistakes.
d->sub_done(i)->cntl.allow_done_to_run_in_place();

// Propagate all HTTP headers from parent controller to sub-controllers.
// This preserves application-set headers (e.g., Authorization) on each sub-call.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it need to check if sub-controller headers has exists key before AppendHeader or SetHeader? Avoid overwrite existing header.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants