Merge pull request square#1 from square/master

merge

CHANGELOG.md

@@ -5,7 +5,7 @@ Change Log
 
 _2016-07-10_
 
- *  **Fix an major bug in encoding HTTP headers.** In 3.4.0 and 3.4.0-RC1 OkHttp
+ *  **Fix a major bug in encoding HTTP headers.** In 3.4.0 and 3.4.0-RC1 OkHttp
     had an off-by-one bug in our HPACK encoder. This bug could have caused the
     wrong headers to be emitted after a sequence of HTTP/2 requests! Everyone
     who is using OkHttp 3.4.0 or 3.4.0-RC1 should upgrade for this bug fix.

checkstyle.xml

@@ -4,6 +4,7 @@
     "http://www.puppycrawl.com/dtds/configuration_1_2.dtd">
 
 <module name="Checker">
+  <module name="SuppressWarningsFilter"/>
   <module name="NewlineAtEndOfFile"/>
   <module name="FileLength"/>
   <module name="FileTabCharacter"/>
@@ -139,5 +140,8 @@
     <!--module name="FinalParameters"/-->
     <!--module name="TodoComment"/-->
     <module name="UpperEll"/>
+
+    <!-- Make the @SuppressWarnings annotations available to Checkstyle -->
+    <module name="SuppressWarningsHolder"/>
   </module>
 </module>

mockwebserver/pom.xml

@@ -24,11 +24,6 @@
       <version>${project.version}</version>
       <scope>test</scope>
     </dependency>
-    <dependency>
-      <groupId>${project.groupId}</groupId>
-      <artifactId>okhttp-ws</artifactId>
-      <version>${project.version}</version>
-    </dependency>
     <dependency>
       <groupId>org.bouncycastle</groupId>
       <artifactId>bcprov-jdk15on</artifactId>

mockwebserver/src/main/java/okhttp3/internal/tls/SslClient.java

