Add support for Ubuntu 16.04's packaged nghttp2. (#29)

Motivation:

We'd done all our development against newer copies of nghttp2, that
behaved a bit differently. We should support Ubuntu's copy, at least
while nghttp2 is relevant to us.

Modifications:

- Shimmed over a version difference.
- Fixed the tests to stop relying on differences.
- Removed the Swift 4.0.3 dockerfiles as we don't support Swift
  4.0 anyway.

Result:

Users can build against Ubuntu 16.04's nghttp2 package.

Sources/CNIONghttp2/include/c_nio_nghttp2.h

@@ -11,9 +11,21 @@
 // SPDX-License-Identifier: Apache-2.0
 //
 //===----------------------------------------------------------------------===//
-#ifndef C_NIO_NGHTTP2_H
-#define C_NIO_NGHTTP2_H
+#ifndef CNIONGHTTP2_H
+#define CNIONGHTTP2_H
 
 #include <nghttp2/nghttp2.h>
 
+// There are some shims we need to provide here.
+typedef int (*CNIONghttp2_nghttp2_error_callback)(nghttp2_session *session,
+                                                  const char *msg,
+                                                  size_t len,
+                                                  void *user_data);
+
+void CNIONghttp2_nghttp2_session_callbacks_set_error_callback(
+    nghttp2_session_callbacks *cbs,
+    CNIONghttp2_nghttp2_error_callback error_callback);
+
+int CNIONghttp2_nghttp2_version_number(void);
+
 #endif

Sources/CNIONghttp2/shims.c

@@ -0,0 +1,32 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the SwiftNIO open source project
+//
+// Copyright (c) 2017-2018 Apple Inc. and the SwiftNIO project authors
+// Licensed under Apache License v2.0
+//
+// See LICENSE.txt for license information
+// See CONTRIBUTORS.txt for the list of SwiftNIO project authors
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+//===----------------------------------------------------------------------===//
+#include <c_nio_nghttp2.h>
+
+// This file provides an implementation of a number of shim functions to
+// handle different versions of nghttp2.
+
+// This shim only works on recent versions of nghttp2, otherwise it does
+// nothing.
+void CNIONghttp2_nghttp2_session_callbacks_set_error_callback(
+    nghttp2_session_callbacks *cbs,
+    CNIONghttp2_nghttp2_error_callback error_callback) {
+#if NGHTTP2_VERSION_NUM >= 0x010900
+    return nghttp2_session_callbacks_set_error_callback(cbs, error_callback);
+#endif
+}
+
+// This shim turns the macro into something we can see.
+int CNIONghttp2_nghttp2_version_number(void) {
+    return NGHTTP2_VERSION_NUM;
+}

Sources/NIOHTTP2/NGHTTP2Session.swift

@@ -39,7 +39,7 @@ private func withCallbacks<T>(fn: (OpaquePointer) throws -> T) rethrows -> T {
         nghttp2_session_callbacks_del(nghttp2Callbacks)
     }
 
-    nghttp2_session_callbacks_set_error_callback(nghttp2Callbacks, errorCallback)
+    CNIONghttp2_nghttp2_session_callbacks_set_error_callback(nghttp2Callbacks, errorCallback)
     nghttp2_session_callbacks_set_on_frame_recv_callback(nghttp2Callbacks, onFrameRecvCallback)
     nghttp2_session_callbacks_set_on_begin_frame_callback(nghttp2Callbacks, onBeginFrameCallback)
     nghttp2_session_callbacks_set_on_data_chunk_recv_callback(nghttp2Callbacks, onDataChunkRecvCallback)

Tests/NIOHTTP2Tests/SimpleClientServerTests.swift

@@ -15,6 +15,7 @@
 import XCTest
 import NIO
 import NIOHTTP1
+import CNIONghttp2
 @testable import NIOHTTP2
 
 /// A channel handler that passes writes through but fires EOF once the first one hits.
