From b24866269dd23b51a419b06223ccc3ae6f247512 Mon Sep 17 00:00:00 2001
From: Bryce Mecum <petridish@gmail.com>
Date: Thu, 9 Mar 2023 12:11:24 -0900
Subject: [PATCH 01/11] Let GcsFileSystem take a path for json_credentials

---
 r/R/filesystem.R            |  9 +++++++--
 r/man/FileSystem.Rd         |  4 ++--
 r/tests/testthat/test-gcs.R | 12 ++++++++++++
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/r/R/filesystem.R b/r/R/filesystem.R
index d6554239f630..461f48918e8d 100644
--- a/r/R/filesystem.R
+++ b/r/R/filesystem.R
@@ -167,8 +167,8 @@ FileSelector$create <- function(base_dir, allow_not_found = FALSE, recursive = F
 #'   with `expiration`
 #' - `expiration`: `POSIXct`. optional datetime representing point at which
 #'   `access_token` will expire.
-#' - `json_credentials`: optional string for authentication. Point to a JSON
-#'   credentials file downloaded from GCS.
+#' - `json_credentials`: optional string for authentication. Either a string
+#'   containing JSON credentials or a path to their location on the filesystem.
 #' - `endpoint_override`: if non-empty, will connect to provided host name / port,
 #'   such as "localhost:9001", instead of default GCS ones. This is primarily useful
 #'   for testing purposes.
@@ -572,6 +572,11 @@ GcsFileSystem$create <- function(anonymous = FALSE, retry_limit_seconds = 15, ..
 
   options$retry_limit_seconds <- retry_limit_seconds
 
+  # Handle reading json_credentials from the filesystem
+  if ("json_credentials" %in% names(options) && file.exists(options[["json_credentials"]])) {
+    options[["json_credentials"]] <- paste(readLines(options[["json_credentials"]]), collapse = "")
+  }
+
   fs___GcsFileSystem__Make(anonymous, options)
 }
 
diff --git a/r/man/FileSystem.Rd b/r/man/FileSystem.Rd
index c9586f70e716..b8267510ce27 100644
--- a/r/man/FileSystem.Rd
+++ b/r/man/FileSystem.Rd
@@ -69,8 +69,8 @@ credentials using standard GCS configuration methods.
 with \code{expiration}
 \item \code{expiration}: \code{POSIXct}. optional datetime representing point at which
 \code{access_token} will expire.
-\item \code{json_credentials}: optional string for authentication. Point to a JSON
-credentials file downloaded from GCS.
+\item \code{json_credentials}: optional string for authentication. Either a string
+containing JSON credentials or a path to their location on the filesystem.
 \item \code{endpoint_override}: if non-empty, will connect to provided host name / port,
 such as "localhost:9001", instead of default GCS ones. This is primarily useful
 for testing purposes.
diff --git a/r/tests/testthat/test-gcs.R b/r/tests/testthat/test-gcs.R
index e284beb225e2..fe9d21dffbc6 100644
--- a/r/tests/testthat/test-gcs.R
+++ b/r/tests/testthat/test-gcs.R
@@ -91,6 +91,18 @@ test_that("GcsFileSystem$create() input validation", {
   )
 })
 
+test_that("GcsFileSystem$create() can read json_credentials", {
+  # From string
+  fs <- GcsFileSystem$create(json_credentials = "fromstring")
+  expect_equal(fs$options$json_credentials, "fromstring")
+
+  # From disk
+  cred_path <- tempfile()
+  writeLines("fromdisk", cred_path)
+  fs <- GcsFileSystem$create(json_credentials = cred_path)
+  expect_equal(fs$options$json_credentials, "fromdisk")
+})
+
 skip_on_cran()
 skip_if_not(system('python -c "import testbench"') == 0, message = "googleapis-storage-testbench is not installed.")
 library(dplyr)

From f5519a78cf854333098901bf93060339cefc919d Mon Sep 17 00:00:00 2001
From: Bryce Mecum <petridish@gmail.com>
Date: Thu, 9 Mar 2023 12:20:30 -0900
Subject: [PATCH 02/11] Add an unlink to test-gcs

