From da650fbfc0a7a495d5190f3426e83c4f9f068ee5 Mon Sep 17 00:00:00 2001 From: Ian Thomas Date: Tue, 12 May 2026 07:51:49 +0100 Subject: [PATCH 1/3] Separate read buffer class instead of in response class --- CMakeLists.txt | 2 ++ src/wasm/read_buffer.cpp | 12 ++++++++++++ src/wasm/read_buffer.hpp | 18 ++++++++++++++++++ src/wasm/response.cpp | 9 ++------- src/wasm/response.hpp | 7 ++----- src/wasm/stream.cpp | 20 ++++++++++++-------- 6 files changed, 48 insertions(+), 20 deletions(-) create mode 100644 src/wasm/read_buffer.cpp create mode 100644 src/wasm/read_buffer.hpp diff --git a/CMakeLists.txt b/CMakeLists.txt index a045f1e..6f542a4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -103,6 +103,8 @@ set(GIT2CPP_SRC ${GIT2CPP_SOURCE_DIR}/wasm/constants.hpp ${GIT2CPP_SOURCE_DIR}/wasm/libgit2_internals.cpp ${GIT2CPP_SOURCE_DIR}/wasm/libgit2_internals.hpp + ${GIT2CPP_SOURCE_DIR}/wasm/read_buffer.cpp + ${GIT2CPP_SOURCE_DIR}/wasm/read_buffer.hpp ${GIT2CPP_SOURCE_DIR}/wasm/response.cpp ${GIT2CPP_SOURCE_DIR}/wasm/response.hpp ${GIT2CPP_SOURCE_DIR}/wasm/scope.cpp diff --git a/src/wasm/read_buffer.cpp b/src/wasm/read_buffer.cpp new file mode 100644 index 0000000..c4a59ea --- /dev/null +++ b/src/wasm/read_buffer.cpp @@ -0,0 +1,12 @@ +#ifdef EMSCRIPTEN + +# include "read_buffer.hpp" + +read_buffer_t::read_buffer_t(char* buffer, size_t buffer_size, size_t* bytes_read) + : m_buffer(buffer) + , m_buffer_size(buffer_size) + , m_bytes_read(bytes_read) +{ +} + +#endif // EMSCRIPTEN diff --git a/src/wasm/read_buffer.hpp b/src/wasm/read_buffer.hpp new file mode 100644 index 0000000..e874f26 --- /dev/null +++ b/src/wasm/read_buffer.hpp @@ -0,0 +1,18 @@ +#pragma once + +#ifdef EMSCRIPTEN + +# include + +// Buffer used when reading remote data is created by libgit2 and passed to transport layer to use. +// We only fill the buffer and otherwise return it unmodified. +struct read_buffer_t +{ + read_buffer_t(char* buffer, size_t buffer_size, size_t* bytes_read); + + char* m_buffer; + size_t m_buffer_size; + size_t* m_bytes_read; +}; + +#endif // EMSCRIPTEN diff --git a/src/wasm/response.cpp b/src/wasm/response.cpp index 8ba0b1b..620a4c2 100644 --- a/src/wasm/response.cpp +++ b/src/wasm/response.cpp @@ -5,13 +5,9 @@ # include "../utils/common.hpp" # include "libgit2_internals.hpp" -wasm_http_response::wasm_http_response(char* buffer, size_t buffer_size, size_t* bytes_read) - : m_buffer(buffer) - , m_buffer_size(buffer_size) - , m_bytes_read(bytes_read) - , m_status(0) +wasm_http_response::wasm_http_response() + : m_status(0) { - *m_bytes_read = 0; } void wasm_http_response::add_header(const std::string& key, const std::string& value) @@ -21,7 +17,6 @@ void wasm_http_response::add_header(const std::string& key, const std::string& v void wasm_http_response::clear() { - *m_bytes_read = 0; m_status = 0; m_status_text.clear(); m_response_headers.clear(); diff --git a/src/wasm/response.hpp b/src/wasm/response.hpp index 42b31c3..84d96e5 100644 --- a/src/wasm/response.hpp +++ b/src/wasm/response.hpp @@ -13,7 +13,7 @@ class wasm_http_response { public: - wasm_http_response(char* buffer, size_t buffer_size, size_t* bytes_read); + wasm_http_response(); void add_header(const std::string& key, const std::string& value); @@ -29,10 +29,7 @@ class wasm_http_response void set_git_error(std::string_view url) const; - char* m_buffer; // Not owned. - size_t m_buffer_size; - size_t* m_bytes_read; // Not owned. - int32_t m_status; // Specific type corresponding to i32 in emscripten setValue call. + int32_t m_status; // Specific type corresponding to i32 in emscripten setValue call. std::string m_status_text; private: diff --git a/src/wasm/stream.cpp b/src/wasm/stream.cpp index 8b356a5..2662805 100644 --- a/src/wasm/stream.cpp +++ b/src/wasm/stream.cpp @@ -9,6 +9,7 @@ # include "../utils/common.hpp" # include "constants.hpp" +# include "read_buffer.hpp" # include "response.hpp" // Buffer size used in transport_smart, hardcoded in libgit2. @@ -308,7 +309,8 @@ static void delete_request(wasm_http_stream* stream) } } -static int read(wasm_http_stream* stream, wasm_http_response& response, bool is_read_response) +static int +read(wasm_http_stream* stream, read_buffer_t& read_buffer, wasm_http_response& response, bool is_read_response) { if (is_read_response) { @@ -348,8 +350,8 @@ static int read(wasm_http_stream* stream, wasm_http_response& response, bool is_ // Actual read. size_t bytes_read = js_read( stream->m_request_index, - response.m_buffer, - response.m_buffer_size, + read_buffer.m_buffer, + read_buffer.m_buffer_size, &response.m_status, &status_text, &response_headers @@ -399,7 +401,7 @@ static int read(wasm_http_stream* stream, wasm_http_response& response, bool is_ } } - *response.m_bytes_read = bytes_read; + *read_buffer.m_bytes_read = bytes_read; return 0; } @@ -541,12 +543,13 @@ void wasm_http_stream_free(git_smart_subtransport_stream* s) int wasm_http_stream_read(git_smart_subtransport_stream* s, char* buffer, size_t buffer_size, size_t* bytes_read) { wasm_http_stream* stream = reinterpret_cast(s); - wasm_http_response response(buffer, buffer_size, bytes_read); + read_buffer_t read_buffer(buffer, buffer_size, bytes_read); + wasm_http_response response; bool send = true; while (send) { - if (read(stream, response, false) == static_cast(-1)) + if (read(stream, read_buffer, response, false) == static_cast(-1)) { return -1; // git error already set. } @@ -597,8 +600,9 @@ int wasm_http_stream_read_response(git_smart_subtransport_stream* s, char* buffe { wasm_http_stream* stream = reinterpret_cast(s); - wasm_http_response response(buffer, buffer_size, bytes_read); - int error = read(stream, response, true); + read_buffer_t read_buffer(buffer, buffer_size, bytes_read); + wasm_http_response response; + int error = read(stream, read_buffer, response, true); // May need similar handling of response status and headers as occurs in read() above, but so // far this has not been necessary. From 1afa191cbd1947c27dd213061e6d76ca6dad3ba6 Mon Sep 17 00:00:00 2001 From: Ian Thomas Date: Tue, 12 May 2026 08:40:43 +0100 Subject: [PATCH 2/3] Store response in stream --- src/wasm/stream.cpp | 36 +++++++++++++++++------------------- src/wasm/stream.hpp | 2 ++ 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/wasm/stream.cpp b/src/wasm/stream.cpp index 2662805..3aeb3cc 100644 --- a/src/wasm/stream.cpp +++ b/src/wasm/stream.cpp @@ -309,8 +309,7 @@ static void delete_request(wasm_http_stream* stream) } } -static int -read(wasm_http_stream* stream, read_buffer_t& read_buffer, wasm_http_response& response, bool is_read_response) +static int read(wasm_http_stream* stream, read_buffer_t& read_buffer, bool is_read_response) { if (is_read_response) { @@ -344,6 +343,7 @@ read(wasm_http_stream* stream, read_buffer_t& read_buffer, wasm_http_response& r } } + auto& response = stream->m_response; const char* status_text = nullptr; const char* response_headers = nullptr; @@ -429,7 +429,7 @@ static int write(wasm_http_stream* stream, const char* buffer, size_t buffer_siz // C credential functions. -static int create_credential(wasm_http_stream* stream, const wasm_http_response& response) +static int create_credential(wasm_http_stream* stream) { wasm_http_subtransport* subtransport = stream->m_subtransport; @@ -442,7 +442,7 @@ static int create_credential(wasm_http_stream* stream, const wasm_http_response& subtransport->m_authorization_header = ""; // Check that response headers show support for 'www-authenticate: Basic'. - if (!response.has_header_starts_with("www-authenticate", "Basic")) + if (!stream->m_response.has_header_starts_with("www-authenticate", "Basic")) { git_error_set( GIT_ERROR_HTTP, @@ -544,36 +544,35 @@ int wasm_http_stream_read(git_smart_subtransport_stream* s, char* buffer, size_t { wasm_http_stream* stream = reinterpret_cast(s); read_buffer_t read_buffer(buffer, buffer_size, bytes_read); - wasm_http_response response; bool send = true; while (send) { - if (read(stream, read_buffer, response, false) == static_cast(-1)) + if (read(stream, read_buffer, false) == static_cast(-1)) { return -1; // git error already set. } send = false; - auto final_url_header = response.get_header("x-final-url"); + auto final_url_header = stream->m_response.get_header("x-final-url"); if (final_url_header.has_value() && stream->ensure_final_url(final_url_header.value()) - && response.m_status != GIT_HTTP_STATUS_OK) + && stream->m_response.m_status != GIT_HTTP_STATUS_OK) { // Resend only if status not OK, if OK next request will use updated URL. send = true; } - if (response.has_header("strict-transport-security") && stream->ensure_https() - && response.m_status != GIT_HTTP_STATUS_OK) + if (stream->m_response.has_header("strict-transport-security") && stream->ensure_https() + && stream->m_response.m_status != GIT_HTTP_STATUS_OK) { // Resend only if status not OK, if OK next request will use https not http. send = true; } - if (response.m_status == GIT_HTTP_STATUS_UNAUTHORIZED) + if (stream->m_response.m_status == GIT_HTTP_STATUS_UNAUTHORIZED) { // Request and create new credentials. - if (create_credential(stream, response) < 0) + if (create_credential(stream) < 0) { return -1; // git error already set. } @@ -583,13 +582,13 @@ int wasm_http_stream_read(git_smart_subtransport_stream* s, char* buffer, size_t if (send) { delete_request(stream); - response.clear(); + stream->m_response.clear(); } } - if (response.m_status != GIT_HTTP_STATUS_OK) + if (stream->m_response.m_status != GIT_HTTP_STATUS_OK) { - response.set_git_error(stream->m_unconverted_url); + stream->m_response.set_git_error(stream->m_unconverted_url); return -1; } @@ -601,15 +600,14 @@ int wasm_http_stream_read_response(git_smart_subtransport_stream* s, char* buffe wasm_http_stream* stream = reinterpret_cast(s); read_buffer_t read_buffer(buffer, buffer_size, bytes_read); - wasm_http_response response; - int error = read(stream, read_buffer, response, true); + int error = read(stream, read_buffer, true); // May need similar handling of response status and headers as occurs in read() above, but so // far this has not been necessary. - if (error == 0 && response.m_status != GIT_HTTP_STATUS_OK) + if (error == 0 && stream->m_response.m_status != GIT_HTTP_STATUS_OK) { - response.set_git_error(stream->m_unconverted_url); + stream->m_response.set_git_error(stream->m_unconverted_url); error = -1; } diff --git a/src/wasm/stream.hpp b/src/wasm/stream.hpp index a17c6f0..712a35a 100644 --- a/src/wasm/stream.hpp +++ b/src/wasm/stream.hpp @@ -5,6 +5,7 @@ # include # include "libgit2_internals.hpp" +# include "response.hpp" # include "subtransport.hpp" // A stream represents a single http/https request. @@ -26,6 +27,7 @@ struct wasm_http_stream http_service m_service; std::string m_unconverted_url; int m_request_index; + wasm_http_response m_response; }; void wasm_http_stream_free(git_smart_subtransport_stream* s); From f2ef4fb04c06d41612f8184a0ac5fa5ebce1e4fe Mon Sep 17 00:00:00 2001 From: Ian Thomas Date: Tue, 12 May 2026 10:38:00 +0100 Subject: [PATCH 3/3] Show prompt for large downloads --- src/wasm/response.cpp | 4 ++ src/wasm/response.hpp | 2 + src/wasm/stream.cpp | 92 +++++++++++++++++++++++++++---------------- src/wasm/utils.cpp | 42 ++++++++++++++++++++ src/wasm/utils.hpp | 4 ++ 5 files changed, 109 insertions(+), 35 deletions(-) diff --git a/src/wasm/response.cpp b/src/wasm/response.cpp index 620a4c2..af391f8 100644 --- a/src/wasm/response.cpp +++ b/src/wasm/response.cpp @@ -7,6 +7,8 @@ wasm_http_response::wasm_http_response() : m_status(0) + , m_read_count(0) + , m_total_bytes(0) { } @@ -19,6 +21,8 @@ void wasm_http_response::clear() { m_status = 0; m_status_text.clear(); + m_read_count = 0; + m_total_bytes = 0; m_response_headers.clear(); } diff --git a/src/wasm/response.hpp b/src/wasm/response.hpp index 84d96e5..127b705 100644 --- a/src/wasm/response.hpp +++ b/src/wasm/response.hpp @@ -31,6 +31,8 @@ class wasm_http_response int32_t m_status; // Specific type corresponding to i32 in emscripten setValue call. std::string m_status_text; + unsigned int m_read_count; + int64_t m_total_bytes; // Specific type corresponding to i64 in emscripten setValue call. private: diff --git a/src/wasm/stream.cpp b/src/wasm/stream.cpp index 3aeb3cc..b16dcc3 100644 --- a/src/wasm/stream.cpp +++ b/src/wasm/stream.cpp @@ -11,6 +11,7 @@ # include "constants.hpp" # include "read_buffer.hpp" # include "response.hpp" +# include "utils.hpp" // Buffer size used in transport_smart, hardcoded in libgit2. # define EMFORGE_BUFSIZE 65536 @@ -158,8 +159,10 @@ EM_JS( (int request_index, char* buffer, size_t buffer_size, + bool first_read, // Set subsequent arguments only if this is true. int32_t* status, const char** status_text, + int64_t* total_bytes, const char** response_headers), { try @@ -174,22 +177,28 @@ EM_JS( request.content = null; } - let bytes_read = 0; + let byte_length = 0; // Total number of bytes that will be returned. + let bytes_read = 0; // Number of bytes returned by this call. if (xhr.response && xhr.response.byteLength) { - bytes_read = xhr.response.byteLength - request.result_buffer_pointer; + byte_length = xhr.response.byteLength; + bytes_read = byte_length - request.result_buffer_pointer; if (bytes_read > buffer_size) { bytes_read = buffer_size; } } - // Caller must delete the returned status_text and response_headers. - // clang-format off - setValue(status, xhr.status, 'i32*'); - setValue(status_text, stringToNewUTF8(xhr.statusText ?? ""), 'i8**'); - setValue(response_headers, stringToNewUTF8(xhr.getAllResponseHeaders() ?? ""), 'i8**'); - // clang-format on + if (first_read) + { + // Caller must delete the returned status_text and response_headers. + // clang-format off + setValue(status, xhr.status, 'i32*'); + setValue(status_text, stringToNewUTF8(xhr.statusText ?? ""), 'i8**'); + setValue(total_bytes, byte_length, 'i64*'); + setValue(response_headers, stringToNewUTF8(xhr.getAllResponseHeaders() ?? ""), 'i8**'); + // clang-format on + } if (bytes_read > 0) { @@ -344,6 +353,7 @@ static int read(wasm_http_stream* stream, read_buffer_t& read_buffer, bool is_re } auto& response = stream->m_response; + bool first_read = response.m_read_count == 0; const char* status_text = nullptr; const char* response_headers = nullptr; @@ -352,10 +362,13 @@ static int read(wasm_http_stream* stream, read_buffer_t& read_buffer, bool is_re stream->m_request_index, read_buffer.m_buffer, read_buffer.m_buffer_size, + first_read, &response.m_status, &status_text, + &response.m_total_bytes, &response_headers ); + stream->m_response.m_read_count++; if (bytes_read == static_cast(-1)) { convert_js_to_git_error(stream); @@ -365,38 +378,47 @@ static int read(wasm_http_stream* stream, read_buffer_t& read_buffer, bool is_re return -1; } - response.m_status_text = status_text; - delete status_text; // Delete const char* allocated in JavaScript. - - // Split single string with response headers separated by \r\n into individual headers. - auto lines = split_input_at_newlines(response_headers); - for (const auto& line : lines) + if (first_read) { - auto pos = line.find(":"); - if (pos == std::string::npos) + response.m_status_text = status_text; + delete status_text; // Delete const char* allocated in JavaScript. + + // Split single string with response headers separated by \r\n into individual headers. + auto lines = split_input_at_newlines(response_headers); + for (const auto& line : lines) { - // Skip invalid lines. Should this be an error condition? - continue; + auto pos = line.find(":"); + if (pos == std::string::npos) + { + // Skip invalid lines. Should this be an error condition? + continue; + } + response.add_header(line.substr(0, pos), line.substr(pos + 1)); } - response.add_header(line.substr(0, pos), line.substr(pos + 1)); - } - delete response_headers; // Delete const char* allocated in JavaScript. + delete response_headers; // Delete const char* allocated in JavaScript. - // If successful, check expected response content-type is correct. - if (response.m_status == GIT_HTTP_STATUS_OK) - { - auto expected_response_type = stream->m_service.m_response_type; - if (!expected_response_type.empty() - && !response.has_header_matches("content-type", expected_response_type)) + // If successful, check expected response content-type is correct. + if (response.m_status == GIT_HTTP_STATUS_OK) { - // Not sure this should be checked at all, as CORS proxy may be doing something - // with it. - git_error_set( - GIT_ERROR_HTTP, - "expected response content-type header '%s' to request %s", - expected_response_type.c_str(), - stream->m_unconverted_url.c_str() - ); + auto expected_response_type = stream->m_service.m_response_type; + if (!expected_response_type.empty() + && !response.has_header_matches("content-type", expected_response_type)) + { + // Not sure this should be checked at all, as CORS proxy may be doing something + // with it. + git_error_set( + GIT_ERROR_HTTP, + "expected response content-type header '%s' to request %s", + expected_response_type.c_str(), + stream->m_unconverted_url.c_str() + ); + return -1; + } + } + + if (!maybe_prompt_to_download(response.m_total_bytes)) + { + git_error_set(GIT_ERROR_NONE, "Download aborted"); return -1; } } diff --git a/src/wasm/utils.cpp b/src/wasm/utils.cpp index 7cc34e5..3027326 100644 --- a/src/wasm/utils.cpp +++ b/src/wasm/utils.cpp @@ -2,8 +2,14 @@ # include "utils.hpp" +# include +# include +# include +# include + # include +# include "../utils/input_output.hpp" # include "constants.hpp" unsigned long get_request_timeout_ms() @@ -35,4 +41,40 @@ unsigned long get_request_timeout_ms() return 1000 * timeout_seconds; } +std::string human_readable_size(double bytes) +{ + static constexpr const char* units[] = {"B", "kB", "MB", "GB", "TB"}; + int nunits = sizeof(units) / sizeof(units[0]); + int index = std::trunc(std::log10(bytes) / 3); + index = std::clamp(index, 0, nunits - 1, std::less()); + + std::ostringstream oss; + oss << std::fixed << std::setprecision(1) << (bytes / std::pow(1000.0, index)) << ' ' << units[index]; + return oss.str(); +} + +bool maybe_prompt_to_download(double bytes) +{ + if (bytes <= 0.0) + { + return true; + } + + // Limit + double bytes_limit = 1e7; // 10 MB + if (bytes > bytes_limit) + { + if (!termcolor::_internal::is_atty(std::cout)) + { + // If not a tty cannot prompt user so prevent the download. + return false; + } + + std::string prompt = "Download size is " + human_readable_size(bytes) + ", continue [y/N]? "; + return prompt_yes_or_no(prompt, false); + } + + return true; +} + #endif // EMSCRIPTEN diff --git a/src/wasm/utils.hpp b/src/wasm/utils.hpp index 2325479..ace9191 100644 --- a/src/wasm/utils.hpp +++ b/src/wasm/utils.hpp @@ -5,4 +5,8 @@ // Get wasm http request timeout in milliseconds from environment variable or default value. unsigned long get_request_timeout_ms(); +// Return true if OK to continue with download, false otherwise. +// May or may not prompt the user for input depending on the size of the download. +bool maybe_prompt_to_download(double bytes); + #endif // EMSCRIPTEN