Skip to content

Commit c158677

Browse files
committed
refactored StaticFileHandler to improve error handling, enhance sanitization logic, and optimize shared cache usage; streamlined test cases for clarity
1 parent 04cba97 commit c158677

File tree

2 files changed

+47
-64
lines changed

2 files changed

+47
-64
lines changed

src/main/java/org/example/StaticFileHandler.java

Lines changed: 25 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
public class StaticFileHandler {
1414
private static final String DEFAULT_WEB_ROOT = "www";
1515

16-
// EN shared cache för alla threads
16+
// EN shared cache för alla threads
1717
private static final CacheFilter SHARED_CACHE = new CacheFilter();
1818

1919
private final String webRoot;
@@ -28,46 +28,44 @@ public StaticFileHandler(String webRoot) {
2828
}
2929

3030
public void sendGetRequest(OutputStream outputStream, String uri) throws IOException {
31+
String sanitizedUri = sanitizeUri(uri);
32+
33+
if (isPathTraversal(sanitizedUri)) {
34+
sendErrorResponse(outputStream, 403, "Forbidden");
35+
return;
36+
}
37+
3138
try {
32-
String sanitizedUri = sanitizeUri(uri);
33-
34-
if (isPathTraversal(sanitizedUri)) {
35-
sendErrorResponse(outputStream, 403, "Forbidden");
36-
return;
37-
}
38-
39-
// Använd shared cache istället för ny instans
4039
byte[] fileBytes = SHARED_CACHE.getOrFetch(sanitizedUri,
41-
path -> Files.readAllBytes(new File(webRoot, path).toPath())
40+
path -> Files.readAllBytes(new File(webRoot, path).toPath())
4241
);
43-
42+
4443
HttpResponseBuilder response = new HttpResponseBuilder();
45-
response.setHeaders(Map.of("Content-Type", "text/html; charset=UTF-8"));
44+
response.setContentTypeFromFilename(sanitizedUri);
4645
response.setBody(fileBytes);
4746
outputStream.write(response.build());
4847
outputStream.flush();
49-
48+
5049
} catch (IOException e) {
51-
try {
52-
sendErrorResponse(outputStream, 404, "Not Found");
53-
} catch (IOException ex) {
54-
ex.printStackTrace();
55-
}
50+
sendErrorResponse(outputStream, 404, "Not Found");
5651
}
5752
}
5853

5954
private String sanitizeUri(String uri) {
60-
uri = uri.split("\\?")[0];
61-
uri = uri.split("#")[0];
62-
uri = uri.replace("\0", "");
63-
64-
while (uri.startsWith("/")) {
65-
uri = uri.substring(1);
66-
}
67-
55+
// Entydlig: ta bort query string och fragment
56+
int queryIndex = uri.indexOf('?');
57+
int fragmentIndex = uri.indexOf('#');
58+
int endIndex = Math.min(
59+
queryIndex > 0 ? queryIndex : uri.length(),
60+
fragmentIndex > 0 ? fragmentIndex : uri.length()
61+
);
62+
63+
uri = uri.substring(0, endIndex)
64+
.replace("\0", "")
65+
.replaceAll("^/+", ""); // Bort med leading slashes
66+
6867
return uri;
6968
}
70-
7169
private boolean isPathTraversal(String uri) {
7270
try {
7371
Path webRootPath = Paths.get(webRoot).toRealPath();

src/test/java/org/example/StaticFileHandlerTest.java

Lines changed: 22 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,17 @@
2828
*/
2929
class StaticFileHandlerTest {
3030

31-
private StaticFileHandler handler;
31+
private StaticFileHandler createHandler(){
32+
return new StaticFileHandler(tempDir.toString());
33+
}
34+
35+
private String sendRequest(String uri) throws IOException {
36+
StaticFileHandler handler = createHandler();
37+
ByteArrayOutputStream output = new ByteArrayOutputStream();
38+
handler.sendGetRequest(output, uri);
39+
return output.toString();
40+
}
41+
3242

3343
//Junit creates a temporary folder which can be filled with temporary files that gets removed after tests
3444
@TempDir
@@ -45,60 +55,35 @@ void setUp() {
4555
void testCaching_HitOnSecondRequest() throws IOException {
4656
// Arrange
4757
Files.writeString(tempDir.resolve("cached.html"), "Content");
48-
StaticFileHandler handler = new StaticFileHandler(tempDir.toString());
4958

50-
// Act - Första anropet (cache miss)
51-
handler.sendGetRequest(new ByteArrayOutputStream(), "cached.html");
59+
// Act
60+
sendRequest("cached.html");
5261
int sizeAfterFirst = StaticFileHandler.getCacheStats().entries;
53-
54-
// Act - Andra anropet (cache hit)
55-
handler.sendGetRequest(new ByteArrayOutputStream(), "cached.html");
62+
sendRequest("cached.html");
5663
int sizeAfterSecond = StaticFileHandler.getCacheStats().entries;
5764

58-
// Assert - Cache ska innehålla samma entry
59-
assertThat(sizeAfterFirst).isEqualTo(1);
60-
assertThat(sizeAfterSecond).isEqualTo(1);
65+
// Assert
66+
assertThat(sizeAfterFirst).isEqualTo(sizeAfterSecond).isEqualTo(1);
6167
}
6268

69+
6370
@Test
6471
void testSanitization_QueryString() throws IOException {
65-
// Arrange
6672
Files.writeString(tempDir.resolve("index.html"), "Home");
67-
StaticFileHandler handler = new StaticFileHandler(tempDir.toString());
68-
ByteArrayOutputStream output = new ByteArrayOutputStream();
69-
70-
// Act - URI med query string
71-
handler.sendGetRequest(output, "index.html?foo=bar");
72-
73-
// Assert
74-
assertThat(output.toString()).contains("HTTP/1.1 200");
73+
assertThat(sendRequest("index.html?foo=bar")).contains("HTTP/1.1 200");
7574
}
7675

76+
7777
@Test
7878
void testSanitization_LeadingSlash() throws IOException {
79-
// Arrange
8079
Files.writeString(tempDir.resolve("page.html"), "Page");
81-
StaticFileHandler handler = new StaticFileHandler(tempDir.toString());
82-
ByteArrayOutputStream output = new ByteArrayOutputStream();
83-
84-
// Act
85-
handler.sendGetRequest(output, "/page.html");
86-
87-
// Assert
88-
assertThat(output.toString()).contains("HTTP/1.1 200");
80+
assertThat(sendRequest("/page.html")).contains("HTTP/1.1 200");
8981
}
9082

83+
9184
@Test
9285
void testSanitization_NullBytes() throws IOException {
93-
// Arrange
94-
StaticFileHandler handler = new StaticFileHandler(tempDir.toString());
95-
ByteArrayOutputStream output = new ByteArrayOutputStream();
96-
97-
// Act
98-
handler.sendGetRequest(output, "file.html\0../../secret");
99-
100-
// Assert
101-
assertThat(output.toString()).contains("HTTP/1.1 404");
86+
assertThat(sendRequest("file.html\0../../secret")).contains("HTTP/1.1 404");
10287
}
10388

10489
@Test

0 commit comments

Comments
 (0)