@@ -275,9 +276,8 @@ class SimpleClientServerTests: XCTestCase {
         self.serverChannel.writeAndFlush(goAwayFrame, promise: nil)
         self.interactInMemory(self.clientChannel, self.serverChannel)
 
-        // The client should not receive this GOAWAY frame, as it has shut down.
-        self.clientChannel.assertNoFramesReceived()
-
+        // In some nghttp2 versions the client will receive a GOAWAY, in others
+        // it will not. There is no meaningful assertion to apply here.
         // All should be good.
         self.serverChannel.assertNoFramesReceived()
         XCTAssertNoThrow(try self.clientChannel.finish())
@@ -969,9 +969,8 @@ class SimpleClientServerTests: XCTestCase {
         // The data frame write should have exploded. nghttp2 synthesises an error code for this.
         XCTAssertEqual((writeError as? NIOHTTP2Errors.StreamClosed)?.streamID, clientStreamID)
 
-        // No other frames should be emitted.
-        self.clientChannel.assertNoFramesReceived()
-        self.serverChannel.assertNoFramesReceived()
+        // No other frames should be emitted, but we're ok if they were: depending on performance
+        // we may see a WINDOW_UPDATE or two here depending on timings.
         XCTAssertNoThrow(try self.clientChannel.finish())
         XCTAssertNoThrow(try self.serverChannel.finish())
     }
@@ -1000,7 +999,18 @@ class SimpleClientServerTests: XCTestCase {
         let respFrame = HTTP2Frame(streamID: serverStreamID, payload: .headers(responseHeaders))
         var respTrailersFrame = HTTP2Frame(streamID: serverStreamID, payload: .headers(trailers))
         respTrailersFrame.flags.insert(.endStream)
-        try self.assertFramesRoundTrip(frames: [respFrame, respTrailersFrame], sender: self.serverChannel, receiver: self.clientChannel)
+
+        // v1.11.0 onwards don't send a 0-length DATA frame here, but earlier ones do. We send it explicitly to get
+        // the output to match on all platforms.
+        let expectedFrames: [HTTP2Frame]
+        if CNIONghttp2_nghttp2_version_number() < 0x011100 {
+            let emptyDataFrame = HTTP2Frame(streamID: serverStreamID, payload: .data(.byteBuffer(self.serverChannel.allocator.buffer(capacity: 0))))
+            expectedFrames = [respFrame, emptyDataFrame, respTrailersFrame]
+        } else {
+            expectedFrames = [respFrame, respTrailersFrame]
+        }
+
+        try self.assertFramesRoundTrip(frames: expectedFrames, sender: self.serverChannel, receiver: self.clientChannel)
 
         XCTAssertNoThrow(try self.clientChannel.finish())
         XCTAssertNoThrow(try self.serverChannel.finish())
@@ -1026,16 +1036,30 @@ class SimpleClientServerTests: XCTestCase {
 
         let serverStreamID = try self.assertFramesRoundTrip(frames: [reqFrame, reqBodyFrame], sender: self.clientChannel, receiver: self.serverChannel).first!.streamID
 
-        // Now we can send the next trailers.
-        XCTAssertNoThrow(try self.assertFramesRoundTrip(frames: [trailerFrame], sender: self.clientChannel, receiver: self.serverChannel))
+        // Now we can send the next trailers. Again, old versions of nghttp2 send an empty data frame here too.
+        var expectedFrames: [HTTP2Frame]
+        if CNIONghttp2_nghttp2_version_number() < 0x011100 {
+            let emptyDataFrame = HTTP2Frame(streamID: clientStreamID, payload: .data(.byteBuffer(self.clientChannel.allocator.buffer(capacity: 0))))
+            expectedFrames = [emptyDataFrame, trailerFrame]
+        } else {
+            expectedFrames = [trailerFrame]
+        }
+        XCTAssertNoThrow(try self.assertFramesRoundTrip(frames: expectedFrames, sender: self.clientChannel, receiver: self.serverChannel))
 
         // Let's send a quick response back. This response should also contain trailers.
         let responseHeaders = HTTPHeaders([(":status", "200"), ("content-length", "0")])
         let respFrame = HTTP2Frame(streamID: serverStreamID, payload: .headers(responseHeaders))
         var respTrailersFrame = HTTP2Frame(streamID: serverStreamID, payload: .headers(trailers))
         respTrailersFrame.flags.insert(.endStream)
         XCTAssertNoThrow(try self.assertFramesRoundTrip(frames: [respFrame], sender: self.serverChannel, receiver: self.clientChannel))
-        XCTAssertNoThrow(try self.assertFramesRoundTrip(frames: [respTrailersFrame], sender: self.serverChannel, receiver: self.clientChannel))
+
+        if CNIONghttp2_nghttp2_version_number() < 0x011100 {
+            let emptyDataFrame = HTTP2Frame(streamID: serverStreamID, payload: .data(.byteBuffer(self.serverChannel.allocator.buffer(capacity: 0))))
+            expectedFrames = [emptyDataFrame, respTrailersFrame]
+        } else {
+            expectedFrames = [respTrailersFrame]
+        }
+        XCTAssertNoThrow(try self.assertFramesRoundTrip(frames: expectedFrames, sender: self.serverChannel, receiver: self.clientChannel))
 
         XCTAssertNoThrow(try self.clientChannel.finish())
         XCTAssertNoThrow(try self.serverChannel.finish())

docker/docker-compose.ubuntu-1604.41.yaml

@@ -8,7 +8,6 @@ services:
       args:
         ubuntu_version : "16.04"
         swift_version : "4.1"
-        install_nghttp2_from_source: "true"
 
   test:
     image: swift-nio-http2:ubuntu-16.04-4.1