Skip to content

Commit 16c24e7

Browse files
Yundi339Yundi339
committed
refactor: enhance FileCache with safe fallback in prepend_header and add max_file_size setter
1 parent a9790f6 commit 16c24e7

3 files changed

Lines changed: 76 additions & 62 deletions

File tree

‎http/server/FileCache.cpp‎

Lines changed: 67 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@
55
#include "htime.h"
66
#include "hlog.h"
77

8-
#include "httpdef.h" // import http_content_type_str_by_suffix
8+
#include "httpdef.h" // import http_content_type_str_by_suffix
99
#include "http_page.h" // import make_index_of_page
1010

1111
#ifdef OS_WIN
12-
#include "hstring.h" // import hv::utf8_to_wchar
12+
#include "hstring.h" // import hv::utf8_to_wchar
1313
#endif
1414

1515
#define ETAG_FMT "\"%zx-%zx\""
@@ -56,6 +56,7 @@ file_cache_ptr FileCache::Open(const char* filepath, OpenParam* param) {
5656
flags |= O_BINARY;
5757
#endif
5858
int fd = -1;
59+
bool is_dir = false;
5960
#ifdef OS_WIN
6061
if (wfilepath.empty()) wfilepath = hv::utf8_to_wchar(filepath);
6162
if (_wstat(wfilepath.c_str(), (struct _stat*)&st) != 0) {
@@ -65,7 +66,7 @@ file_cache_ptr FileCache::Open(const char* filepath, OpenParam* param) {
6566
if (S_ISREG(st.st_mode)) {
6667
fd = _wopen(wfilepath.c_str(), flags);
6768
} else if (S_ISDIR(st.st_mode)) {
68-
fd = 0;
69+
is_dir = true;
6970
}
7071
#else
7172
if (::stat(filepath, &st) != 0) {
@@ -74,15 +75,11 @@ file_cache_ptr FileCache::Open(const char* filepath, OpenParam* param) {
7475
}
7576
fd = open(filepath, flags);
7677
#endif
77-
if (fd < 0) {
78+
if (fd < 0 && !is_dir) {
7879
param->error = ERR_OPEN_FILE;
7980
return NULL;
8081
}
81-
#ifdef OS_WIN
82-
defer(if (fd > 0) { close(fd); }) // fd=0 is Windows directory sentinel
83-
#else
84-
defer(close(fd);)
85-
#endif
82+
defer(if (fd >= 0) { close(fd); })
8683
if (fc == NULL) {
8784
if (S_ISREG(st.st_mode) ||
8885
(S_ISDIR(st.st_mode) &&
@@ -100,61 +97,67 @@ file_cache_ptr FileCache::Open(const char* filepath, OpenParam* param) {
10097
return NULL;
10198
}
10299
}
103-
// Hold fc->mutex for the remainder of initialization
104-
std::lock_guard<std::mutex> lock(fc->mutex);
105-
if (S_ISREG(fc->st.st_mode)) {
106-
param->filesize = fc->st.st_size;
107-
// FILE
108-
if (param->need_read) {
109-
if (fc->st.st_size > param->max_read) {
110-
param->error = ERR_OVER_LIMIT;
111-
// Don't cache incomplete entries
112-
return NULL;
113-
}
114-
fc->resize_buf(fc->st.st_size, max_header_length);
115-
// Loop to handle partial reads (EINTR, etc.)
116-
char* dst = fc->filebuf.base;
117-
size_t remaining = fc->filebuf.len;
118-
while (remaining > 0) {
119-
ssize_t nread = read(fd, dst, remaining);
120-
if (nread < 0) {
121-
if (errno == EINTR) continue;
122-
hloge("Failed to read file: %s", filepath);
123-
param->error = ERR_READ_FILE;
100+
// Hold fc->mutex for initialization, but release before put()
101+
// to avoid lock-order inversion with RemoveExpiredFileCache().
102+
// Lock order: LRUCache mutex → fc->mutex (never reverse).
103+
{
104+
std::lock_guard<std::mutex> lock(fc->mutex);
105+
// Sync local stat result into cached entry
106+
fc->st = st;
107+
if (S_ISREG(fc->st.st_mode)) {
108+
param->filesize = fc->st.st_size;
109+
// FILE
110+
if (param->need_read) {
111+
if (fc->st.st_size > param->max_read) {
112+
param->error = ERR_OVER_LIMIT;
113+
// Don't cache incomplete entries
124114
return NULL;
125115
}
126-
if (nread == 0) {
127-
hloge("Unexpected EOF reading file: %s", filepath);
128-
param->error = ERR_READ_FILE;
129-
return NULL;
116+
fc->resize_buf(fc->st.st_size, max_header_length);
117+
// Loop to handle partial reads (EINTR, etc.)
118+
char* dst = fc->filebuf.base;
119+
size_t remaining = fc->filebuf.len;
120+
while (remaining > 0) {
121+
ssize_t nread = read(fd, dst, remaining);
122+
if (nread < 0) {
123+
if (errno == EINTR) continue;
124+
hloge("Failed to read file: %s", filepath);
125+
param->error = ERR_READ_FILE;
126+
return NULL;
127+
}
128+
if (nread == 0) {
129+
hloge("Unexpected EOF reading file: %s", filepath);
130+
param->error = ERR_READ_FILE;
131+
return NULL;
132+
}
133+
dst += nread;
134+
remaining -= nread;
130135
}
131-
dst += nread;
132-
remaining -= nread;
133136
}
134-
}
135-
const char* suffix = strrchr(filepath, '.');
136-
if (suffix) {
137-
http_content_type content_type = http_content_type_enum_by_suffix(suffix + 1);
138-
if (content_type == TEXT_HTML) {
139-
fc->content_type = "text/html; charset=utf-8";
140-
} else if (content_type == TEXT_PLAIN) {
141-
fc->content_type = "text/plain; charset=utf-8";
142-
} else {
143-
fc->content_type = http_content_type_str_by_suffix(suffix + 1);
137+
const char* suffix = strrchr(filepath, '.');
138+
if (suffix) {
139+
http_content_type content_type = http_content_type_enum_by_suffix(suffix + 1);
140+
if (content_type == TEXT_HTML) {
141+
fc->content_type = "text/html; charset=utf-8";
142+
} else if (content_type == TEXT_PLAIN) {
143+
fc->content_type = "text/plain; charset=utf-8";
144+
} else {
145+
fc->content_type = http_content_type_str_by_suffix(suffix + 1);
146+
}
144147
}
148+
} else if (S_ISDIR(fc->st.st_mode)) {
149+
// DIR
150+
std::string page;
151+
make_index_of_page(filepath, page, param->path);
152+
fc->resize_buf(page.size(), max_header_length);
153+
memcpy(fc->filebuf.base, page.c_str(), page.size());
154+
fc->content_type = "text/html; charset=utf-8";
145155
}
146-
} else if (S_ISDIR(fc->st.st_mode)) {
147-
// DIR
148-
std::string page;
149-
make_index_of_page(filepath, page, param->path);
150-
fc->resize_buf(page.size(), max_header_length);
151-
memcpy(fc->filebuf.base, page.c_str(), page.size());
152-
fc->content_type = "text/html; charset=utf-8";
153-
}
154-
gmtime_fmt(fc->st.st_mtime, fc->last_modified);
155-
snprintf(fc->etag, sizeof(fc->etag), ETAG_FMT,
156-
(size_t)fc->st.st_mtime, (size_t)fc->st.st_size);
157-
// Cache the fully initialized entry
156+
gmtime_fmt(fc->st.st_mtime, fc->last_modified);
157+
snprintf(fc->etag, sizeof(fc->etag), ETAG_FMT,
158+
(size_t)fc->st.st_mtime, (size_t)fc->st.st_size);
159+
} // release fc->mutex before put() to maintain lock ordering
160+
// Cache the fully initialized entry (acquires LRUCache mutex only)
158161
put(filepath, fc);
159162
}
160163
return fc;
@@ -179,7 +182,12 @@ file_cache_ptr FileCache::Get(const char* filepath) {
179182
void FileCache::RemoveExpiredFileCache() {
180183
time_t now = time(NULL);
181184
remove_if([this, now](const std::string& filepath, const file_cache_ptr& fc) {
182-
std::lock_guard<std::mutex> lock(fc->mutex);
185+
// Use try_to_lock to avoid lock-order inversion with Open().
186+
// If the entry is busy, skip it — it will be checked next cycle.
187+
std::unique_lock<std::mutex> lock(fc->mutex, std::try_to_lock);
188+
if (!lock.owns_lock()) {
189+
return false;
190+
}
183191
return (now - fc->stat_time > expired_time);
184192
});
185193
}

‎http/server/FileCache.h‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,14 @@ typedef struct file_cache_s {
8787

8888
// Thread-safe: prepend header into reserved space.
8989
// Returns true on success, false if header exceeds reserved space.
90+
// On failure, httpbuf falls back to filebuf (body only, no header).
9091
bool prepend_header(const char* header, int len) {
9192
std::lock_guard<std::mutex> lock(mutex);
92-
if (len <= 0 || len > header_reserve) return false;
93+
if (len <= 0 || len > header_reserve) {
94+
// Safe fallback: point httpbuf at filebuf so callers always get valid data
95+
httpbuf = filebuf;
96+
return false;
97+
}
9398
httpbuf.base = filebuf.base - len;
9499
httpbuf.len = (size_t)len + filebuf.len;
95100
memcpy(httpbuf.base, header, len);
@@ -144,8 +149,8 @@ class HV_EXPORT FileCache : public hv::LRUCache<std::string, file_cache_ptr> {
144149
int GetExpiredTime() const { return expired_time; }
145150

146151
// --- new: setters ---
147-
void SetMaxHeaderLength(int len) { max_header_length = len; }
148-
void SetMaxFileSize(int size) { max_file_size = size; }
152+
void SetMaxHeaderLength(int len) { max_header_length = len < 0 ? 0 : len; }
153+
void SetMaxFileSize(int size) { max_file_size = size < 1 ? 1 : size; }
149154

150155
protected:
151156
file_cache_ptr Get(const char* filepath);

‎http/server/HttpServer.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ static void loop_thread(void* userdata) {
136136
FileCache* filecache = &privdata->filecache;
137137
filecache->stat_interval = service->file_cache_stat_interval;
138138
filecache->expired_time = service->file_cache_expired_time;
139+
filecache->max_file_size = service->max_file_cache_size;
139140
if (filecache->expired_time > 0) {
140141
// NOTE: add timer to remove expired file cache
141142
htimer_t* timer = htimer_add(hloop, [](htimer_t* timer) {

0 commit comments

Comments
 (0)