Copilot commented on code in PR #3061:
URL: https://github.com/apache/brpc/pull/3061#discussion_r2268569823
##########
src/brpc/parallel_channel.cpp:
##########
@@ -139,6 +139,18 @@ class ParallelChannelDone : public
google::protobuf::Closure {
new (d->sub_done(i)) SubDone;
d->sub_done(i)->cntl.ApplyClientSettings(settings);
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.
+ if (cntl->has_http_request()) {
+ auto& parent_hdr = cntl->http_request();
+ 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);
+ }
Review Comment:
[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.
```suggestion
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);
```
##########
src/brpc/parallel_channel.cpp:
##########
@@ -139,6 +139,18 @@ class ParallelChannelDone : public
google::protobuf::Closure {
new (d->sub_done(i)) SubDone;
d->sub_done(i)->cntl.ApplyClientSettings(settings);
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.
+ if (cntl->has_http_request()) {
+ auto& parent_hdr = cntl->http_request();
+ 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);
Review Comment:
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.
```suggestion
sub_hdr.SetHeader(it->first, it->second);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org
