Merge pull request #51 from stephenc-pace/fix/invalid-headers

Escape error message header

.github/workflows/ci.yaml

@@ -15,10 +15,10 @@ jobs:
     runs-on: ubuntu-latest
     steps:
     - name: checkout
-      uses: actions/checkout@v2
+      uses: actions/checkout@v3
 
     - name: setup python
-      uses: actions/setup-python@v2
+      uses: actions/setup-python@v4
       with:
         python-version: 3.9
 
@@ -33,7 +33,7 @@ jobs:
          TOXENV: static
 
   test:
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-20.04
     strategy:
       matrix:
         python-version:
@@ -44,10 +44,10 @@ jobs:
 
     steps:
     - name: checkout
-      uses: actions/checkout@v2
+      uses: actions/checkout@v3
 
     - name: setup python
-      uses: actions/setup-python@v2
+      uses: actions/setup-python@v4
       with:
         python-version: ${{ matrix.python-version }}
 
@@ -74,10 +74,10 @@ jobs:
 
     steps:
     - name: checkout
-      uses: actions/checkout@v2
+      uses: actions/checkout@v3
 
     - name: setup python
-      uses: actions/setup-python@v2
+      uses: actions/setup-python@v4
       with:
         python-version: 3.9
 

.gitignore

@@ -3,4 +3,8 @@ __pycache__
 .tox
 *_pb2.py
 *_pb2_grpc.py
-.coverage
+.coverage
+.DS_Store
+.idea/
+build/
+dist/

nameko_grpc/errors.py

@@ -4,6 +4,8 @@
 from grpc import StatusCode
 from nameko import config
 
+from nameko_grpc.headers import quote_decode, quote_encode
+
 from google.protobuf.any_pb2 import Any
 from google.rpc.error_details_pb2 import DebugInfo
 from google.rpc.status_pb2 import Status
@@ -26,7 +28,7 @@ def as_headers(self):
         headers = {
             # ("content-length", "0"),
             "grpc-status": str(STATUS_CODE_ENUM_TO_INT_MAP[self.code]),
-            "grpc-message": self.message,
+            "grpc-message": quote_encode(self.message),
         }
         if self.status:
             headers[GRPC_DETAILS_METADATA_KEY] = self.status.SerializeToString()
@@ -41,7 +43,7 @@ def from_headers(cls, headers):
 
         return cls(
             code=STATUS_CODE_INT_TO_ENUM_MAP[code],
-            message=message,
+            message=quote_decode(message),
             status=Status.FromString(status) if status else None,
         )
 

nameko_grpc/headers.py

@@ -1,5 +1,6 @@
 # -*- coding: utf-8 -*-
 import base64
+from urllib.parse import quote, unquote
 
 
 def is_grpc_header(name):
@@ -83,6 +84,24 @@ def comma_join(values):
     return separator.join(values)
 
 
+_UNQUOTED = "".join(
+    [chr(i) for i in range(0x20, 0x24 + 1)] + [chr(i) for i in range(0x26, 0x7E + 1)]
+)
+
+
+def quote_encode(message):
+    """
+    We must percent encode error messages to ensure we don't transmit invalid
+    header values.
+    ref: https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#requests
+    """
+    return quote(message, safe=_UNQUOTED, encoding="utf-8")
+
+
+def quote_decode(value):
+    return unquote(value, encoding="utf-8", errors="replace")
+
+
 class HeaderManager:
     def __init__(self):
         """

test/test_errors.py

@@ -2,6 +2,7 @@
 import json
 import re
 import string
+import sys
 import time
 from unittest import mock
 
@@ -376,3 +377,26 @@ def test_response_stream_closed(self, client, protobufs):
                 client.unary_unary(protobufs.ExampleRequest(value="hello"))
         assert error.value.code == StatusCode.UNAVAILABLE
         assert error.value.message == "Stream was closed mid-request"
+
+
+class TestGrpcErrorEncodesHeader:
+    def test_encode_grpc_message(self):
+        try:
+            raise NotImplementedError("\na")
+        except NotImplementedError:
+            error = GrpcError.from_exception(sys.exc_info())
+
+        headers = error.as_headers()
+        grpc_header = [header for header in headers if header[0] == "grpc-message"][0]
+        assert grpc_header == ("grpc-message", "%0Aa")
+
+    def test_decode_grpc_message(self):
+        try:
+            raise NotImplementedError("\na")
+        except NotImplementedError:
+            error = GrpcError.from_exception(sys.exc_info())
+
+        headers_dict = {header: value for (header, value) in error.as_headers()}
+        error_received = GrpcError.from_headers(headers_dict)
+
+        assert error_received.message == "\na"

tox.ini

@@ -3,7 +3,7 @@ envlist = static, {py3.6,py3.7,py3.8,py3.9}-test
 skipsdist = True
 
 [testenv]
-whitelist_externals = make
+allowlist_externals = make
 
 commands =
     static: pip install pre-commit