T8 by nmaguiar · Pull Request #1747 · OpenAF/openaf

nmaguiar · 2026-03-09T09:52:19Z

This pull request introduces several important security and correctness improvements to the file serving logic in both src/com/nwu/httpd/responses/FileResponse.java and src/com/nwu2/httpd/responses/FileResponse.java, as well as minor dependency and JavaScript updates. The main focus is on tightening path validation to prevent directory traversal and invalid path access, improving directory listing and file serving behavior, and updating dependencies.

File serving and security improvements:

Refactored URI handling to robustly extract and sanitize the relative path, including removing leading slashes, handling null bytes, and preventing directory traversal attacks by checking for .. in the path. Added canonical path checks to ensure files served are within the intended directory. [1] [2] [3] [4]
Improved directory listing and navigation by using a sanitized uriForLinks variable for all links and display logic, ensuring correct and safe URLs in directory listings. [1] [2] [3] [4] [5] [6]
Added validation for HTTP range requests to return a RANGE_NOT_SATISFIABLE response if the requested byte range is invalid, and simplified file input stream handling for better reliability. [1] [2]

Dependency updates:

Updated Jetty dependencies (jetty-io and jetty-util) from version 12.1.6 to 12.1.7 in pom.xml for improved stability and security.

JavaScript minor improvements:

Replaced usage of $do with $doV in two locations in js/openaf.js for improved asynchronous control flow. [1] [2]

- Refactored serveFile logic to use canonical paths and stricter validation - Enhanced security by preventing directory traversal and invalid path access - Improved code clarity and robustness for file serving in both modules

Bumps org.eclipse.jetty:jetty-util from 12.1.6 to 12.1.7. --- updated-dependencies: - dependency-name: org.eclipse.jetty:jetty-util dependency-version: 12.1.7 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>

Bumps org.eclipse.jetty:jetty-io from 12.1.6 to 12.1.7. --- updated-dependencies: - dependency-name: org.eclipse.jetty:jetty-io dependency-version: 12.1.7 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>

…jetty-jetty-io-12.1.7

…jetty-jetty-util-12.1.7

- Updated jetty-io and jetty-util libraries to the latest 12.1.7 release.\n- No functional changes, just dependency updates.

Copilot

Pull request overview

This PR introduces security and correctness improvements to the HTTP file serving logic in both src/com/nwu/httpd/responses/FileResponse.java and src/com/nwu2/httpd/responses/FileResponse.java, updates Jetty dependencies, and makes minor JavaScript improvements.

Changes:

Refactored URI path handling in both FileResponse classes to prevent directory traversal attacks via null bytes, .. segments, and canonical path checks; improved directory listing link generation using a sanitized uriForLinks variable
Added HTTP 416 (Range Not Satisfiable) validation for byte-range requests and simplified file stream handling
Updated Jetty dependencies from 12.1.6 to 12.1.7 and replaced $do with $doV in two places in js/openaf.js

Reviewed changes

Copilot reviewed 4 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`src/com/nwu2/httpd/responses/FileResponse.java`	Security improvements: null-byte blocking, canonical path check, improved directory listing, range request validation
`src/com/nwu/httpd/responses/FileResponse.java`	Same security improvements as nwu2 (mirrored change)
`pom.xml`	Bump Jetty `jetty-io` and `jetty-util` from 12.1.6 to 12.1.7
`js/openaf.js`	Replace `$do` with `$doV` for async stream control flow; fix indentation
`js/oafp.js`	Version bump and minified code refactoring/optimizations

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-09T09:54:56Z

src/com/nwu/httpd/responses/FileResponse.java

+		if (relativeUri.startsWith("..") || relativeUri.endsWith("..")
+				|| relativeUri.indexOf("../") >= 0)
+			return new com.nwu.httpd.responses.SimpleResponse(httpd,
+					Codes.HTTP_FORBIDDEN, Codes.MIME_PLAINTEXT,
+					"FORBIDDEN: Won't serve ../ for security reasons.");


Same incomplete traversal check as in nwu2: only ../ is checked but not ..\. The canonical path check provides a safety net, but the explicit check should be comprehensive.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2026-03-09T09:54:56Z

src/com/nwu2/httpd/responses/FileResponse.java

+				if (startFrom < 0 || startFrom >= f.length()) {
+					return new com.nwu2.httpd.responses.SimpleResponse(httpd,
+							com.nwu2.httpd.NanoHTTPD.Response.Status.RANGE_NOT_SATISFIABLE, Codes.MIME_PLAINTEXT,
+							"Requested Range Not Satisfiable");
+				}


The RANGE_NOT_SATISFIABLE check at line 239 fires even when no range header was provided (startFrom defaults to 0) and the file is empty (f.length() == 0). Since startFrom < 0 is always false for the default of 0, and startFrom >= f.length() is true for a 0-byte file, a valid request for an empty file will be incorrectly rejected with a 416 error instead of a 200 OK.

More critically, the check should only be applied when a range header was actually present. When range is null, startFrom remains 0 and the response should always be served normally. The guard should be: if (range != null && (startFrom < 0 || startFrom >= f.length())).

Additionally, if startFrom is 0 and no range was requested, returning a 416 for an empty file is incorrect behavior.

@copilot open a new pull request to apply changes based on this feedback

src/com/nwu/httpd/responses/FileResponse.java

src/com/nwu2/httpd/responses/FileResponse.java

Copilot · 2026-03-09T09:54:57Z

src/com/nwu/httpd/responses/FileResponse.java