---
 r/tests/testthat/test-gcs.R | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/r/tests/testthat/test-gcs.R b/r/tests/testthat/test-gcs.R
index fe9d21dffbc6..2ae7ddd4ac07 100644
--- a/r/tests/testthat/test-gcs.R
+++ b/r/tests/testthat/test-gcs.R
@@ -98,6 +98,8 @@ test_that("GcsFileSystem$create() can read json_credentials", {
 
   # From disk
   cred_path <- tempfile()
+  on.exit(unlink(cred_path))
+
   writeLines("fromdisk", cred_path)
   fs <- GcsFileSystem$create(json_credentials = cred_path)
   expect_equal(fs$options$json_credentials, "fromdisk")

From d12fd16575c76aead3e109a289133d7631134d5a Mon Sep 17 00:00:00 2001
From: Bryce Mecum <petridish@gmail.com>
Date: Fri, 17 Mar 2023 11:51:15 -0800
Subject: [PATCH 03/11] Collapse with \n

---
 r/R/filesystem.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/r/R/filesystem.R b/r/R/filesystem.R
index 461f48918e8d..ced603e94035 100644
--- a/r/R/filesystem.R
+++ b/r/R/filesystem.R
@@ -574,7 +574,7 @@ GcsFileSystem$create <- function(anonymous = FALSE, retry_limit_seconds = 15, ..
 
   # Handle reading json_credentials from the filesystem
   if ("json_credentials" %in% names(options) && file.exists(options[["json_credentials"]])) {
-    options[["json_credentials"]] <- paste(readLines(options[["json_credentials"]]), collapse = "")
+    options[["json_credentials"]] <- paste(readLines(options[["json_credentials"]]), collapse = "\n")
   }
 
   fs___GcsFileSystem__Make(anonymous, options)

From f3018b0351082f26f5bb0c6e69ee87749041a3f4 Mon Sep 17 00:00:00 2001
From: Bryce Mecum <petridish@gmail.com>
Date: Fri, 17 Mar 2023 12:50:03 -0800
Subject: [PATCH 04/11] Use >ASCII characters in test-gcs test

---
 r/tests/testthat/test-gcs.R | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/r/tests/testthat/test-gcs.R b/r/tests/testthat/test-gcs.R
index 2ae7ddd4ac07..4a48f3b2fba1 100644
--- a/r/tests/testthat/test-gcs.R
+++ b/r/tests/testthat/test-gcs.R
@@ -99,10 +99,16 @@ test_that("GcsFileSystem$create() can read json_credentials", {
   # From disk
   cred_path <- tempfile()
   on.exit(unlink(cred_path))
-
-  writeLines("fromdisk", cred_path)
-  fs <- GcsFileSystem$create(json_credentials = cred_path)
-  expect_equal(fs$options$json_credentials, "fromdisk")
+  con <- file(cred_path, open = "wb")
+  writeBin('{"key" : "valu\u00e9"}', con)
+  close(con)
+
+  # This calls readLines which complains about embedded nuls and missing a
+  # final newline (See ?readLines)
+  suppressWarnings({
+    fs <- GcsFileSystem$create(json_credentials = cred_path)
+  })
+  expect_equal(fs$options$json_credentials, "{\"key\" : \"valué\"}")
 })
 
 skip_on_cran()

From 617d8b341c13a1c5d5b9a073963ad1ee1edd0d73 Mon Sep 17 00:00:00 2001
From: Bryce Mecum <petridish@gmail.com>
Date: Mon, 20 Mar 2023 09:12:03 -0800
Subject: [PATCH 05/11] Use iconv to write a raw string

This fixes two issues that using writeBin directly on a character vector has (embedded nul, no final newline)
---
 r/tests/testthat/test-gcs.R | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/r/tests/testthat/test-gcs.R b/r/tests/testthat/test-gcs.R
index 4a48f3b2fba1..9d00277b4f45 100644
--- a/r/tests/testthat/test-gcs.R
+++ b/r/tests/testthat/test-gcs.R
@@ -100,14 +100,10 @@ test_that("GcsFileSystem$create() can read json_credentials", {
   cred_path <- tempfile()
   on.exit(unlink(cred_path))
   con <- file(cred_path, open = "wb")
-  writeBin('{"key" : "valu\u00e9"}', con)
+  writeBin(iconv('{"key" : "valu\u00e9"}\n', toRaw = TRUE)[[1]], con)
   close(con)
 
-  # This calls readLines which complains about embedded nuls and missing a
-  # final newline (See ?readLines)
-  suppressWarnings({
-    fs <- GcsFileSystem$create(json_credentials = cred_path)
-  })
+  fs <- GcsFileSystem$create(json_credentials = cred_path)
   expect_equal(fs$options$json_credentials, "{\"key\" : \"valué\"}")
 })
 

From 35633e5c4fb35958cd43dc4cc104a5927ba3d636 Mon Sep 17 00:00:00 2001
From: Bryce Mecum <petridish@gmail.com>
Date: Mon, 20 Mar 2023 12:22:23 -0800
Subject: [PATCH 06/11] Debugging Windows CI

---
 r/tests/testthat/test-gcs.R | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/r/tests/testthat/test-gcs.R b/r/tests/testthat/test-gcs.R
index 9d00277b4f45..01a39d6ee829 100644
--- a/r/tests/testthat/test-gcs.R
+++ b/r/tests/testthat/test-gcs.R
@@ -103,6 +103,16 @@ test_that("GcsFileSystem$create() can read json_credentials", {
   writeBin(iconv('{"key" : "valu\u00e9"}\n', toRaw = TRUE)[[1]], con)
   close(con)
 
+  # WIP: Debugging https://github.com/apache/arrow/pull/34524 via CI
+  cred_path <- tempfile()
+  on.exit(unlink(cred_path))
+  con <- file(cred_path, open = "wb")
+  input <- '{"key" : "valu\u00e9"}\n'
+  print(Encoding(input))
+  writeBin(iconv(input, from = Encoding(input), out = "UTF-8", toRaw = TRUE)[[1]], con)
+  close(con) # Close now, Windows file handles are special
+  print(readBin(cred_path, "raw", file.size(cred_path)))
+
   fs <- GcsFileSystem$create(json_credentials = cred_path)
   expect_equal(fs$options$json_credentials, "{\"key\" : \"valué\"}")
 })

From 853487c09c9b81a6c11a8b0fb9e630a10c2107b1 Mon Sep 17 00:00:00 2001
From: Bryce Mecum <petridish@gmail.com>
Date: Mon, 20 Mar 2023 13:58:29 -0800
Subject: [PATCH 07/11] Debugging Windows CI part 2

---
 r/tests/testthat/test-gcs.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/r/tests/testthat/test-gcs.R b/r/tests/testthat/test-gcs.R
index 01a39d6ee829..be8f1fdd41aa 100644
--- a/r/tests/testthat/test-gcs.R
+++ b/r/tests/testthat/test-gcs.R
@@ -109,7 +109,7 @@ test_that("GcsFileSystem$create() can read json_credentials", {
   con <- file(cred_path, open = "wb")
   input <- '{"key" : "valu\u00e9"}\n'
   print(Encoding(input))
-  writeBin(iconv(input, from = Encoding(input), out = "UTF-8", toRaw = TRUE)[[1]], con)
+  writeBin(iconv(input, from = Encoding(input), to = "UTF-8", toRaw = TRUE)[[1]], con)
   close(con) # Close now, Windows file handles are special
   print(readBin(cred_path, "raw", file.size(cred_path)))
 

From e2bf45f0cb44dfa5555cd6a33ae2847cea67e2ce Mon Sep 17 00:00:00 2001
From: Bryce Mecum <petridish@gmail.com>
Date: Mon, 20 Mar 2023 15:22:51 -0800
Subject: [PATCH 08/11] Assume UTF-8 when reading credentials

---
 r/R/filesystem.R    | 3 ++-
 r/man/FileSystem.Rd | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/r/R/filesystem.R b/r/R/filesystem.R
index ced603e94035..98f956e8d7d1 100644
--- a/r/R/filesystem.R
+++ b/r/R/filesystem.R
@@ -169,6 +169,7 @@ FileSelector$create <- function(base_dir, allow_not_found = FALSE, recursive = F
 #'   `access_token` will expire.
 #' - `json_credentials`: optional string for authentication. Either a string
 #'   containing JSON credentials or a path to their location on the filesystem.
+#'   If a path to credentials is given, the file should be UTF-8 encoded.
 #' - `endpoint_override`: if non-empty, will connect to provided host name / port,
 #'   such as "localhost:9001", instead of default GCS ones. This is primarily useful
 #'   for testing purposes.
@@ -574,7 +575,7 @@ GcsFileSystem$create <- function(anonymous = FALSE, retry_limit_seconds = 15, ..
 
   # Handle reading json_credentials from the filesystem
   if ("json_credentials" %in% names(options) && file.exists(options[["json_credentials"]])) {
-    options[["json_credentials"]] <- paste(readLines(options[["json_credentials"]]), collapse = "\n")
+    options[["json_credentials"]] <- paste(readLines(options[["json_credentials"]], encoding = "UTF-8"), collapse = "\n")
   }
 
   fs___GcsFileSystem__Make(anonymous, options)
diff --git a/r/man/FileSystem.Rd b/r/man/FileSystem.Rd
index b8267510ce27..38c694af995c 100644
--- a/r/man/FileSystem.Rd
+++ b/r/man/FileSystem.Rd
@@ -71,6 +71,7 @@ with \code{expiration}
 \code{access_token} will expire.
 \item \code{json_credentials}: optional string for authentication. Either a string
 containing JSON credentials or a path to their location on the filesystem.
+If a path to credentials is given, the file should be UTF-8 encoded.
 \item \code{endpoint_override}: if non-empty, will connect to provided host name / port,
 such as "localhost:9001", instead of default GCS ones. This is primarily useful
 for testing purposes.

From 4ee689ce99940eee9c96897bb91df7576de7d2d4 Mon Sep 17 00:00:00 2001
From: Bryce Mecum <petridish@gmail.com>
Date: Mon, 20 Mar 2023 15:23:03 -0800
Subject: [PATCH 09/11] Update tests

---
 r/tests/testthat/test-gcs.R | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/r/tests/testthat/test-gcs.R b/r/tests/testthat/test-gcs.R
index be8f1fdd41aa..a3585ceacc18 100644
--- a/r/tests/testthat/test-gcs.R
+++ b/r/tests/testthat/test-gcs.R
@@ -100,19 +100,9 @@ test_that("GcsFileSystem$create() can read json_credentials", {
   cred_path <- tempfile()
   on.exit(unlink(cred_path))
   con <- file(cred_path, open = "wb")
-  writeBin(iconv('{"key" : "valu\u00e9"}\n', toRaw = TRUE)[[1]], con)
+  writeBin(iconv('{"key" : "valu\u00e9"}\n', to = "UTF-8", toRaw = TRUE)[[1]], con)
   close(con)
 
-  # WIP: Debugging https://github.com/apache/arrow/pull/34524 via CI
-  cred_path <- tempfile()
-  on.exit(unlink(cred_path))
-  con <- file(cred_path, open = "wb")
-  input <- '{"key" : "valu\u00e9"}\n'
-  print(Encoding(input))
-  writeBin(iconv(input, from = Encoding(input), to = "UTF-8", toRaw = TRUE)[[1]], con)
-  close(con) # Close now, Windows file handles are special
-  print(readBin(cred_path, "raw", file.size(cred_path)))
-
   fs <- GcsFileSystem$create(json_credentials = cred_path)
   expect_equal(fs$options$json_credentials, "{\"key\" : \"valué\"}")
 })

From a9bd856673b2151a2c064e5a3e08622d40f843b3 Mon Sep 17 00:00:00 2001
From: Bryce Mecum <petridish@gmail.com>
Date: Mon, 20 Mar 2023 16:36:32 -0800
Subject: [PATCH 10/11] Swap out readLines for more explicit read function

---
 r/R/filesystem.R            | 9 ++++++++-
 r/tests/testthat/test-gcs.R | 2 +-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/r/R/filesystem.R b/r/R/filesystem.R
index 98f956e8d7d1..5a1e0eab6066 100644
--- a/r/R/filesystem.R
+++ b/r/R/filesystem.R
@@ -575,7 +575,7 @@ GcsFileSystem$create <- function(anonymous = FALSE, retry_limit_seconds = 15, ..
 
   # Handle reading json_credentials from the filesystem
   if ("json_credentials" %in% names(options) && file.exists(options[["json_credentials"]])) {
-    options[["json_credentials"]] <- paste(readLines(options[["json_credentials"]], encoding = "UTF-8"), collapse = "\n")
+    options[["json_credentials"]] <- paste(read_file_utf8(options[["json_credentials"]]), collapse = "\n")
   }
 
   fs___GcsFileSystem__Make(anonymous, options)
@@ -663,3 +663,10 @@ clean_path_rel <- function(path) {
   path_sep <- ifelse(tolower(Sys.info()[["sysname"]]) == "windows", "\\\\", "/")
   gsub(path_sep, "/", path)
 }
+
+read_file_utf8 <- function(file) {
+  res <- readBin(file, "raw", n = file.size(file))
+  res <- rawToChar(res)
+  Encoding(res) <- "UTF-8"
+  res
+}
diff --git a/r/tests/testthat/test-gcs.R b/r/tests/testthat/test-gcs.R
index a3585ceacc18..8728616238cd 100644
--- a/r/tests/testthat/test-gcs.R
+++ b/r/tests/testthat/test-gcs.R
@@ -100,7 +100,7 @@ test_that("GcsFileSystem$create() can read json_credentials", {
   cred_path <- tempfile()
   on.exit(unlink(cred_path))
   con <- file(cred_path, open = "wb")
-  writeBin(iconv('{"key" : "valu\u00e9"}\n', to = "UTF-8", toRaw = TRUE)[[1]], con)
+  writeBin(iconv('{"key" : "valu\u00e9"}', to = "UTF-8", toRaw = TRUE)[[1]], con)
   close(con)
 
   fs <- GcsFileSystem$create(json_credentials = cred_path)

From 8ea6d55b08dfbf109a987eeff8d27435d50cf838 Mon Sep 17 00:00:00 2001
From: Bryce Mecum <petridish@gmail.com>
Date: Tue, 21 Mar 2023 14:58:14 -0800
Subject: [PATCH 11/11] Update test-gcs.R

---
 r/tests/testthat/test-gcs.R | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/r/tests/testthat/test-gcs.R b/r/tests/testthat/test-gcs.R
index 8728616238cd..fd173e923b27 100644
--- a/r/tests/testthat/test-gcs.R
+++ b/r/tests/testthat/test-gcs.R
@@ -97,10 +97,18 @@ test_that("GcsFileSystem$create() can read json_credentials", {
   expect_equal(fs$options$json_credentials, "fromstring")
 
   # From disk
+  cred_string <- '{"key" : "valu\u00e9"}'
+  cred_string_bytes_utf8 <- iconv(
+    cred_string,
+    from = Encoding(cred_string),
+    to = "UTF-8",
+    toRaw = TRUE
+  )[[1]]
+
   cred_path <- tempfile()
   on.exit(unlink(cred_path))
   con <- file(cred_path, open = "wb")
-  writeBin(iconv('{"key" : "valu\u00e9"}', to = "UTF-8", toRaw = TRUE)[[1]], con)
+  writeBin(cred_string_bytes_utf8, con)
   close(con)
 
   fs <- GcsFileSystem$create(json_credentials = cred_path)