@@ -85,12 +85,12 @@ public static class Builder {
      * the server's certificate, further certificates are included in the handshake so the client
      * can build a trusted path to a CA certificate.
      */
-    public Builder certificateChain(HeldCertificate serverCert, HeldCertificate... chain) {
+    public Builder certificateChain(HeldCertificate localCert, HeldCertificate... chain) {
       X509Certificate[] certificates = new X509Certificate[chain.length];
       for (int i = 0; i < chain.length; i++) {
         certificates[i] = chain[i].certificate;
       }
-      return certificateChain(serverCert.keyPair, serverCert.certificate, certificates);
+      return certificateChain(localCert.keyPair, localCert.certificate, certificates);
     }
 
     public Builder certificateChain(KeyPair keyPair, X509Certificate keyCert,

mockwebserver/src/main/java/okhttp3/mockwebserver/MockResponse.java

@@ -21,7 +21,7 @@
 import okhttp3.Headers;
 import okhttp3.internal.Internal;
 import okhttp3.internal.http2.Settings;
-import okhttp3.ws.WebSocketListener;
+import okhttp3.WebSocketListener;
 import okio.Buffer;
 
 /** A scripted response to be replayed by the mock web server. */

mockwebserver/src/main/java/okhttp3/mockwebserver/MockWebServer.java

@@ -17,6 +17,7 @@
 
 package okhttp3.mockwebserver;
 
+import java.io.Closeable;
 import java.io.IOException;
 import java.io.InterruptedIOException;
 import java.net.InetAddress;
@@ -70,7 +71,6 @@
 import okhttp3.internal.platform.Platform;
 import okhttp3.internal.ws.RealWebSocket;
 import okhttp3.internal.ws.WebSocketProtocol;
-import okhttp3.ws.WebSocketListener;
 import okio.Buffer;
 import okio.BufferedSink;
 import okio.BufferedSource;
@@ -99,7 +99,7 @@
  * A scriptable web server. Callers supply canned responses and the server replays them upon request
  * in sequence.
  */
-public final class MockWebServer implements TestRule {
+public final class MockWebServer implements TestRule, Closeable {
   static {
     Internal.initializeInstanceForTests();
   }
@@ -655,22 +655,6 @@ private void handleWebSocketUpgrade(Socket socket, BufferedSource source, Buffer
 
     writeHttpResponse(socket, sink, response);
 
-    final WebSocketListener listener = response.getWebSocketListener();
-    final CountDownLatch connectionClose = new CountDownLatch(1);
-
-    ThreadPoolExecutor replyExecutor =
-        new ThreadPoolExecutor(1, 1, 1, SECONDS, new LinkedBlockingDeque<Runnable>(),
-            Util.threadFactory(Util.format("MockWebServer %s WebSocket", request.getPath()),
-                true));
-    replyExecutor.allowCoreThreadTimeOut(true);
-    final RealWebSocket webSocket =
-        new RealWebSocket(false /* is server */, source, sink, new SecureRandom(), replyExecutor,
-            listener, request.getPath()) {
-          @Override protected void close() throws IOException {
-            connectionClose.countDown();
-          }
-        };
-
     // Adapt the request and response into our Request and Response domain model.
     String scheme = request.getTlsVersion() != null ? "https" : "http";
     String authority = request.getHeader("Host"); // Has host and port.
@@ -686,16 +670,27 @@ private void handleWebSocketUpgrade(Socket socket, BufferedSource source, Buffer
         .protocol(Protocol.HTTP_1_1)
         .build();
 
-    listener.onOpen(webSocket, fancyResponse);
+    String name = request.getPath();
+    ThreadPoolExecutor replyExecutor =
+        new ThreadPoolExecutor(1, 1, 1, SECONDS, new LinkedBlockingDeque<Runnable>(),
+            Util.threadFactory(Util.format("MockWebServer %s WebSocket Replier", name), true));
+    replyExecutor.allowCoreThreadTimeOut(true);
+
+    final CountDownLatch connectionClose = new CountDownLatch(1);
+    RealWebSocket webSocket =
+        new RealWebSocket(false /* is server */, source, sink, new SecureRandom(), replyExecutor,
+            response.getWebSocketListener(), fancyResponse, name) {
+          @Override protected void shutdown() {
+            connectionClose.countDown();
+          }
+        };
 
-    while (webSocket.readMessage()) {
-    }
+    webSocket.loopReader();
 
     // Even if messages are no longer being read we need to wait for the connection close signal.
     try {
       connectionClose.await();
-    } catch (InterruptedException e) {
-      throw new RuntimeException(e);
+    } catch (InterruptedException ignored) {
     }
 
     replyExecutor.shutdown();
@@ -807,6 +802,10 @@ public void setDispatcher(Dispatcher dispatcher) {
     return "MockWebServer[" + port + "]";
   }
 
+  @Override public void close() throws IOException {
+    shutdown();
+  }
+
   /** A buffer wrapper that drops data after {@code bodyLimit} bytes. */
   private static class TruncatingBuffer implements Sink {
     private final Buffer buffer = new Buffer();
@@ -878,6 +877,11 @@ private FramedSocketHandler(Socket socket, Protocol protocol) {
         logger.info(MockWebServer.this + " received request: " + request
             + " and responded: " + response + " protocol is " + protocol.toString());
       }
+
+      if (response.getSocketPolicy() == DISCONNECT_AT_END) {
+        Http2Connection connection = stream.getConnection();
+        connection.shutdown(ErrorCode.NO_ERROR);
+      }
     }
 
     private RecordedRequest readRequest(Http2Stream stream) throws IOException {

mockwebserver/src/main/java/okhttp3/mockwebserver/SocketPolicy.java

@@ -37,7 +37,10 @@ public enum SocketPolicy {
   KEEP_OPEN,
 
   /**
-   * Close the socket after the response. This is the default HTTP/1.0 behavior.
+   * Close the socket after the response. This is the default HTTP/1.0 behavior. For HTTP/2
+   * connections, this sends a <a href="https://tools.ietf.org/html/rfc7540#section-6.8">GOAWAY
+   * frame</a> immediately after the response and will close the connection when the client's socket
+   * is exhausted.
    *
    * <p>See {@link SocketPolicy} for reasons why this can cause test flakiness and how to avoid it.
    */

mockwebserver/src/test/java/okhttp3/mockwebserver/MockWebServerTest.java

@@ -16,6 +16,7 @@
 package okhttp3.mockwebserver;
 
 import java.io.BufferedReader;
+import java.io.Closeable;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
@@ -339,6 +340,11 @@ private List<String> headersToList(MockResponse response) {
     server.shutdown();
   }
 
+  @Test public void closeViaClosable() throws IOException {
+    Closeable server = new MockWebServer();
+    server.close();
+  }
+
   @Test public void shutdownWithoutEnqueue() throws IOException {
     MockWebServer server = new MockWebServer();
     server.start();

okhttp-android-support/src/main/java/okhttp3/internal/huc/JavaApiConverter.java

@@ -58,8 +58,6 @@
  * Helper methods that convert between Java and OkHttp representations.
  */
 public final class JavaApiConverter {
-  private static final RequestBody EMPTY_REQUEST_BODY = RequestBody.create(null, new byte[0]);
-
   /** Synthetic response header: the local time when the request was sent. */
   private static final String SENT_MILLIS = Platform.get().getPrefix() + "-Sent-Millis";
 
@@ -92,7 +90,7 @@ public static Response createOkResponseForCachePut(URI uri, URLConnection urlCon
     // OkHttp's Call API requires a placeholder body; the real body will be streamed separately.
     String requestMethod = httpUrlConnection.getRequestMethod();
     RequestBody placeholderBody = HttpMethod.requiresRequestBody(requestMethod)
-        ? EMPTY_REQUEST_BODY
+        ? Util.EMPTY_REQUEST
         : null;
 
     Request okRequest = new Request.Builder()
@@ -280,7 +278,7 @@ public static Request createOkRequest(
       URI uri, String requestMethod, Map<String, List<String>> requestHeaders) {
     // OkHttp's Call API requires a placeholder body; the real body will be streamed separately.
     RequestBody placeholderBody = HttpMethod.requiresRequestBody(requestMethod)
-        ? EMPTY_REQUEST_BODY
+        ? Util.EMPTY_REQUEST
         : null;
 
     Request.Builder builder = new Request.Builder()

okhttp-apache/src/main/java/okhttp3/apache/OkApacheClient.java

@@ -10,6 +10,7 @@
 import okhttp3.RequestBody;
 import okhttp3.Response;
 import okhttp3.ResponseBody;
+import okhttp3.internal.Util;
 import org.apache.http.Header;
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpEntityEnclosingRequest;
@@ -66,7 +67,7 @@ private static Request transformRequest(HttpRequest request) {
           builder.header(encoding.getName(), encoding.getValue());
         }
       } else {
-        body = RequestBody.create(null, new byte[0]);
+        body = Util.EMPTY_REQUEST;
       }
     }
     builder.method(method, body);

okhttp-ws-tests/fuzzingserver-test.sh → okhttp-tests/fuzzingserver-test.sh

@@ -21,7 +21,7 @@ set -ex
 wstest -m fuzzingserver -s fuzzingserver-config.json &
 sleep 2 # wait for wstest to start
 
-java -jar target/okhttp-ws-tests-*-jar-with-dependencies.jar
+java -jar target/okhttp-tests-*-jar-with-dependencies.jar
 
 jq '.[] as $in | $in | keys[] | . + " " + $in[.].behavior' target/fuzzingserver-report/index.json > target/fuzzingserver-actual.txt
 

okhttp-tests/pom.xml

@@ -49,6 +49,28 @@
 
   <build>
     <plugins>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-assembly-plugin</artifactId>
+        <configuration>
+          <descriptorRefs>
+            <descriptorRef>jar-with-dependencies</descriptorRef>
+          </descriptorRefs>
+          <archive>
+            <manifest>
+              <mainClass>okhttp3.AutobahnTester</mainClass>
+            </manifest>
+          </archive>
+        </configuration>
+        <executions>
+          <execution>
+            <phase>package</phase>
+            <goals>
+              <goal>single</goal>
+            </goals>
+          </execution>
+        </executions>
+      </plugin>
       <!-- Do not deploy this as an artifact to Maven central. -->
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>