Fix bug with non-ASCII HTTP headers

[steinarvk-oda]

gRPC metadata (for the non-binary keys) must be printable ASCII, i.e. each byte must be between 0x20 and 0x7E inclusive.

References to other Issues or PRs

Have not created an issue, but this fixes a problem we've had in our deployment of argoproj/argo-workflows (which uses grpc-gateway) where any API request with a header value containing a non-ASCII character gets rejected.

HTTP headers can by convention be general ISO-8859-1, not just US ASCII. E.g. 'æ', 'ø', 'å' should be fine in HTTP headers, but they're not acceptable in gRPC metadata.

Have you read the Contributing Guidelines?

Yes.

Brief description of what is fixed or changed

When copying HTTP headers over to gRPC metadata, skip headers where the key or the value would not be accepted by gRPC. (These requests would be rejected anyway if the metadata were included.)

Other comments

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[johanbrandhorst]

Hi, thanks for your PR! Can you please add a test for this too? It could be an integration test, see https://github.com/grpc-ecosystem/grpc-gateway/blob/main/examples/internal/integration/integration_test.go.

[johanbrandhorst]

LGTM

[johanbrandhorst]

Looks like we have a bazel error, see CONTRIBUTING.md for how do auto-update the bazel files (it's because of the new import).

[steinarvk-oda]

Is the change in runtime/BUILD.bazel the only one required?

I ran the commands using docker, but the final build of :buildifier failed.

[...]
INFO: Analyzed target //:buildifier (102 packages loaded, 2094 targets configured).
INFO: Found 1 target...
ERROR: /home/vscode/.cache/bazel/_bazel_vscode/040e700c618bb93b01649f0eaa236eab/external/com_google_absl/absl/time/internal/cctz/BUILD.bazel:32:11: Compiling absl/time/internal/cctz/src/zone_info_source.cc [for tool] failed: (Exit 1): gcc failed: error executing command (from target @com_google_absl//absl/time/internal/cctz:time_zone) /usr/bin/gcc -U_FORTIFY_SOURCE -fstack-protector -Wall -Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer -g0 -O2 '-D_FORTIFY_SOURCE=1' -DNDEBUG -ffunction-sections ... (remaining 22 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
In file included from external/com_google_absl/absl/base/config.h:86,
                 from external/com_google_absl/absl/time/internal/cctz/include/cctz/zone_info_source.h:23,
                 from external/com_google_absl/absl/time/internal/cctz/src/zone_info_source.cc:15:
external/com_google_absl/absl/base/policy_checks.h:79:2: error: #error "C++ versions less than C++14 are not supported."
   79 | #error "C++ versions less than C++14 are not supported."
      |  ^~~~~
ERROR: /home/vscode/.cache/bazel/_bazel_vscode/040e700c618bb93b01649f0eaa236eab/external/com_google_absl/absl/base/BUILD.bazel:53:11: Compiling absl/base/log_severity.cc [for tool] failed: (Exit 1): gcc failed: error executing command (from target @com_google_absl//absl/base:log_severity) /usr/bin/gcc -U_FORTIFY_SOURCE -fstack-protector -Wall -Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer -g0 -O2 '-D_FORTIFY_SOURCE=1' -DNDEBUG -ffunction-sections ... (remaining 37 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
In file included from external/com_google_absl/absl/base/config.h:86,
                 from external/com_google_absl/absl/base/attributes.h:37,
                 from external/com_google_absl/absl/base/log_severity.h:21,
                 from external/com_google_absl/absl/base/log_severity.cc:15:
external/com_google_absl/absl/base/policy_checks.h:79:2: error: #error "C++ versions less than C++14 are not supported."
   79 | #error "C++ versions less than C++14 are not supported."
      |  ^~~~~
Target //:buildifier failed to build
Use --verbose_failures to see the command lines of failed build steps.
ERROR: /home/vscode/.cache/bazel/_bazel_vscode/040e700c618bb93b01649f0eaa236eab/external/com_google_protobuf/src/google/protobuf/BUILD.bazel:142:11 Middleman _middlemen/@com_Ugoogle_Uprotobuf_S_Ssrc_Sgoogle_Sprotobuf_Cwkt_Ucc_Uproto-cc_library-compile failed: (Exit 1): gcc failed: error executing command (from target @com_google_absl//absl/base:log_severity) /usr/bin/gcc -U_FORTIFY_SOURCE -fstack-protector -Wall -Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer -g0 -O2 '-D_FORTIFY_SOURCE=1' -DNDEBUG -ffunction-sections ... (remaining 37 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
INFO: Elapsed time: 5.418s, Critical Path: 0.84s
INFO: 215 processes: 210 internal, 5 processwrapper-sandbox.
FAILED: Build did NOT complete successfully
ERROR: Build failed. Not running target

Nevertheless, maybe the file that needed regenerating was regenerated by an earlier step?

[johanbrandhorst]

Hm, you can fix that error with a .bazelrc like so: https://github.com/grpc-ecosystem/grpc-gateway/blob/main/.github/workflows/ci.yml#L72-L84. I didn't know it would affect users, we will have to update the contribution guide, thanks for letting me know. I think that single BUILD file change was probably all that was needed.

[steinarvk-oda]

Okay, looks like this is ready to merge then? Thanks for the review!

[johanbrandhorst]

Thank you for your contribution :)!

[johanbrandhorst]

This fix was just released with v2.15.1.