diff --git a/changelog.d/3077-sendfile-absolute-directory.fixed.md b/changelog.d/3077-sendfile-absolute-directory.fixed.md new file mode 100644 index 0000000000..17bee351ae --- /dev/null +++ b/changelog.d/3077-sendfile-absolute-directory.fixed.md @@ -0,0 +1 @@ +- `sendFile()`: an absolute `directory` argument is now used verbatim instead of being post-processed, honouring the documented "must be a full path … outside of the web root" contract. Previously the resolver ran the caller-supplied absolute path through the `/wheels` mapping rewrite — substring-hijacking any directory containing `/wheels` (e.g. `/var/www/wheels/uploads`) on every engine — and through an `ExpandPath()` fallback that web-root-prefixed the path on Adobe CF, so serving files from outside the web root threw `Wheels.FileNotFound` there. The verbatim branch only engages when the directory actually exists on disk, so the long-standing webroot-relative idiom (`directory="/reports/"`) keeps resolving against the web root via `ExpandPath()`. Relative `filePath`-based resolution and the `..`-traversal guard are unchanged (#3077) diff --git a/vendor/wheels/controller/miscellaneous.cfc b/vendor/wheels/controller/miscellaneous.cfc index 18985539cc..127b67d0fd 100644 --- a/vendor/wheels/controller/miscellaneous.cfc +++ b/vendor/wheels/controller/miscellaneous.cfc @@ -260,34 +260,62 @@ component { if (!Len(local.folder)) { local.folder = local.relativeRoot & $get("filePath"); } - if (Left(local.folder, Len(local.root)) == local.root) { - local.folder = RemoveChars(local.folder, 1, Len(local.root)); + + // https://github.com/wheels-dev/wheels/issues/3077 — when the caller supplies an + // absolute `directory` (the documented "must be a full path … outside of the web + // root" contract) use it verbatim: build `fullPath` from the directory + file and + // skip both the `/wheels` mapping rewrite and the `ExpandPath()` fallback below. + // Those rewrites assume a relative, mapping-based path and otherwise (1) web-root- + // prefix the absolute path on Adobe CF — where `ExpandPath()` resolves against the + // web root rather than returning an absolute path unchanged as Lucee does — and + // (2) substring-hijack any directory containing "/wheels" (e.g. /var/www/wheels). + // The verbatim branch is additionally gated on `DirectoryExists()`: a leading "/" + // is also the long-standing webroot-relative idiom (`directory="/reports/"`), so a + // root-anchored path that does NOT exist on disk falls through to the legacy + // `ExpandPath()` resolution below and keeps resolving against the web root. + // The `..`-traversal guard above still applies to both arguments. + local.normalizedDir = Replace(arguments.directory, "\", "/", "all"); + if (Len(local.normalizedDir) > 1 && Right(local.normalizedDir, 1) == "/") { + local.normalizedDir = Left(local.normalizedDir, Len(local.normalizedDir) - 1); } - local.fullPath = Replace(local.folder, "\", "/", "all"); - local.fullPath = ListAppend(local.fullPath, arguments.file, "/"); - // https://github.com/wheels-dev/wheels/issues/873 Don't expand path if already contains root - if (local.fullPath DOES NOT CONTAIN Replace(local.root, "\", "/", "all")) { - //added this section for the "/wheels" mapping to work correctly - if (local.fullPath CONTAINS "/wheels") { - local.startPos = findNoCase("/wheels", local.fullPath); - - // Prefer /vendor/wheels if available - local.vendorWheelsPos = findNoCase("/vendor/wheels/", local.fullPath); - if (local.vendorWheelsPos > 0) { - local.startPos = local.vendorWheelsPos + len("/vendor"); - } + local.isAbsoluteDirectory = Len(local.normalizedDir) + && (Left(local.normalizedDir, 1) == "/" || REFind("^[A-Za-z]:", local.normalizedDir)) + && DirectoryExists(local.normalizedDir); + + if (local.isAbsoluteDirectory) { + local.directory = local.normalizedDir; + local.file = arguments.file; + local.fullPath = local.directory & "/" & local.file; + } else { + if (Left(local.folder, Len(local.root)) == local.root) { + local.folder = RemoveChars(local.folder, 1, Len(local.root)); + } + local.fullPath = Replace(local.folder, "\", "/", "all"); + local.fullPath = ListAppend(local.fullPath, arguments.file, "/"); + // https://github.com/wheels-dev/wheels/issues/873 Don't expand path if already contains root + if (local.fullPath DOES NOT CONTAIN Replace(local.root, "\", "/", "all")) { + //added this section for the "/wheels" mapping to work correctly + if (local.fullPath CONTAINS "/wheels") { + local.startPos = findNoCase("/wheels", local.fullPath); + + // Prefer /vendor/wheels if available + local.vendorWheelsPos = findNoCase("/vendor/wheels/", local.fullPath); + if (local.vendorWheelsPos > 0) { + local.startPos = local.vendorWheelsPos + len("/vendor"); + } - if (local.startPos > 0) { - local.fullPath = ExpandPath(mid(local.fullPath, local.startPos, len(local.fullPath) - local.startPos + 1)); + if (local.startPos > 0) { + local.fullPath = ExpandPath(mid(local.fullPath, local.startPos, len(local.fullPath) - local.startPos + 1)); + local.fullPath = Replace(local.fullPath, "\", "/", "all"); + local.file = ListLast(local.fullPath, "/"); + local.directory = Reverse(ListRest(Reverse(local.fullPath), "/")); + } + } else{ + local.fullPath = ExpandPath(local.fullPath); local.fullPath = Replace(local.fullPath, "\", "/", "all"); local.file = ListLast(local.fullPath, "/"); local.directory = Reverse(ListRest(Reverse(local.fullPath), "/")); } - } else{ - local.fullPath = ExpandPath(local.fullPath); - local.fullPath = Replace(local.fullPath, "\", "/", "all"); - local.file = ListLast(local.fullPath, "/"); - local.directory = Reverse(ListRest(Reverse(local.fullPath), "/")); } } diff --git a/vendor/wheels/tests/specs/controller/miscellaneousSpec.cfc b/vendor/wheels/tests/specs/controller/miscellaneousSpec.cfc index abd9b5fd69..8b3407840d 100644 --- a/vendor/wheels/tests/specs/controller/miscellaneousSpec.cfc +++ b/vendor/wheels/tests/specs/controller/miscellaneousSpec.cfc @@ -208,6 +208,90 @@ component extends="wheels.WheelsTest" { expect(r.name).toBe("weirdname.png") }) + it("serves a file from an absolute directory outside the web root", () => { + // https://github.com/wheels-dev/wheels/issues/3077 — an absolute `directory` + // outside the web root must be used verbatim, not re-resolved via ExpandPath + // (which web-root-prefixes the path on Adobe CF). + local.outsideDir = GetTempDirectory() & "dlprobe3077_outside" + if (!DirectoryExists(local.outsideDir)) { + DirectoryCreate(local.outsideDir, true) + } + local.target = local.outsideDir & "/secret.txt" + FileWrite(local.target, "secret payload") + try { + args.file = "secret.txt" + args.directory = local.outsideDir + r = _controller.sendFile(argumentCollection = args) + + expect(Replace(r.file, "\", "/", "all")).toInclude("dlprobe3077_outside/secret.txt") + expect(r.name).toBe("secret.txt") + } finally { + if (FileExists(local.target)) { + FileDelete(local.target) + } + if (DirectoryExists(local.outsideDir)) { + DirectoryDelete(local.outsideDir, true) + } + } + }) + + it("does not rewrite an absolute directory containing the '/wheels' substring", () => { + // https://github.com/wheels-dev/wheels/issues/3077 — the `/wheels` mapping + // fallback substring-matched ANY absolute path containing "/wheels" + // (e.g. /var/www/wheels/uploads), silently rewriting it. + local.wheelsDir = GetTempDirectory() & "wheels3077-dl" + if (!DirectoryExists(local.wheelsDir)) { + DirectoryCreate(local.wheelsDir, true) + } + local.target = local.wheelsDir & "/secret.txt" + FileWrite(local.target, "secret payload") + try { + args.file = "secret.txt" + args.directory = local.wheelsDir + r = _controller.sendFile(argumentCollection = args) + + expect(Replace(r.file, "\", "/", "all")).toInclude("wheels3077-dl/secret.txt") + } finally { + if (FileExists(local.target)) { + FileDelete(local.target) + } + if (DirectoryExists(local.wheelsDir)) { + DirectoryDelete(local.wheelsDir, true) + } + } + }) + + it("still resolves a leading-slash webroot-relative directory against the web root", () => { + // Regression guard for the long-standing webroot-relative idiom + // (`directory="/reports/"`): a root-anchored path that does NOT exist on + // disk must fall through to the legacy `ExpandPath()` resolution instead + // of being treated as a verbatim filesystem path (which would miss the + // file and throw). On Adobe CF this idiom was historically the only + // working form of `directory`, so it must keep working. + local.relDir = "dlprobe3077_rel" + local.absDir = ExpandPath("/" & local.relDir) + if (!DirectoryExists(local.absDir)) { + DirectoryCreate(local.absDir, true) + } + local.target = local.absDir & "/report.txt" + FileWrite(local.target, "webroot-relative payload") + try { + args.file = "report.txt" + args.directory = "/" & local.relDir & "/" + r = _controller.sendFile(argumentCollection = args) + + expect(Replace(r.file, "\", "/", "all")).toInclude("dlprobe3077_rel/report.txt") + expect(r.name).toBe("report.txt") + } finally { + if (FileExists(local.target)) { + FileDelete(local.target) + } + if (DirectoryExists(local.absDir)) { + DirectoryDelete(local.absDir, true) + } + } + }) + it("is specifying a directory", () => { // Skip this test temporarily to debug in CI skip("Temporarily skipping to debug path issues in CI"); diff --git a/web/sites/guides/src/content/docs/v4-0-0/digging-deeper/file-uploads-and-downloads.mdx b/web/sites/guides/src/content/docs/v4-0-0/digging-deeper/file-uploads-and-downloads.mdx index b602605671..d100104844 100644 --- a/web/sites/guides/src/content/docs/v4-0-0/digging-deeper/file-uploads-and-downloads.mdx +++ b/web/sites/guides/src/content/docs/v4-0-0/digging-deeper/file-uploads-and-downloads.mdx @@ -197,7 +197,7 @@ component extends="Controller" { Signature: `sendFile(file, name, type, disposition, directory, deleteFile, deliver)`. Defaults worth knowing: -- `file` is resolved relative to the `filePath` setting, which defaults to `files/` under the web root (`public/files/` in the default app layout). `directory="/var/uploads"` is meant to serve from outside the web root, but it is currently broken on Adobe ColdFusion (the absolute path gets web-root-prefixed and throws `Wheels.FileNotFound`) and on every engine when the path contains `/wheels` — see [#3077](https://github.com/wheels-dev/wheels/issues/3077). On Lucee, absolute paths without a `/wheels` segment work. +- `file` is resolved relative to the `filePath` setting, which defaults to `files/` under the web root (`public/files/` in the default app layout). Pass an absolute `directory` (e.g. `directory="/var/uploads"`) to serve from outside the web root — absolute paths that exist on disk are used verbatim on all engines ([#3077](https://github.com/wheels-dev/wheels/issues/3077)). A leading-slash path that does not exist on disk (e.g. `directory="/reports/"`) keeps its historical meaning and resolves relative to the web root. - `name` overrides what the browser shows in the Save dialog. Use it to hide storage filenames from the client. - `type` overrides the auto-detected MIME type. - `disposition` is `"attachment"` by default (force download) — pass `"inline"` to render in-browser for PDFs and images.