From e634f9749fe32b71ff3cbe40c368dfad0745ac12 Mon Sep 17 00:00:00 2001 From: Vyacheslav Egorov Date: Thu, 12 Nov 2020 09:51:57 +0100 Subject: [PATCH] Fix for duplicate headers (#397) --- .github/workflows/dart.yml | 11 +- CHANGELOG.md | 2 + CONTRIBUTING.md | 6 +- lib/src/client/transport/xhr_transport.dart | 3 - test/grpc_web_server.dart | 96 +++++++++++---- test/grpc_web_test.dart | 124 +++++++++++++------- tool/install-grpcwebproxy.sh | 44 ------- 7 files changed, 168 insertions(+), 118 deletions(-) delete mode 100755 tool/install-grpcwebproxy.sh diff --git a/.github/workflows/dart.yml b/.github/workflows/dart.yml index 0490b2b..623d725 100644 --- a/.github/workflows/dart.yml +++ b/.github/workflows/dart.yml @@ -65,11 +65,16 @@ jobs: release-channel: ${{ matrix.sdk }} - name: Report version run: dart --version - - name: Install grpcwebproxy + - name: Install envoy if: ${{ matrix.platform == 'chrome' }} run: | - ./tool/install-grpcwebproxy.sh - echo "/tmp/grpcwebproxy" >> $GITHUB_PATH + sudo apt update + sudo apt install apt-transport-https ca-certificates curl gnupg-agent software-properties-common + curl -sL 'https://getenvoy.io/gpg' | sudo apt-key add - + apt-key fingerprint 6FF974DB | grep "5270 CEAC 57F6 3EBD 9EA9 005D 0253 D0B2 6FF9 74DB" + sudo add-apt-repository "deb [arch=amd64] https://dl.bintray.com/tetrate/getenvoy-deb $(lsb_release -cs) stable" + sudo apt update + sudo apt install -y getenvoy-envoy env: MATRIX_OS: ${{ matrix.os }} shell: bash diff --git a/CHANGELOG.md b/CHANGELOG.md index 50469da..3c5fc07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ newer of protobuf compiler plugin. * `Client.$createCall` is deprecated because it does not invoke client interceptors. +* Fix an issue [#380](https://github.com/grpc/grpc-dart/issues/380) causing + incorrect duplicated headers in gRPC-Web requests. * Change minimum required Dart SDK to 2.8 to enable access to Unix domain sockets. * Add support for Unix domain sockets in `Socket.serve` and `ClientChannel`. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b647aa2..16fc729 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -37,9 +37,9 @@ pub get pub run test ``` -gRPC-web tests require [`grpcwebproxy`]( -https://github.com/improbable-eng/grpc-web/tree/master/go/grpcwebproxy) by -Improbable Engineering to be available in the PATH. Pre-built binaries are [available](https://github.com/improbable-eng/grpc-web/releases). +gRPC-web tests require [`envoy`]( +https://www.envoyproxy.io/docs/envoy/latest/start/start.html) binary to be +available in the PATH. ## Guidelines for Pull Requests diff --git a/lib/src/client/transport/xhr_transport.dart b/lib/src/client/transport/xhr_transport.dart index 30ceabb..afb65be 100644 --- a/lib/src/client/transport/xhr_transport.dart +++ b/lib/src/client/transport/xhr_transport.dart @@ -169,9 +169,6 @@ class XhrClientConnection extends ClientConnection { for (final header in metadata.keys) { request.setRequestHeader(header, metadata[header]); } - request.setRequestHeader('Content-Type', 'application/grpc-web+proto'); - request.setRequestHeader('X-User-Agent', 'grpc-web-dart/0.1'); - request.setRequestHeader('X-Grpc-Web', '1'); // Overriding the mimetype allows us to stream and parse the data request.overrideMimeType('text/plain; charset=x-user-defined'); request.responseType = 'text'; diff --git a/test/grpc_web_server.dart b/test/grpc_web_server.dart index 451fd00..3106aac 100644 --- a/test/grpc_web_server.dart +++ b/test/grpc_web_server.dart @@ -4,6 +4,7 @@ import 'dart:convert'; import 'dart:io'; import 'package:grpc/grpc.dart'; +import 'package:path/path.dart' as p; import 'package:stream_channel/stream_channel.dart'; import 'src/generated/echo.pbgrpc.dart'; @@ -30,31 +31,71 @@ class EchoService extends EchoServiceBase { } } +final envoyPort = 9999; + +final envoyConfig = ''' +static_resources: + listeners: + - name: listener_0 + address: + socket_address: { address: 0.0.0.0, port_value: 0 } + filter_chains: + - filters: + - name: envoy.http_connection_manager + config: + codec_type: auto + stat_prefix: ingress_http + route_config: + name: local_route + virtual_hosts: + - name: local_service + domains: ["*"] + routes: + - match: { prefix: "/" } + route: { cluster: echo_service } + cors: + allow_origin_string_match: + - prefix: "*" + allow_methods: GET, PUT, DELETE, POST, OPTIONS + allow_headers: keep-alive,user-agent,cache-control,content-type,content-transfer-encoding,custom-header-1,x-accept-content-transfer-encoding,x-accept-response-streaming,x-user-agent,x-grpc-web,grpc-timeout + max_age: "1728000" + expose_headers: custom-header-1,grpc-status,grpc-message + http_filters: + - name: envoy.filters.http.grpc_web + - name: envoy.filters.http.cors + - name: envoy.filters.http.router + clusters: + - name: echo_service + connect_timeout: 0.25s + type: static + http2_protocol_options: {} + lb_policy: round_robin + hosts: + - socket_address: { address: 127.0.0.1, port_value: %TARGET_PORT% } +'''; + hybridMain(StreamChannel channel) async { // Spawn a gRPC server. final server = Server([EchoService()]); await server.serve(port: 0); _info('grpc server listening on ${server.port}'); + // Create Envoy configuration. + final tempDir = await Directory.systemTemp.createTemp(); + final config = p.join(tempDir.path, 'config.yaml'); + await File(config).writeAsString( + envoyConfig.replaceAll('%TARGET_PORT%', server.port.toString())); + // Spawn a proxy that would translate gRPC-web protocol into gRPC protocol - // for us. We use grpcwebproxy by Improbable Engineering. See CONTRIBUTING.md - // for setup. + // for us. We use envoy proxy. See CONTRIBUTING.md for setup. Process proxy; try { - proxy = - await Process.start('grpcwebproxy${Platform.isWindows ? '.exe' : ''}', [ - '--backend_addr', - 'localhost:${server.port}', - '--run_tls_server=false', - '--server_http_debug_port', - '0', - '--allow_all_origins', - ]); + proxy = await Process.start('envoy', ['-c', config, '-l', 'debug']); } catch (e) { print(''' -Failed to start grpcwebproxy: $e. +Failed to start envoy: $e. -Make sure that grpcwebproxy is available in the PATH see CONTRIBUTING.md +Make sure that envoy is available in the PATH see CONTRIBUTING.md if you are running tests locally. '''); channel.sink.add(0); @@ -62,28 +103,41 @@ if you are running tests locally. } // Parse output of the proxy process looking for a port it selected. - final portRe = RegExp(r'listening for http on: .*:(\d+)'); + final portRe = RegExp( + r'Set listener listener_0 socket factory local address to 0.0.0.0:(\d+)'); proxy.stderr .transform(utf8.decoder) .transform(const LineSplitter()) .listen((line) { - _info('grpcwebproxy|stderr] $line'); + _info('envoy|stderr] $line'); + final m = portRe.firstMatch(line); + if (m != null) { + channel.sink.add(int.parse(m[1])); + } }); proxy.stdout .transform(utf8.decoder) .transform(const LineSplitter()) .listen((line) { - _info('grpcwebproxy|stdout] $line'); - final m = portRe.firstMatch(line); - if (m != null) { - final port = int.parse(m[1]); - channel.sink.add(port); + _info('envoy|stdout] $line'); + }); + + proxy.exitCode.then((value) { + _info('proxy quit with ${value}'); + if (value != 0) { + channel.sink.addError('proxy exited with ${value}'); } }); - proxy.exitCode.then((value) => _info('proxy quit with ${value}')); + // Wait for the harness to tell us to shutdown. + await channel.stream.first; + proxy.kill(); + if (tempDir.existsSync()) { + tempDir.deleteSync(recursive: true); + } + channel.sink.add('EXITED'); } void _info(String line) { diff --git a/test/grpc_web_test.dart b/test/grpc_web_test.dart index 98765b5..604baf9 100644 --- a/test/grpc_web_test.dart +++ b/test/grpc_web_test.dart @@ -1,64 +1,100 @@ @TestOn('browser') +import 'dart:async'; import 'dart:math' as math; +import 'package:stream_channel/stream_channel.dart'; import 'package:test/test.dart'; import 'package:grpc/grpc_web.dart'; import 'src/generated/echo.pbgrpc.dart'; -/// Starts gRPC server and a gRPC-web proxy (see grpc_web_server.dart for -/// implementation. -/// -/// Returns uri which can be used to talk to using gRPC-web channel. -/// -/// Note: server will be shut down when the test which spawned it finishes -/// running. -Future startServer() async { - // Spawn the server code on the server side, it will send us back port - // number we should be talking to. - final serverChannel = spawnHybridUri('grpc_web_server.dart'); - final port = await serverChannel.stream.first; - - // Note: we would like to test https as well, but we can't easily do it - // because browsers like chrome don't trust self-signed certificates by - // default. - return Uri.parse('http://localhost:$port'); -} - void main() { // Test verifies that gRPC-web echo example works by talking to a gRPC // server (written in Dart) via gRPC-web protocol through a third party // gRPC-web proxy. test('gRPC-web echo test', () async { - final serverUri = await startServer(); - final channel = GrpcWebClientChannel.xhr(serverUri); - final service = EchoServiceClient(channel); + final server = await GrpcWebServer.start(); + try { + final channel = GrpcWebClientChannel.xhr(server.uri); + final service = EchoServiceClient(channel); - const testMessage = 'hello from gRPC-web'; + const testMessage = 'hello from gRPC-web'; - // First test a simple echo request. - final response = await service.echo(EchoRequest()..message = testMessage); - expect(response.message, equals(testMessage)); - - // Now test that streaming requests also works by asking echo server - // to send us a number of messages every 100 ms. Check that we receive - // them fast enough (if streaming is broken we will receive all of them - // in one go). - final sw = Stopwatch()..start(); - final timings = await service - .serverStreamingEcho(ServerStreamingEchoRequest() - ..message = testMessage - ..messageCount = 20 - ..messageInterval = 100) - .map((response) { + // First test a simple echo request. + final response = await service.echo(EchoRequest()..message = testMessage); expect(response.message, equals(testMessage)); - final timing = sw.elapsedMilliseconds; - sw.reset(); - return timing; - }).toList(); - final maxDelay = timings.reduce(math.max); - expect(maxDelay, lessThan(500)); + + // Now test that streaming requests also works by asking echo server + // to send us a number of messages every 100 ms. Check that we receive + // them fast enough (if streaming is broken we will receive all of them + // in one go). + final sw = Stopwatch()..start(); + final timings = await service + .serverStreamingEcho(ServerStreamingEchoRequest() + ..message = testMessage + ..messageCount = 20 + ..messageInterval = 100) + .map((response) { + expect(response.message, equals(testMessage)); + final timing = sw.elapsedMilliseconds; + sw.reset(); + return timing; + }).toList(); + final maxDelay = timings.reduce(math.max); + expect(maxDelay, lessThan(500)); + } finally { + await server.shutdown(); + } }); } + +class GrpcWebServer { + final StreamChannel channel; + final Future whenExited; + final Uri uri; + + GrpcWebServer(this.channel, this.whenExited, this.uri); + + Future shutdown() async { + channel.sink.add('shutdown'); + await whenExited; + } + + /// Starts gRPC server and a gRPC-web proxy (see grpc_web_server.dart for + /// implementation. + /// + /// Returns uri which can be used to talk to using gRPC-web channel. + /// + /// Note: you need to explicitly call shutdown on the returned object + /// otherwise envoy proxy process leaks. + static Future start() async { + // Spawn the server code on the server side, it will send us back port + // number we should be talking to. + final serverChannel = spawnHybridUri('grpc_web_server.dart'); + final portCompleter = Completer(); + final exitCompleter = Completer(); + serverChannel.stream.listen((event) { + if (!portCompleter.isCompleted) { + portCompleter.complete(event); + } else if (event == 'EXITED') { + exitCompleter.complete(); + } + }, onError: (e) { + if (!portCompleter.isCompleted) { + portCompleter.completeError(e); + } else if (!exitCompleter.isCompleted) { + exitCompleter.completeError(e); + } + }); + + final port = await portCompleter.future; + + // Note: we would like to test https as well, but we can't easily do it + // because browsers like chrome don't trust self-signed certificates by + // default. + return GrpcWebServer(serverChannel, exitCompleter.future, + Uri.parse('http://localhost:$port')); + } +} diff --git a/tool/install-grpcwebproxy.sh b/tool/install-grpcwebproxy.sh deleted file mode 100755 index be5a24d..0000000 --- a/tool/install-grpcwebproxy.sh +++ /dev/null @@ -1,44 +0,0 @@ -#!/bin/sh - -set -ex - -VERSION=v0.13.0 -SUFFIX= -WGET=wget - -case $TRAVIS_OS_NAME in - linux) - VARIANT=linux-x86_64 - ;; - osx) - VARIANT=osx-x86_64 - ;; - windows) - VARIANT=win64.exe - SUFFIX=.exe - ;; -esac - -case $MATRIX_OS in - ubuntu-latest) - VARIANT=linux-x86_64 - ;; - macos-latest) - VARIANT=osx-x86_64 - ;; - windows-latest) - VARIANT=win64.exe - SUFFIX=.exe - WGET=C:/msys64/usr/bin/wget.exe - ;; -esac - -BINARY=grpcwebproxy-${VERSION}-${VARIANT} - -${WGET} https://github.com/improbable-eng/grpc-web/releases/download/${VERSION}/${BINARY}.zip -O /tmp/grpcwebproxy.zip -rm -rf /tmp/grpcwebproxy -mkdir /tmp/grpcwebproxy -cd /tmp/grpcwebproxy -unzip /tmp/grpcwebproxy.zip -mv dist/${BINARY} ./grpcwebproxy${SUFFIX} -