dmaax/ladybird
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

LibHTTP: Publish disk cache entries by rename

Browse source 
Write cache entries to a temporary file and rename them over the final
path only after the entry has been fully flushed. This keeps previously
mapped cache bodies tied to their original inode instead of exposing
them to truncation when the same cache key is replaced.

Clean up only the temporary file for incomplete writers so a failed
replacement does not remove the still-valid existing cache entry. Add
coverage that replaces an entry while an old body mapping remains live
and verifies that it still exposes the original bytes.
This commit is contained in:
Andreas Kling 2026-05-15 18:40:14 +02:00 committed by Andreas Kling
parent 7e7d57fb1a
commit efa7c1ec97
3 changed files with 56 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

33
Libraries/LibHTTP/Cache/CacheEntry.cpp
View file

@ -119,6 +119,7 @@ CacheEntryWriter::CacheEntryWriter(DiskCache& disk_cache, CacheIndex& index, u64
ErrorOr<void> CacheEntryWriter::write_status_and_reason(u32 status_code, Optional<String> reason_phrase, HeaderList const& request_headers, HeaderList const& response_headers)
{
if (m_marked_for_deletion) {
remove_incomplete_temporary_file();
close_and_destroy_cache_entry();
return Error::from_string_literal("Cache entry has been deleted");
}
@ -137,6 +138,7 @@ ErrorOr<void> CacheEntryWriter::write_status_and_reason(u32 status_code, Optiona
m_vary_key = create_vary_key(request_headers, response_headers);
m_path = path_for_cache_entry(m_disk_cache.cache_directory(), m_cache_key, m_vary_key);
m_temporary_path = LexicalPath::join(m_disk_cache.cache_directory().string(), ByteString::formatted("{}.tmp", m_path->basename()));
auto freshness_lifetime = calculate_freshness_lifetime(status_code, response_headers, m_current_time_offset_for_testing);
auto current_age = calculate_age(response_headers, m_request_time, m_response_time, m_current_time_offset_for_testing);
@ -146,7 +148,8 @@ ErrorOr<void> CacheEntryWriter::write_status_and_reason(u32 status_code, Optiona
if (cache_lifetime_status(request_headers, response_headers, freshness_lifetime, current_age) == CacheLifetimeStatus::Expired)
return Error::from_string_literal("Response has already expired");
auto unbuffered_file = TRY(Core::File::open(m_path->string(), Core::File::OpenMode::Write));
(void)FileSystem::remove(m_temporary_path->string(), FileSystem::RecursionMode::Disallowed);
auto unbuffered_file = TRY(Core::File::open(m_temporary_path->string(), Core::File::OpenMode::Write | Core::File::OpenMode::MustBeNew));
m_file = TRY(Core::OutputBufferedFile::create(move(unbuffered_file)));
TRY(m_file->write_value(m_cache_header));
@ -161,7 +164,7 @@ ErrorOr<void> CacheEntryWriter::write_status_and_reason(u32 status_code, Optiona
if (result.is_error()) {
dbgln_if(HTTP_DISK_CACHE_DEBUG, "\033[36m[disk]\033[0m \033[31;1mUnable to write status/reason to cache entry for\033[0m {}: {}", m_url, result.error());
remove();
remove_incomplete_temporary_file();
close_and_destroy_cache_entry();
return result.release_error();
@ -173,6 +176,7 @@ ErrorOr<void> CacheEntryWriter::write_status_and_reason(u32 status_code, Optiona
ErrorOr<void> CacheEntryWriter::write_data(ReadonlyBytes data)
{
if (m_marked_for_deletion) {
remove_incomplete_temporary_file();
close_and_destroy_cache_entry();
return Error::from_string_literal("Cache entry has been deleted");
}
@ -180,7 +184,7 @@ ErrorOr<void> CacheEntryWriter::write_data(ReadonlyBytes data)
if (auto result = m_file->write_until_depleted(data); result.is_error()) {
dbgln_if(HTTP_DISK_CACHE_DEBUG, "\033[36m[disk]\033[0m \033[31;1mUnable to write data to cache entry for\033[0m {}: {}", m_url, result.error());
remove();
remove_incomplete_temporary_file();
close_and_destroy_cache_entry();
return result.release_error();
@ -206,19 +210,29 @@ ErrorOr<void> CacheEntryWriter::flush_impl(NonnullRefPtr<HeaderList> request_hea
{
ScopeGuard guard { [&]() { close_and_destroy_cache_entry(); } };
if (m_marked_for_deletion)
if (m_marked_for_deletion) {
remove_incomplete_temporary_file();
return Error::from_string_literal("Cache entry has been deleted");
}
VERIFY(m_path.has_value());
VERIFY(m_temporary_path.has_value());
ArmedScopeGuard remove_temporary_file = [&]() {
remove_incomplete_temporary_file();
};
m_cache_footer.header_hash = m_cache_header.hash();
if (auto result = m_file->write_value(m_cache_footer); result.is_error()) {
dbgln_if(HTTP_DISK_CACHE_DEBUG, "\033[36m[disk]\033[0m \033[31;1mUnable to flush cache entry for\033[0m {}: {}", m_url, result.error());
remove();
return result.release_error();
}
TRY(m_file->flush_buffer());
m_file.clear();
TRY(Core::System::rename(m_temporary_path->string(), m_path->string()));
remove_temporary_file.disarm();
int body_fd = -1;
ArmedScopeGuard close_body_fd = [&] {
@ -254,10 +268,17 @@ ErrorOr<void> CacheEntryWriter::flush_impl(NonnullRefPtr<HeaderList> request_hea
void CacheEntryWriter::remove_incomplete_entry()
{
remove();
remove_incomplete_temporary_file();
close_and_destroy_cache_entry();
}
void CacheEntryWriter::remove_incomplete_temporary_file()
{
if (!m_temporary_path.has_value())
return;
(void)FileSystem::remove(m_temporary_path->string(), FileSystem::RecursionMode::Disallowed);
}
ErrorOr<NonnullOwnPtr<CacheEntryReader>> CacheEntryReader::create(DiskCache& disk_cache, CacheIndex& index, u64 cache_key, u64 vary_key, NonnullRefPtr<HeaderList> response_headers, u64 data_size)
{
auto path = path_for_cache_entry(disk_cache.cache_directory(), cache_key, vary_key);

2
Libraries/LibHTTP/Cache/CacheEntry.h
View file

@ -107,8 +107,10 @@ private:
CacheEntryWriter(DiskCache&, CacheIndex&, u64 cache_key, String url, CacheHeader, UnixDateTime request_time, AK::Duration current_time_offset_for_testing);
ErrorOr<void> flush_impl(NonnullRefPtr<HeaderList> request_headers, NonnullRefPtr<HeaderList> response_headers, CacheEntryBodyFile*);
void remove_incomplete_temporary_file();
OwnPtr<Core::OutputBufferedFile> m_file;
Optional<LexicalPath> m_temporary_path;
u64 m_data_offset { 0 };
UnixDateTime m_request_time;

27
Tests/LibHTTP/TestDiskCache.cpp
View file

@ -135,6 +135,33 @@ TEST_CASE(flush_returns_mappable_body_file)
EXPECT_EQ(body.bytes(), "console.log('hello');"sv.bytes());
}
TEST_CASE(replacing_cache_entry_keeps_existing_body_mapping_stable)
{
auto disk_cache = MUST(HTTP::DiskCache::create(HTTP::DiskCache::Mode::Testing));
TestCacheRequest request;
auto url = parse_url("https://example.com/script.js"sv);
auto request_headers = create_cacheable_request_headers();
auto response_headers = create_cacheable_response_headers();
auto& writer = create_cache_entry(disk_cache, request, url, *request_headers);
TRY_OR_FAIL(writer.write_status_and_reason(200, "OK"_string, *request_headers, *response_headers));
TRY_OR_FAIL(writer.write_data("console.log('old');"sv.bytes()));
auto old_body_file = TRY_OR_FAIL(writer.flush_and_take_body_file(request_headers, response_headers));
auto old_body = TRY_OR_FAIL(Core::ImmutableBytes::map_from_fd_range_and_close(old_body_file.fd, "old cache body"sv, old_body_file.offset, old_body_file.size));
EXPECT_EQ(old_body.bytes(), "console.log('old');"sv.bytes());
auto replacement_request_headers = create_cacheable_request_headers();
auto replacement_response_headers = create_cacheable_response_headers();
auto& replacement_writer = create_cache_entry(disk_cache, request, url, *replacement_request_headers);
TRY_OR_FAIL(replacement_writer.write_status_and_reason(200, "OK"_string, *replacement_request_headers, *replacement_response_headers));
TRY_OR_FAIL(replacement_writer.write_data("console.log('new');"sv.bytes()));
TRY_OR_FAIL(replacement_writer.flush(replacement_request_headers, replacement_response_headers));
EXPECT_EQ(old_body.bytes(), "console.log('old');"sv.bytes());
}
TEST_CASE(associated_data_round_trips_with_explicit_vary_key)
{
auto disk_cache = MUST(HTTP::DiskCache::create(HTTP::DiskCache::Mode::Testing));