From 37b16b40914407325eafad5f43f3ad468c840690 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Cantar=C3=ADn?= <omega_canta@yahoo.com>
Date: Wed, 5 May 2021 02:53:23 -0300
Subject: [PATCH] HTTP File upload fixes and tweaks

- Do not write the HTTP PUT response to cache which can potentially overflow the cache buffer as it is not consumed.
- VLOG(1) instead of LOG(ERROR) on HttpFile::Size() as it can be called during normal code execution.
- Add a command line flag `--user_agent` to allow users to specify their custom user agent string.

Fixes #939.
---
 packager/file/http_file.cc | 21 ++++++++++++++++-----
 packager/file/http_file.h  |  1 +
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/packager/file/http_file.cc b/packager/file/http_file.cc
index 8ba7b4719a..33c856bcb5 100644
--- a/packager/file/http_file.cc
+++ b/packager/file/http_file.cc
@@ -16,6 +16,8 @@
 #include "packager/base/strings/stringprintf.h"
 #include "packager/base/threading/worker_pool.h"
 
+DEFINE_string(user_agent, "",
+              "Set a custom User-Agent string for HTTP requests.");
 DEFINE_string(ca_file,
               "",
               "Absolute path to the Certificate Authority file for the "
@@ -45,8 +47,14 @@ constexpr const int kMinLogLevelForCurlDebugFunction = 2;
 
 size_t CurlWriteCallback(char* buffer, size_t size, size_t nmemb, void* user) {
   IoCache* cache = reinterpret_cast<IoCache*>(user);
-  size_t length = cache->Write(buffer, size * nmemb);
-  VLOG(3) << "CurlWriteCallback length=" << length;
+  size_t length = size * nmemb;
+  if (cache) {
+    length = cache->Write(buffer, length);
+    VLOG(3) << "CurlWriteCallback length=" << length;
+  } else {
+    // For the case of HTTP Put, the returned data may not be consumed. Return
+    // the size of the data to avoid curl errors.
+  }
   return length;
 }
 
@@ -159,6 +167,7 @@ HttpFile::HttpFile(HttpMethod method,
       upload_cache_(FLAGS_io_cache_size),
       curl_(curl_easy_init()),
       status_(Status::OK),
+      user_agent_(FLAGS_user_agent),
       task_exit_event_(base::WaitableEvent::ResetPolicy::MANUAL,
                        base::WaitableEvent::InitialState::NOT_SIGNALED) {
   static LibCurlInitializer lib_curl_initializer;
@@ -236,7 +245,7 @@ int64_t HttpFile::Write(const void* buffer, uint64_t length) {
 }
 
 int64_t HttpFile::Size() {
-  LOG(ERROR) << "HttpFile does not support Size().";
+  VLOG(1) << "HttpFile does not support Size().";
   return -1;
 }
 
@@ -279,12 +288,14 @@ void HttpFile::SetupRequest() {
   }
 
   curl_easy_setopt(curl, CURLOPT_URL, url_.c_str());
-  curl_easy_setopt(curl, CURLOPT_USERAGENT, kUserAgent);
+  curl_easy_setopt(curl, CURLOPT_USERAGENT,
+                   user_agent_.empty() ? kUserAgent : user_agent_.data());
   curl_easy_setopt(curl, CURLOPT_TIMEOUT, timeout_in_seconds_);
   curl_easy_setopt(curl, CURLOPT_FAILONERROR, 1L);
   curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L);
   curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, &CurlWriteCallback);
-  curl_easy_setopt(curl, CURLOPT_WRITEDATA, &download_cache_);
+  curl_easy_setopt(curl, CURLOPT_WRITEDATA,
+                   method_ == HttpMethod::kPut ? nullptr : &download_cache_);
   if (method_ != HttpMethod::kGet) {
     curl_easy_setopt(curl, CURLOPT_READFUNCTION, &CurlReadCallback);
     curl_easy_setopt(curl, CURLOPT_READDATA, &upload_cache_);
diff --git a/packager/file/http_file.h b/packager/file/http_file.h
index c0bd681093..92f30b4d19 100644
--- a/packager/file/http_file.h
+++ b/packager/file/http_file.h
@@ -83,6 +83,7 @@ class HttpFile : public File {
   // The headers need to remain alive for the duration of the request.
   std::unique_ptr<curl_slist, CurlDelete> request_headers_;
   Status status_;
+  std::string user_agent_;
 
   // Signaled when the "curl easy perform" task completes.
   base::WaitableEvent task_exit_event_;