From b69b61c100303cb58dd130efad14232024f2db0e Mon Sep 17 00:00:00 2001 From: Joey Parrish Date: Wed, 23 Oct 2024 20:33:52 -0700 Subject: [PATCH 1/2] feat(http): Add DELETE method support When we try to remove an old segment from a live stream we uploaded via HTTP, we need to send DELETE requests. --- docs/source/tutorials/http_upload.rst | 8 -------- packager/file/file.cc | 12 ++++++++++-- packager/file/http_file.cc | 20 +++++++++++++++++--- packager/file/http_file.h | 4 ++++ packager/file/http_file_unittest.cc | 12 ++++++++++++ 5 files changed, 43 insertions(+), 13 deletions(-) diff --git a/docs/source/tutorials/http_upload.rst b/docs/source/tutorials/http_upload.rst index e770326a6ef..efe0e3109f9 100644 --- a/docs/source/tutorials/http_upload.rst +++ b/docs/source/tutorials/http_upload.rst @@ -101,14 +101,6 @@ Backlog Please note the HTTP upload feature still lacks some features probably important for production. Contributions are welcome! -HTTP DELETE -=========== -Nothing has be done to support this yet: - - Packager supports removing old segments automatically. - See ``preserved_segments_outside_live_window`` option in - DASH_ options or HLS_ options for details. - Software tests ============== We should do some minimal QA, check whether the test diff --git a/packager/file/file.cc b/packager/file/file.cc index 9e97f96c867..02704f5e9bc 100644 --- a/packager/file/file.cc +++ b/packager/file/file.cc @@ -111,6 +111,10 @@ File* CreateHttpsFile(const char* file_name, const char* mode) { return new HttpFile(method, std::string("https://") + file_name); } +bool DeleteHttpsFile(const char* file_name) { + return HttpFile::Delete(std::string("https://") + file_name); +} + File* CreateHttpFile(const char* file_name, const char* mode) { HttpMethod method = HttpMethod::kGet; if (strcmp(mode, "r") != 0) { @@ -119,6 +123,10 @@ File* CreateHttpFile(const char* file_name, const char* mode) { return new HttpFile(method, std::string("http://") + file_name); } +bool DeleteHttpFile(const char* file_name) { + return HttpFile::Delete(std::string("http://") + file_name); +} + File* CreateMemoryFile(const char* file_name, const char* mode) { return new MemoryFile(file_name, mode); } @@ -138,8 +146,8 @@ static const FileTypeInfo kFileTypeInfo[] = { {kUdpFilePrefix, &CreateUdpFile, nullptr, nullptr}, {kMemoryFilePrefix, &CreateMemoryFile, &DeleteMemoryFile, nullptr}, {kCallbackFilePrefix, &CreateCallbackFile, nullptr, nullptr}, - {kHttpFilePrefix, &CreateHttpFile, nullptr, nullptr}, - {kHttpsFilePrefix, &CreateHttpsFile, nullptr, nullptr}, + {kHttpFilePrefix, &CreateHttpFile, &DeleteHttpFile, nullptr}, + {kHttpsFilePrefix, &CreateHttpsFile, &DeleteHttpsFile, nullptr}, }; std::string_view GetFileTypePrefix(std::string_view file_name) { diff --git a/packager/file/http_file.cc b/packager/file/http_file.cc index b50fef6f072..83bfd475f00 100644 --- a/packager/file/http_file.cc +++ b/packager/file/http_file.cc @@ -14,6 +14,7 @@ #include #include +#include #include #include #include @@ -174,6 +175,7 @@ HttpFile::HttpFile(HttpMethod method, upload_content_type_(upload_content_type), timeout_in_seconds_(timeout_in_seconds), method_(method), + isUpload_(method == HttpMethod::kPut || method == HttpMethod::kPost), download_cache_(absl::GetFlag(FLAGS_io_cache_size)), upload_cache_(absl::GetFlag(FLAGS_io_cache_size)), curl_(curl_easy_init()), @@ -201,8 +203,7 @@ HttpFile::HttpFile(HttpMethod method, !AppendHeader("Content-Type: " + upload_content_type_, &temp_headers)) { return; } - if (method != HttpMethod::kGet && - !AppendHeader("Transfer-Encoding: chunked", &temp_headers)) { + if (isUpload_ && !AppendHeader("Transfer-Encoding: chunked", &temp_headers)) { return; } for (const auto& item : headers) { @@ -215,6 +216,16 @@ HttpFile::HttpFile(HttpMethod method, HttpFile::~HttpFile() {} +// static +bool HttpFile::Delete(const std::string& url) { + std::unique_ptr file( + new HttpFile(HttpMethod::kDelete, url)); + if (!file->Open()) { + return false; + } + return file->Close(); +} + bool HttpFile::Open() { VLOG(2) << "Opening " << url_; @@ -313,6 +324,9 @@ void HttpFile::SetupRequest() { case HttpMethod::kPut: curl_easy_setopt(curl, CURLOPT_PUT, 1L); break; + case HttpMethod::kDelete: + curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, "DELETE"); + break; } curl_easy_setopt(curl, CURLOPT_URL, url_.c_str()); @@ -322,7 +336,7 @@ void HttpFile::SetupRequest() { curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L); curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, &CurlWriteCallback); curl_easy_setopt(curl, CURLOPT_WRITEDATA, &download_cache_); - if (method_ != HttpMethod::kGet) { + if (isUpload_) { 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 f0207e2cac5..9d55acfa286 100644 --- a/packager/file/http_file.h +++ b/packager/file/http_file.h @@ -24,6 +24,7 @@ enum class HttpMethod { kGet, kPost, kPut, + kDelete, }; /// HttpFile reads or writes network requests. @@ -47,6 +48,8 @@ class HttpFile : public File { HttpFile(const HttpFile&) = delete; HttpFile& operator=(const HttpFile&) = delete; + static bool Delete(const std::string& url); + Status CloseWithStatus(); /// @name File implementation overrides. @@ -78,6 +81,7 @@ class HttpFile : public File { const std::string upload_content_type_; const int32_t timeout_in_seconds_; const HttpMethod method_; + const bool isUpload_; IoCache download_cache_; IoCache upload_cache_; std::unique_ptr curl_; diff --git a/packager/file/http_file_unittest.cc b/packager/file/http_file_unittest.cc index a4633acccf3..427dc0639e2 100644 --- a/packager/file/http_file_unittest.cc +++ b/packager/file/http_file_unittest.cc @@ -273,6 +273,18 @@ TEST_F(HttpFileTest, MultipleChunks) { ASSERT_JSON_STRING(json, "headers.Transfer-Encoding", "chunked"); } +TEST_F(HttpFileTest, BasicDelete) { + FilePtr file(new HttpFile(HttpMethod::kDelete, server_.ReflectUrl(), + kNoContentType, kNoHeaders, kDefaultTestTimeout)); + ASSERT_TRUE(file); + ASSERT_TRUE(file->Open()); + + auto json = HandleResponse(file); + ASSERT_TRUE(json.is_object()); + ASSERT_TRUE(file.release()->Close()); + ASSERT_JSON_STRING(json, "method", "DELETE"); +} + TEST_F(HttpFileTest, Error404) { FilePtr file(new HttpFile(HttpMethod::kGet, server_.StatusCodeUrl(404), kNoContentType, kNoHeaders, kDefaultTestTimeout)); From 96b6dc36c06cbc62a34d7c2743dfba127aff2f9e Mon Sep 17 00:00:00 2001 From: Joey Parrish Date: Wed, 23 Oct 2024 21:22:25 -0700 Subject: [PATCH 2/2] Fix crash --- packager/file/http_file.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packager/file/http_file.cc b/packager/file/http_file.cc index 83bfd475f00..eb1967e9094 100644 --- a/packager/file/http_file.cc +++ b/packager/file/http_file.cc @@ -223,7 +223,7 @@ bool HttpFile::Delete(const std::string& url) { if (!file->Open()) { return false; } - return file->Close(); + return file.release()->Close(); } bool HttpFile::Open() {