Make Request.toCurl work more like Chrome's 'copy as cURL' (#9112)

* Make Request.toCurl work more like Chrome's 'copy as cURL'

Changes:
 - single quotes
 - URL first
 - omit the default method

Also change binary detection to use isProbablyUtf8
instead, which is promoted to the main okhttp module.

Also replace the BinaryMode enum with a boolean to
optionally not include the body.

* More aggressive character escaping

---------

Co-authored-by: Jesse Wilson <[email protected]>

Author: swankjesse

okhttp-logging-interceptor/src/main/kotlin/okhttp3/logging/HttpLoggingInterceptor.kt

@@ -35,8 +35,8 @@ import okhttp3.ResponseBody
 import okhttp3.internal.charsetOrUtf8
 import okhttp3.internal.connection.RealCall
 import okhttp3.internal.http.promisesBody
+import okhttp3.internal.isProbablyUtf8
 import okhttp3.internal.platform.Platform
-import okhttp3.logging.internal.isProbablyUtf8
 import okio.Buffer
 import okio.BufferedSink
 import okio.BufferedSource
@@ -336,7 +336,7 @@ class HttpLoggingInterceptor
           val charset: Charset = requestBody.contentType().charsetOrUtf8()
 
           logger.log("")
-          if (!buffer.isProbablyUtf8()) {
+          if (!buffer.isProbablyUtf8(16L)) {
             logger.log(
               "--> END ${request.method} (binary ${requestBody.contentLength()}-byte body omitted)",
             )
@@ -403,7 +403,7 @@ class HttpLoggingInterceptor
 
           val charset: Charset = responseBody.contentType().charsetOrUtf8()
 
-          if (!buffer.isProbablyUtf8()) {
+          if (!buffer.isProbablyUtf8(16L)) {
             logger.log("")
             logger.log("<-- END HTTP (${totalMs}ms, binary ${buffer.size}-byte body omitted)")
             return response

okhttp-logging-interceptor/src/test/java/okhttp3/logging/IsProbablyUtf8Test.kt

@@ -37,16 +37,6 @@ public abstract interface class okhttp3/Authenticator {
 public final class okhttp3/Authenticator$Companion {
 }
 
-public final class okhttp3/BinaryMode : java/lang/Enum {
-	public static final field FILE Lokhttp3/BinaryMode;
-	public static final field HEX Lokhttp3/BinaryMode;
-	public static final field OMIT Lokhttp3/BinaryMode;
-	public static final field STDIN Lokhttp3/BinaryMode;
-	public static fun getEntries ()Lkotlin/enums/EnumEntries;
-	public static fun valueOf (Ljava/lang/String;)Lokhttp3/BinaryMode;
-	public static fun values ()[Lokhttp3/BinaryMode;
-}
-
 public final class okhttp3/Cache : java/io/Closeable, java/io/Flushable {
 	public static final field Companion Lokhttp3/Cache$Companion;
 	public final fun -deprecated_directory ()Ljava/io/File;
@@ -1030,10 +1020,9 @@ public final class okhttp3/Request {
 	public final fun tag ()Ljava/lang/Object;
 	public final fun tag (Ljava/lang/Class;)Ljava/lang/Object;
 	public final fun tag (Lkotlin/reflect/KClass;)Ljava/lang/Object;
-  public final fun toCurl ()Ljava/lang/String;
-  public final fun toCurl (Lokhttp3/BinaryMode;)Ljava/lang/String;
-  public final fun toCurl (Lokhttp3/BinaryMode;Ljava/lang/String;)Ljava/lang/String;
-  public static synthetic fun toCurl$default (Lokhttp3/Request;Lokhttp3/BinaryMode;Ljava/lang/String;ILjava/lang/Object;)Ljava/lang/String;
+	public final fun toCurl ()Ljava/lang/String;
+	public final fun toCurl (Z)Ljava/lang/String;
+	public static synthetic fun toCurl$default (Lokhttp3/Request;ZILjava/lang/Object;)Ljava/lang/String;
 	public fun toString ()Ljava/lang/String;
 	public final fun url ()Lokhttp3/HttpUrl;
 }

okhttp/api/android/okhttp.api

@@ -37,16 +37,6 @@ public abstract interface class okhttp3/Authenticator {
 public final class okhttp3/Authenticator$Companion {
 }
 
-public final class okhttp3/BinaryMode : java/lang/Enum {
-	public static final field FILE Lokhttp3/BinaryMode;
-	public static final field HEX Lokhttp3/BinaryMode;
-	public static final field OMIT Lokhttp3/BinaryMode;
-	public static final field STDIN Lokhttp3/BinaryMode;
-	public static fun getEntries ()Lkotlin/enums/EnumEntries;
-	public static fun valueOf (Ljava/lang/String;)Lokhttp3/BinaryMode;
-	public static fun values ()[Lokhttp3/BinaryMode;
-}
-
 public final class okhttp3/Cache : java/io/Closeable, java/io/Flushable {
 	public static final field Companion Lokhttp3/Cache$Companion;
 	public final fun -deprecated_directory ()Ljava/io/File;
@@ -1030,9 +1020,8 @@ public final class okhttp3/Request {
 	public final fun tag (Ljava/lang/Class;)Ljava/lang/Object;
 	public final fun tag (Lkotlin/reflect/KClass;)Ljava/lang/Object;
 	public final fun toCurl ()Ljava/lang/String;
-	public final fun toCurl (Lokhttp3/BinaryMode;)Ljava/lang/String;
-	public final fun toCurl (Lokhttp3/BinaryMode;Ljava/lang/String;)Ljava/lang/String;
-	public static synthetic fun toCurl$default (Lokhttp3/Request;Lokhttp3/BinaryMode;Ljava/lang/String;ILjava/lang/Object;)Ljava/lang/String;
+	public final fun toCurl (Z)Ljava/lang/String;
+	public static synthetic fun toCurl$default (Lokhttp3/Request;ZILjava/lang/Object;)Ljava/lang/String;
 	public fun toString ()Ljava/lang/String;
 	public final fun url ()Lokhttp3/HttpUrl;
 }

okhttp/api/jvm/okhttp.api

@@ -16,13 +16,13 @@
 package okhttp3
 
 import java.net.URL
-import java.nio.charset.StandardCharsets
 import kotlin.reflect.KClass
 import kotlin.reflect.cast
 import okhttp3.Headers.Companion.headersOf
 import okhttp3.HttpUrl.Companion.toHttpUrl
 import okhttp3.internal.http.GzipRequestBody
 import okhttp3.internal.http.HttpMethod
+import okhttp3.internal.isProbablyUtf8
 import okhttp3.internal.isSensitiveHeader
 import okio.Buffer
 
@@ -462,104 +462,55 @@ class Request internal constructor(
   }
 
   /**
-   * Returns a cURL command equivalent to this request, useful for debugging and reproducing requests.
+   * Returns a cURL command equivalent to this request, useful for debugging and reproducing
+   * requests.
    *
    * This includes the HTTP method, headers, request body (if present), and URL.
    *
    * Example:
+   *
    * ```
-   * curl -X POST -H "Authorization: Bearer token" --data "{\"key\":\"value\"}" "https://example.com/api"
+   * curl 'https://example.com/api' \
+   *   -X PUT \
+   *   -H 'Authorization: Bearer token' \
+   *   --data '{\"key\":\"value\"}'
    * ```
    *
-   * **Note:** This method will write the body
-   * to a temporary [okio.Buffer] in memory. This may have side effects if the [RequestBody] is streaming
-   * or can be consumed only once. Calling this method might prevent re-sending the request body later.
-   *
-   * @param binaryFileName default file name to use when dumping binary body data to a file (default: `"request_body.bin"`)
-   * @param binaryMode default mode to use when writing binary body data (default: `"BinaryMode.STDIN"`)
-   * @return a cURL command string representing this request.
+   * **Note:** This will consume the request body. This may have side effects if the [RequestBody]
+   * is streaming or can be consumed only once.
    */
   @JvmOverloads
-  fun toCurl(
-    binaryMode: BinaryMode = BinaryMode.STDIN,
-    binaryFileName: String? = "request_body.bin",
-  ): String {
-    val curl = StringBuilder("curl")
-
-    // Add method if not GET
-    if (method != "GET") {
-      curl.append(" -X ").append(method)
-    }
+  fun toCurl(includeBody: Boolean = true): String =
+    buildString {
+      append("curl ${url.toString().shellEscape()}")
 
-    // Append headers
-    for ((name, value) in headers) {
-      curl
-        .append(" -H \"")
-        .append(name)
-        .append(": ")
-        .append(value)
-        .append("\"")
-    }
-
-    // Append body if present
-    body?.let { requestBody ->
-      val buffer = Buffer()
-      requestBody.writeTo(buffer)
-
-      // Clone so we can read multiple times without consuming
-      val peekBuffer = buffer.clone()
-      val isBinary = isBinaryData(peekBuffer)
-
-      if (isBinary) {
-        when (binaryMode) {
-          BinaryMode.HEX -> {
-            curl.append(" --data-binary \"")
-            val hexBuffer = buffer.clone()
-            while (!hexBuffer.exhausted()) {
-              val b = hexBuffer.readByte().toInt() and 0xFF
-              curl.append("%02x".format(b))
-            }
-            curl.append("\"")
-          }
-          BinaryMode.FILE -> {
-            curl.append(" --data-binary @").append(binaryFileName)
-          }
-          BinaryMode.STDIN -> {
-            curl.append(" --data-binary @-")
-          }
-          BinaryMode.OMIT -> {
-            curl.append(" --data-binary \"[binary body omitted]\"")
-          }
+      // Add method if not the default.
+      val defaultMethod =
+        when {
+          includeBody && body != null -> "POST"
+          else -> "GET"
         }
-      } else {
-        val bodyString = buffer.readString(StandardCharsets.UTF_8)
-        curl
-          .append(" --data \"")
-          .append(bodyString.replace("\"", "\\\""))
-          .append("\"")
+      if (method != defaultMethod) {
+        append(" \\\n  -X ${method.shellEscape()}")
       }
-    }
 
-    curl.append(" \"").append(url).append("\"")
-    return curl.toString()
-  }
+      // Append headers.
+      for ((name, value) in headers) {
+        append(" \\\n  -H ${"$name: $value".shellEscape()}")
+      }
 
-  /**
-   * Detects binary data by checking for non-printable characters in a buffer.
-   */
-  private fun isBinaryData(peekBuffer: Buffer): Boolean {
-    var totalBytes = 0
-    var binaryCount = 0
-    val textSafeBytes = intArrayOf(0x09, 0x0A, 0x0D) // tab, LF, CR
-
-    while (!peekBuffer.exhausted() && totalBytes < 4096) { // limit to first 4KB for performance
-      val b = peekBuffer.readByte().toInt() and 0xFF
-      if ((b < 0x20 && b !in textSafeBytes) || b > 0x7E) {
-        binaryCount++
+      // Append body if present.
+      if (includeBody && body != null) {
+        val bodyBuffer = Buffer()
+        body.writeTo(bodyBuffer)
+
+        if (bodyBuffer.isProbablyUtf8()) {
+          append(" \\\n  --data ${bodyBuffer.readUtf8().shellEscape()}")
+        } else {
+          append(" \\\n  --data-binary ${bodyBuffer.readByteString().hex().shellEscape()}")
+        }
       }
-      totalBytes++
     }
 
-    return totalBytes > 0 && binaryCount > totalBytes * 0.1
-  }
+  private fun String.shellEscape(): String = "'${replace("'", "'\\''")}'"
 }

okhttp/src/commonJvmAndroid/kotlin/okhttp3/BinaryMode.kt

@@ -13,26 +13,26 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package okhttp3.logging.internal
+package okhttp3.internal
 
 import java.io.EOFException
-import okio.Buffer
+import okio.BufferedSource
 
 /**
- * Returns true if the body in question probably contains human readable text. Uses a small
- * sample of code points to detect unicode control characters commonly used in binary file
+ * Returns true if the body in question probably contains human-readable text. Uses a small
+ * sample of code points to detect Unicode control characters commonly used in binary file
  * signatures.
+ *
+ * @param codePointLimit the number of code points to read in order to make a decision.
  */
-fun Buffer.isProbablyUtf8(): Boolean {
+internal fun BufferedSource.isProbablyUtf8(codePointLimit: Long = Long.MAX_VALUE): Boolean {
   try {
-    val prefix = Buffer()
-    val byteCount = size.coerceAtMost(64)
-    copyTo(prefix, 0, byteCount)
-    for (i in 0 until 16) {
-      if (prefix.exhausted()) {
+    val peek = peek()
+    for (i in 0 until codePointLimit) {
+      if (peek.exhausted()) {
         break
       }
-      val codePoint = prefix.readUtf8CodePoint()
+      val codePoint = peek.readUtf8CodePoint()
       if (Character.isISOControl(codePoint) && !Character.isWhitespace(codePoint)) {
         return false
       }

okhttp/src/commonJvmAndroid/kotlin/okhttp3/Request.kt

@@ -0,0 +1,69 @@
+/*
+ * Copyright (C) 2015 Square, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package okhttp3.internal
+
+import assertk.assertThat
+import assertk.assertions.isEqualTo
+import assertk.assertions.isFalse
+import assertk.assertions.isTrue
+import okio.Buffer
+import okio.Source
+import okio.Timeout
+import okio.buffer
+import org.junit.jupiter.api.Test
+
+class IsProbablyUtf8Test {
+  @Test fun isProbablyUtf8() {
+    assertThat(Buffer().isProbablyUtf8(16L)).isTrue()
+    assertThat(Buffer().writeUtf8("abc").isProbablyUtf8(16L)).isTrue()
+    assertThat(Buffer().writeUtf8("new\r\nlines").isProbablyUtf8(16L)).isTrue()
+    assertThat(Buffer().writeUtf8("white\t space").isProbablyUtf8(16L)).isTrue()
+    assertThat(Buffer().writeUtf8("Слава Україні!").isProbablyUtf8(16L)).isTrue()
+    assertThat(Buffer().writeByte(0x80).isProbablyUtf8(16L)).isTrue()
+    assertThat(Buffer().writeByte(0x00).isProbablyUtf8(16L)).isFalse()
+    assertThat(Buffer().writeByte(0xc0).isProbablyUtf8(16L)).isFalse()
+  }
+
+  @Test fun doesNotConsumeBuffer() {
+    val buffer = Buffer()
+    buffer.writeUtf8("hello ".repeat(1024))
+    assertThat(buffer.isProbablyUtf8(100L)).isTrue()
+    assertThat(buffer.readUtf8()).isEqualTo("hello ".repeat(1024))
+  }
+
+  /** Confirm [isProbablyUtf8] doesn't attempt to read the entire stream. */
+  @Test fun doesNotReadEntireSource() {
+    val unlimitedSource =
+      object : Source {
+        override fun read(
+          sink: Buffer,
+          byteCount: Long,
+        ): Long {
+          sink.writeUtf8("a".repeat(byteCount.toInt()))
+          return byteCount
+        }
+
+        override fun close() {
+        }
+
+        override fun timeout() = Timeout.NONE
+      }
+
+    assertThat(unlimitedSource.buffer().isProbablyUtf8(1L)).isTrue()
+    assertThat(unlimitedSource.buffer().isProbablyUtf8(1024L)).isTrue()
+    assertThat(unlimitedSource.buffer().isProbablyUtf8(1024L * 1024L)).isTrue()
+  }
+}