+				fis.skip(startFrom);
+				com.nwu.httpd.responses.Response r = new com.nwu.httpd.responses.SimpleResponse(
+						httpd, Codes.HTTP_OK, mime, fis);
+				r.addHeader("Content-length", "" + (f.length() - startFrom));
+				r.addHeader("Content-range", "" + startFrom + "-"
+						+ (f.length() - 1) + "/" + f.length());
+				return r;


Same resource leak issue as in nwu2: the FileInputStream opened at line 245 is not protected by a try-finally block. If an exception occurs between new FileInputStream(f) and the return r, the stream will be leaked.

Suggested change

fis.skip(startFrom);

com.nwu.httpd.responses.Response r = new com.nwu.httpd.responses.SimpleResponse(

httpd, Codes.HTTP_OK, mime, fis);

r.addHeader("Content-length", "" + (f.length() - startFrom));

r.addHeader("Content-range", "" + startFrom + "-"

+ (f.length() - 1) + "/" + f.length());

return r;

try {

fis.skip(startFrom);

com.nwu.httpd.responses.Response r = new com.nwu.httpd.responses.SimpleResponse(

httpd, Codes.HTTP_OK, mime, fis);

r.addHeader("Content-length", "" + (f.length() - startFrom));

r.addHeader("Content-range", "" + startFrom + "-"

+ (f.length() - 1) + "/" + f.length());

return r;

} catch (IOException e) {

try {

fis.close();

} catch (IOException ignore) {

// ignore secondary exception during close

}

throw e;

}

Copilot · 2026-03-09T09:54:57Z

src/com/nwu2/httpd/responses/FileResponse.java

+
+		// Prohibit getting out of current directory
+		if (relativeUri.startsWith("..") || relativeUri.endsWith("..")
+				|| relativeUri.indexOf("../") >= 0)


The .. path traversal check at line 108–112 is incomplete: it checks for ../ (forward slash) but not ..\ (backslash). On Windows, a path segment like ..\ could bypass this check. While backslashes are normalized to forward slashes at line 97 (replace('\\', '/')), this normalization only covers literal backslashes in the input, not encoded variants (e.g., %5C). URL-decoded traversal sequences could still slip through. The canonical path check added later provides defense-in-depth, but the traversal check should also include ".."+File.separator for completeness.

Suggested change

|| relativeUri.indexOf("../") >= 0)

|| relativeUri.indexOf("../") >= 0

|| relativeUri.indexOf(".." + File.separator) >= 0)

Copilot · 2026-03-09T10:20:44Z

@nmaguiar I've opened a new pull request, #1749, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot · 2026-03-09T10:21:33Z

@nmaguiar I've opened a new pull request, #1750, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

nmaguiar and others added 9 commits March 5, 2026 21:16

sync oafproc

36da053

refactor(jsonrpc): improve code readability and consistency

39d0762

Merge branch 't8' of https://github.com/openaf/openaf into t8

81dee0a

Merge pull request #1744 from OpenAF/dependabot/maven/t8/org.eclipse.…

32c4058

…jetty-jetty-io-12.1.7

Merge pull request #1743 from OpenAF/dependabot/maven/t8/org.eclipse.…

166d9e2

…jetty-jetty-util-12.1.7

chore(deps): bump jetty-io and jetty-util to 12.1.7

850ef91

- Updated jetty-io and jetty-util libraries to the latest 12.1.7 release.\n- No functional changes, just dependency updates.

nmaguiar requested a review from Copilot March 9, 2026 09:52

nmaguiar self-assigned this Mar 9, 2026

nmaguiar added enhancement dependencies Pull requests that update a dependency file labels Mar 9, 2026

nmaguiar added this to Continuous Enhancement Mar 9, 2026

github-project-automation bot moved this to Backlog in Continuous Enhancement Mar 9, 2026

Copilot started reviewing on behalf of nmaguiar March 9, 2026 09:52 View session

Copilot AI reviewed Mar 9, 2026

View reviewed changes

Copilot AI mentioned this pull request Mar 9, 2026

Fix incomplete path traversal check in FileResponse (nwu + nwu2) #1749

Closed

Copilot AI mentioned this pull request Mar 9, 2026

fix(FileResponse): guard 416 response behind range header presence check #1750

Closed

nmaguiar and others added 4 commits March 9, 2026 10:22

Update src/com/nwu2/httpd/responses/FileResponse.java

ff233bc

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

refactor(FileResponse): enhance security checks in serveFile method

8aac368

sync openaf/oafp

0320874

Merge branch 't8' of https://github.com/openaf/openaf into t8

32cb317

nmaguiar merged commit 45c00ae into master Mar 9, 2026
1 of 2 checks passed

github-project-automation bot moved this from Backlog to Done in Continuous Enhancement Mar 9, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

T8#1747

T8#1747
nmaguiar merged 13 commits intomasterfrom
t8

nmaguiar commented Mar 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 9, 2026

Uh oh!

nmaguiar Mar 9, 2026

Uh oh!

Copilot AI Mar 9, 2026

Uh oh!

nmaguiar Mar 9, 2026

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 9, 2026

Uh oh!

Copilot AI Mar 9, 2026

Uh oh!

Copilot AI commented Mar 9, 2026

Uh oh!

Copilot AI commented Mar 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	\|\| relativeUri.indexOf("../") >= 0)
	\|\| relativeUri.indexOf("../") >= 0
	\|\| relativeUri.indexOf(".." + File.separator) >= 0)

Conversation

nmaguiar commented Mar 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

nmaguiar Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

nmaguiar Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Mar 9, 2026

Uh oh!

Copilot AI commented Mar 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants