HttpRequests — implement POST

We cannot replace all usages to HttpRequests due JDK bug (no way to read error stream on http error)
Netty can be used instead, but for now it is ok to wait when IDEA project will be migrated to JDK 9.

platform/platform-api/src/com/intellij/util/io/HttpRequests.java

@@ -93,8 +93,25 @@ public final class HttpRequests {
     @NotNull
     String readString(@Nullable ProgressIndicator indicator) throws IOException;
 
+    @NotNull
+    default String readString() throws IOException {
+      return readString(null);
+    }
+
     @NotNull
     CharSequence readChars(@Nullable ProgressIndicator indicator) throws IOException;
+
+    default void write(@NotNull String data) throws IOException {
+      write(data.getBytes(StandardCharsets.UTF_8));
+    }
+
+    default void write(@NotNull byte[] data) throws IOException {
+      HttpURLConnection connection = (HttpURLConnection)getConnection();
+      connection.setFixedLengthStreamingMode(data.length);
+      try (OutputStream stream = connection.getOutputStream()) {
+        stream.write(data);
+      }
+    }
   }
 
   public interface RequestProcessor<T> {
@@ -147,7 +164,19 @@ public final class HttpRequests {
 
   @NotNull
   public static RequestBuilder head(@NotNull String url) {
-    return new RequestBuilderImpl(url, "HEAD");
+    return new RequestBuilderImpl(url, connection -> ((HttpURLConnection)connection).setRequestMethod("HEAD"));
+  }
+
+  @NotNull
+  public static RequestBuilder post(@NotNull String url, @Nullable String contentType) {
+    return new RequestBuilderImpl(url, rawConnection -> {
+      HttpURLConnection connection = (HttpURLConnection)rawConnection;
+      connection.setRequestMethod("POST");
+      connection.setDoOutput(true);
+      if (contentType != null) {
+        connection.setRequestProperty("Content-Type", contentType);
+      }
+    });
   }
 
   @NotNull
@@ -166,15 +195,14 @@ public final class HttpRequests {
         builder.append("\n, response: ").append(httpConnection.getResponseCode()).append(' ').append(httpConnection.getResponseMessage());
       }
     }
-    catch (Throwable ignored) { }
+    catch (Throwable ignored) {
+    }
 
     return builder.toString();
   }
 
-
-  private static class RequestBuilderImpl extends RequestBuilder {
+  public static class RequestBuilderImpl extends RequestBuilder {
     private final String myUrl;
-    private final String myMethod;
     private int myConnectTimeout = HttpConfigurable.CONNECTION_TIMEOUT;
     private int myTimeout = HttpConfigurable.READ_TIMEOUT;
     private int myRedirectLimit = HttpConfigurable.REDIRECT_LIMIT;
@@ -185,11 +213,12 @@ public final class HttpRequests {
     private String myUserAgent;
     private String myAccept;
     private ConnectionTuner myTuner;
+    private final ConnectionTuner myInternalTuner;
     private UntrustedCertificateStrategy myUntrustedCertificateStrategy = null;
 
-    private RequestBuilderImpl(@NotNull String url, @Nullable String method) {
+    RequestBuilderImpl(@NotNull String url, @Nullable ConnectionTuner internalTuner) {
       myUrl = url;
-      myMethod = method;
+      myInternalTuner = internalTuner;
     }
 
     @Override
@@ -280,6 +309,7 @@ public final class HttpRequests {
 
   private static class RequestImpl implements Request, AutoCloseable {
     private final RequestBuilderImpl myBuilder;
+    @SuppressWarnings("FieldMayBeFinal")
     private String myUrl;
     private URLConnection myConnection;
     private InputStream myInputStream;
@@ -446,12 +476,26 @@ public final class HttpRequests {
   private static <T> T doProcess(RequestBuilderImpl builder, RequestProcessor<T> processor) throws IOException {
     CertificateManager manager = builder.myUntrustedCertificateStrategy == null || ApplicationManager.getApplication() == null ? null : CertificateManager.getInstance();
     try (RequestImpl request = new RequestImpl(builder)) {
+      T result;
       if (manager != null) {
-        return manager.runWithUntrustedCertificateStrategy(() -> processor.process(request), builder.myUntrustedCertificateStrategy);
+        result = manager.runWithUntrustedCertificateStrategy(() -> processor.process(request), builder.myUntrustedCertificateStrategy);
       }
       else {
-        return processor.process(request);
+        result = processor.process(request);
       }
+
+      URLConnection connection = request.myConnection;
+      if (connection instanceof HttpURLConnection && ((HttpURLConnection)connection).getRequestMethod().equals("POST")) {
+        // getResponseCode is not checked on connect for POST, because write must be performed before read
+        // https://stackoverflow.com/questions/613307/read-error-response-body-in-java
+        // the problem is that if you read the HttpUrlConnection.getErrorStream() code, you'll see that it ALWAYS returns nul
+        HttpURLConnection urlConnection = (HttpURLConnection)connection;
+        int responseCode = urlConnection.getResponseCode();
+        if (responseCode >= 400) {
+          throwHttpStatusError(request, responseCode);
+        }
+      }
+      return result;
     }
   }
 
@@ -474,9 +518,8 @@ public final class HttpRequests {
   }
 
   private static URLConnection openConnection(RequestBuilderImpl builder, RequestImpl request) throws IOException {
-    String url = request.myUrl;
-
     for (int i = 0; i < builder.myRedirectLimit; i++) {
+      String url = request.myUrl;
       if (builder.myForceHttps && StringUtil.startsWith(url, "http:")) {
         request.myUrl = url = "https:" + url.substring(5);
       }
@@ -493,25 +536,9 @@ public final class HttpRequests {
       }
 
       if (connection instanceof HttpsURLConnection) {
-        if (ApplicationManager.getApplication() != null) {
-          try {
-            final SSLContext context = CertificateManager.getInstance().getSslContext();
-            final SSLSocketFactory factory = context.getSocketFactory();
-            if (factory != null) {
-              ((HttpsURLConnection)connection).setSSLSocketFactory(factory);
-            }
-            else {
-              LOG.info("SSLSocketFactory is not defined by IDE CertificateManager; Using default SSL configuration to connect to " + url);
-            }
-          }
-          catch (Throwable e) {
-            LOG.info("Problems configuring SSL connection to " + url , e);
-          }
-        }
-        else {
-          LOG.info("Application is not initialized yet; Using default SSL configuration to connect to " + url);
-        }
+        configureSslConnection(url, (HttpsURLConnection)connection);
       }
+
       connection.setConnectTimeout(builder.myConnectTimeout);
       connection.setReadTimeout(builder.myTimeout);
 
@@ -533,8 +560,8 @@ public final class HttpRequests {
 
       connection.setUseCaches(false);
 
-      if (builder.myMethod != null) {
-        ((HttpURLConnection)connection).setRequestMethod(builder.myMethod);
+      if (builder.myInternalTuner != null) {
+        builder.myInternalTuner.tune(connection);
       }
 
       if (builder.myTuner != null) {
@@ -543,33 +570,38 @@ public final class HttpRequests {
 
       checkRequestHeadersForNulBytes(connection);
 
-      if (connection instanceof HttpURLConnection) {
-        HttpURLConnection httpURLConnection = (HttpURLConnection)connection;
-        String method = httpURLConnection.getRequestMethod();
-        LOG.assertTrue(method.equals("GET") || method.equals("HEAD"), "'" + method + "' not supported; please use GET or HEAD");
+      if (!(connection instanceof HttpURLConnection)) {
+        return connection;
+      }
 
-        if (LOG.isDebugEnabled()) LOG.debug("connecting to " + url);
-        int responseCode = httpURLConnection.getResponseCode();
-        if (LOG.isDebugEnabled()) LOG.debug("response: " + responseCode);
+      HttpURLConnection httpURLConnection = (HttpURLConnection)connection;
+      String method = httpURLConnection.getRequestMethod();
+      if (method.equals("POST")) {
+        return connection;
+      }
 
-        if (responseCode < 200 || responseCode >= 300 && responseCode != HttpResponseStatus.NOT_MODIFIED.code()) {
-          httpURLConnection.disconnect();
+      LOG.assertTrue(method.equals("GET") || method.equals("HEAD"), "'" + method + "' not supported; please use GET, HEAD or POST");
 
-          int idx = ArrayUtil.indexOf(REDIRECTS, responseCode);
-          if (idx >= 0) {
-            url = connection.getHeaderField("Location");
-            if (url != null) {
-              if (idx >= PERMANENT_IDX) {
-                LOG.error("HTTP response " + responseCode + " for '" + request.myUrl + "'; should be updated to '" + url + "'");
-              }
-              request.myUrl = url;
-              continue;
+      if (LOG.isDebugEnabled()) LOG.debug("connecting to " + url);
+      int responseCode = httpURLConnection.getResponseCode();
+      if (LOG.isDebugEnabled()) LOG.debug("response: " + responseCode);
+
+      if (responseCode < 200 || responseCode >= 300 && responseCode != HttpURLConnection.HTTP_NOT_MODIFIED) {
+        httpURLConnection.disconnect();
+
+        int idx = ArrayUtil.indexOf(REDIRECTS, responseCode);
+        if (idx >= 0) {
+          url = connection.getHeaderField("Location");
+          if (url != null) {
+            if (idx >= PERMANENT_IDX) {
+              LOG.error("HTTP response " + responseCode + " for '" + request.myUrl + "'; should be updated to '" + url + "'");
             }
+            request.myUrl = url;
+            continue;
           }
-
-          String message = IdeBundle.message("error.connection.failed.with.http.code.N", responseCode);
-          throw new HttpStatusException(message, responseCode, StringUtil.notNullize(url, "Empty URL"));
         }
+
+        return throwHttpStatusError(request, responseCode);
       }
 
       return connection;
@@ -578,6 +610,32 @@ public final class HttpRequests {
     throw new IOException(IdeBundle.message("error.connection.failed.redirects"));
   }
 
+  private static URLConnection throwHttpStatusError(@NotNull RequestImpl request, int responseCode) throws IOException {
+    String message = IdeBundle.message("error.connection.failed.with.http.code.N", responseCode);
+    throw new HttpStatusException(message, responseCode, StringUtil.notNullize(request.myUrl, "Empty URL"));
+  }
+
+  private static void configureSslConnection(@NotNull String url, @NotNull HttpsURLConnection connection) {
+    if (ApplicationManager.getApplication() == null) {
+      LOG.info("Application is not initialized yet; Using default SSL configuration to connect to " + url);
+      return;
+    }
+
+    try {
+      final SSLContext context = CertificateManager.getInstance().getSslContext();
+      final SSLSocketFactory factory = context.getSocketFactory();
+      if (factory == null) {
+        LOG.info("SSLSocketFactory is not defined by IDE CertificateManager; Using default SSL configuration to connect to " + url);
+      }
+      else {
+        connection.setSSLSocketFactory(factory);
+      }
+    }
+    catch (Throwable e) {
+      LOG.info("Problems configuring SSL connection to " + url, e);
+    }
+  }
+
   /**
    * A lot of web servers would not process request and just return 400 (Bad Request) response if any of request headers contains NUL byte.
    * This method checks if any headers contain NUL byte in value and removes those headers from request.

platform/platform-api/src/com/intellij/util/io/RequestBuilder.java

@@ -80,4 +80,18 @@ public abstract class RequestBuilder {
   public CharSequence readChars() throws IOException {
     return readChars(null);
   }
+
+  public void write(@NotNull String data) throws IOException {
+    connect(request -> {
+      request.write(data);
+      return null;
+    });
+  }
+
+  public void write(@NotNull byte[] data) throws IOException {
+    connect(request -> {
+      request.write(data);
+      return null;
+    });
+  }
 }

platform/platform-impl/src/com/intellij/reporting/Reporter.kt

@@ -5,23 +5,18 @@ import com.google.common.net.HttpHeaders
 import com.google.gson.Gson
 import com.intellij.openapi.application.PermanentInstallationID
 import com.intellij.openapi.diagnostic.Logger
+import com.intellij.util.io.HttpRequests
 import org.apache.commons.codec.binary.Base64OutputStream
-import org.apache.http.client.fluent.Request
-import org.apache.http.entity.ContentType
-import org.apache.http.message.BasicHeader
 import java.io.ByteArrayOutputStream
 import java.util.zip.GZIPOutputStream
 
-
 private class StatsServerInfo(@JvmField var status: String,
                               @JvmField var url: String,
                               @JvmField var urlForZipBase64Content: String) {
   fun isServiceAlive() = "ok" == status
 }
 
-private object Utils {
-  val gson = Gson()
-}
+private val gson by lazy { Gson() }
 
 object StatsSender {
   private const val infoUrl = "https://www.jetbrains.com/config/features-service-status.json"
@@ -29,8 +24,7 @@ object StatsSender {
 
   private fun requestServerUrl(): StatsServerInfo? {
     try {
-      val response = Request.Get(infoUrl).execute().returnContent().asString()
-      val info = Utils.gson.fromJson(response, StatsServerInfo::class.java)
+      val info = gson.fromJson(HttpRequests.request(infoUrl).readString(), StatsServerInfo::class.java)
       if (info.isServiceAlive()) return info
     }
     catch (e: Exception) {
@@ -43,11 +37,8 @@ object StatsSender {
   fun send(text: String, compress: Boolean = true): Boolean {
     val info = requestServerUrl() ?: return false
     try {
-      val response = createRequest(info, text, compress).execute()
-      val code = response.handleResponse { it.statusLine.statusCode }
-      if (code in 200..299) {
-        return true
-      }
+      executeRequest(info, text, compress)
+      return true
     }
     catch (e: Exception) {
       LOG.debug(e)
@@ -55,15 +46,17 @@ object StatsSender {
     return false
   }
 
-  private fun createRequest(info: StatsServerInfo, text: String, compress: Boolean): Request {
+  private fun executeRequest(info: StatsServerInfo, text: String, compress: Boolean) {
     if (compress) {
       val data = Base64GzipCompressor.compress(text)
-      val request = Request.Post(info.urlForZipBase64Content).bodyByteArray(data)
-      request.addHeader(BasicHeader(HttpHeaders.CONTENT_ENCODING, "gzip"))
-      return request
+      HttpRequests
+        .post(info.urlForZipBase64Content, null)
+        .tuner { it.setRequestProperty(HttpHeaders.CONTENT_ENCODING, "gzip") }
+        .write(data)
+      return
     }
 
-    return Request.Post(info.url).bodyString(text, ContentType.TEXT_HTML)
+    HttpRequests.post(info.url, "text/html").write(text)
   }
 }
 
@@ -78,7 +71,7 @@ private object Base64GzipCompressor {
 }
 
 fun <T> createReportLine(recorderId: String, sessionId: String, data: T): String {
-  val json = Utils.gson.toJson(data)
+  val json = gson.toJson(data)
   val userUid = PermanentInstallationID.get()
   val stamp = System.currentTimeMillis()
   return "$stamp\t$recorderId\t$userUid\t$sessionId\t$json"

platform/platform-tests/testSrc/com/intellij/util/io/HttpRequestsTest.java

@@ -2,6 +2,8 @@
 package com.intellij.util.io;
 
 import com.intellij.ide.IdeBundle;
+import com.intellij.openapi.util.Ref;
+import com.intellij.openapi.util.io.StreamUtil;
 import com.intellij.util.TimeoutUtil;
 import com.sun.net.httpserver.HttpServer;
 import org.junit.After;
@@ -12,9 +14,11 @@ import java.io.IOException;
 import java.net.HttpURLConnection;
 import java.net.InetSocketAddress;
 import java.net.SocketTimeoutException;
+import java.nio.charset.StandardCharsets;
 
 import static java.net.HttpURLConnection.*;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.catchThrowable;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
 
@@ -101,14 +105,52 @@ public class HttpRequestsTest {
     fail();
   }
 
-  @Test(expected = AssertionError.class)
-  public void testPostNotAllowed() throws IOException {
-    HttpRequests.request(myUrl)
-                .tuner((c) -> ((HttpURLConnection)c).setRequestMethod("POST"))
-                .tryConnect();
-    fail();
+  @Test
+  public void post() throws IOException {
+    Ref<String> receivedData = Ref.create();
+    myServer.createContext("/", ex -> {
+      receivedData.set(StreamUtil.readText(ex.getRequestBody(), StandardCharsets.UTF_8));
+      ex.sendResponseHeaders(HTTP_OK, -1);
+      ex.close();
+    });
+
+    HttpRequests.post(myUrl, null).write("hello");
+    assertThat(receivedData.get()).isEqualTo("hello");
   }
 
+  @Test
+  public void postNotFound() {
+    myServer.createContext("/", ex -> {
+      ex.sendResponseHeaders(HTTP_NOT_FOUND, -1);
+      ex.close();
+    });
+
+    assertThat(catchThrowable(() -> HttpRequests.post(myUrl, null).write("hello")))
+      .isInstanceOf(HttpRequests.HttpStatusException.class);
+  }
+
+  //@Test
+  //public void postNotFoundWithResponse() throws IOException {
+  //  String serverErrorText = "use another url";
+  //  myServer.createContext("/", ex -> {
+  //    ex.sendResponseHeaders(503, -1);
+  //    ex.getResponseBody().write(serverErrorText.getBytes(StandardCharsets.UTF_8));
+  //    ex.close();
+  //  });
+  //
+  //  try {
+  //    HttpRequests
+  //      .post(myUrl, null)
+  //      .isReadResponseOnError(true)
+  //      .write("hello");
+  //  }
+  //  catch (HttpRequests.HttpStatusException e) {
+  //    assertThat(e.getMessage()).isEqualTo(serverErrorText);
+  //  }
+  //
+  //  fail();
+  //}
+
   @Test(timeout = 5000)
   public void testNotModified() throws IOException {
     myServer.createContext("/", ex -> {