-
Notifications
You must be signed in to change notification settings - Fork 2
Server‑side caching Issue36 #15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 15 commits
98b40a1
c1b5f70
bf4d977
148411e
9ac7b57
875d1ef
9289c7d
6bdb1ef
781e34a
511b5ee
aaeba6d
524f33c
b9600f6
07e47bf
8cc69d8
8e0ab50
c0e3de6
bcb828c
945d32b
3128ac7
d4e7481
5c80eaa
6950c14
fffdebf
c69b9dd
bfa3129
3b34b91
b2ef693
515bc8c
0a53b7f
ddd7b40
0c47d58
78f7e21
dd0530a
7f73baf
86f2ba7
0a3c08c
56cc3e8
d20bb6f
103178a
e72f073
ff4cd12
7652687
3231ee1
99f5fd7
04cba97
c158677
811ccce
1df4286
19d0a48
d4227b0
3497f8a
bb0d51a
f5b2f44
d6f1d26
704f8d8
3f107c6
27e627c
db0c574
b7154fa
d87171b
c74b88b
baa3981
fa1599a
f3bb1ac
2f8312b
32cbdd3
331615a
1be780f
56ba520
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| name: Java CI with Maven | ||
|
|
||
| on: | ||
| push: | ||
| branches: [ "main" ] | ||
| pull_request: | ||
| branches: [ "main" ] | ||
|
|
||
| jobs: | ||
| build: | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Get Java Version | ||
| run: | | ||
| JAVA_VERSION=$(mvn help:evaluate "-Dexpression=maven.compiler.release" -q -DforceStdout) | ||
| echo "JAVA_VERSION=$JAVA_VERSION" >> $GITHUB_ENV | ||
|
|
||
| - name: Set up JDK ${{ env.JAVA_VERSION }} | ||
| uses: actions/setup-java@v4 | ||
| with: | ||
| java-version: ${{ env.JAVA_VERSION }} | ||
| distribution: 'temurin' | ||
| cache: maven | ||
|
|
||
| - name: Compile with Maven | ||
| run: mvn -B compile --file pom.xml | ||
|
|
||
| - name: Test with Maven | ||
| run: mvn -B test --file pom.xml |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| name: Publish Docker Image to Github Packages on Release | ||
| on: | ||
| release: | ||
| types: | ||
| - published | ||
| jobs: | ||
| publish: | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: read | ||
| packages: write | ||
| steps: | ||
| - uses: actions/checkout@v6.0.2 | ||
| - uses: docker/setup-qemu-action@v3.7.0 | ||
| - uses: docker/setup-buildx-action@v3.12.0 | ||
| - name: Log in to GHCR | ||
| uses: docker/login-action@v3.7.0 | ||
| with: | ||
| registry: ghcr.io | ||
| username: ${{ github.actor }} | ||
| password: ${{ secrets.GITHUB_TOKEN }} | ||
| - name: Extract metadata | ||
| id: meta | ||
| uses: docker/metadata-action@v5.10.0 | ||
| with: | ||
| images: ghcr.io/ithsjava25/webserver | ||
| - name: Build and push | ||
| uses: docker/build-push-action@v6.18.0 | ||
| with: | ||
| context: . | ||
| push: true | ||
| platforms: linux/amd64, linux/arm64 | ||
| tags: ${{ steps.meta.outputs.tags }} | ||
| labels: ${{ steps.meta.outputs.labels }} | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| FROM maven:3-eclipse-temurin-25-alpine AS build | ||
| WORKDIR /build | ||
| COPY src/ src/ | ||
| COPY pom.xml pom.xml | ||
| RUN mvn compile | ||
|
|
||
| FROM eclipse-temurin:25-jre-alpine | ||
| RUN addgroup -S appgroup && adduser -S appuser -G appgroup | ||
| COPY --from=build /build/target/classes/ /app/ | ||
| ENTRYPOINT ["java", "-classpath", "/app", "org.example.App"] | ||
| USER appuser | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,13 +9,21 @@ | |
| <version>1.0-SNAPSHOT</version> | ||
|
|
||
| <properties> | ||
| <maven.compiler.release>23</maven.compiler.release> | ||
| <maven.compiler.release>25</maven.compiler.release> | ||
| <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> | ||
| <junit.jupiter.version>6.0.2</junit.jupiter.version> | ||
| <assertj.core.version>3.27.7</assertj.core.version> | ||
| <mockito.version>5.21.0</mockito.version> | ||
| <bucket4j.version>8.14.0</bucket4j.version> | ||
|
|
||
| </properties> | ||
|
|
||
| <dependencies> | ||
| <dependency> | ||
| <groupId>com.bucket4j</groupId> | ||
| <artifactId>bucket4j_jdk17-core</artifactId> | ||
| <version>${bucket4j.version}</version> | ||
| </dependency> | ||
|
Comment on lines
+22
to
+26
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: rg -rn 'bucket4j|io\.github\.bucket4j|Bucket4j|Bandwidth|Refill|Bucket\.builder' --type=java src/Repository: ithsjava25/ithsjava25-java-project-webserver-maven-java-template Length of output: 90
Verify whether it's used in files outside this scope (configuration loaders, tests, etc.), or remove it if truly unused to reduce classpath and Docker image size. 🤖 Prompt for AI Agents |
||
| <dependency> | ||
| <groupId>org.junit.jupiter</groupId> | ||
| <artifactId>junit-jupiter</artifactId> | ||
|
|
@@ -28,12 +36,36 @@ | |
| <version>${assertj.core.version}</version> | ||
| <scope>test</scope> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.mockito</groupId> | ||
| <artifactId>mockito-core</artifactId> | ||
| <version>${mockito.version}</version> | ||
| <scope>test</scope> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.mockito</groupId> | ||
| <artifactId>mockito-junit-jupiter</artifactId> | ||
| <version>${mockito.version}</version> | ||
| <scope>test</scope> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.awaitility</groupId> | ||
| <artifactId>awaitility</artifactId> | ||
| <version>4.3.0</version> | ||
| <scope>test</scope> | ||
| </dependency> | ||
|
|
||
| <dependency> | ||
| <groupId>tools.jackson.core</groupId> | ||
| <artifactId>jackson-databind</artifactId> | ||
| <version>3.0.3</version> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>tools.jackson.dataformat</groupId> | ||
| <artifactId>jackson-dataformat-yaml</artifactId> | ||
| <version>3.0.3</version> | ||
| </dependency> | ||
|
|
||
| </dependencies> | ||
| <build> | ||
| <plugins> | ||
|
|
@@ -118,6 +150,39 @@ | |
| </execution> | ||
| </executions> | ||
| </plugin> | ||
| <plugin> | ||
| <groupId>org.pitest</groupId> | ||
| <artifactId>pitest-maven</artifactId> | ||
| <version>1.22.0</version> | ||
| <dependencies> | ||
| <dependency> | ||
| <groupId>org.pitest</groupId> | ||
| <artifactId>pitest-junit5-plugin</artifactId> | ||
| <version>1.2.2</version> | ||
| </dependency> | ||
| </dependencies> | ||
| </plugin> | ||
| <plugin> | ||
| <groupId>com.diffplug.spotless</groupId> | ||
| <artifactId>spotless-maven-plugin</artifactId> | ||
| <version>3.2.1</version> | ||
| <configuration> | ||
| <java> | ||
| <removeUnusedImports/> | ||
| <formatAnnotations/> | ||
| </java> | ||
| </configuration> | ||
| <executions> | ||
| <execution> | ||
| <phase>verify</phase> | ||
| <goals> | ||
| <goal> | ||
| check | ||
| </goal> | ||
| </goals> | ||
| </execution> | ||
| </executions> | ||
| </plugin> | ||
| </plugins> | ||
| </build> | ||
| </project> | ||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,42 @@ | ||||||||||
| package org.example; | ||||||||||
|
|
||||||||||
| import org.example.httpparser.HttpParser; | ||||||||||
|
|
||||||||||
| import java.io.IOException; | ||||||||||
| import java.net.Socket; | ||||||||||
|
|
||||||||||
| public class ConnectionHandler implements AutoCloseable { | ||||||||||
|
|
||||||||||
| Socket client; | ||||||||||
| String uri; | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Both are currently package-private, exposing ♻️ Proposed fix- Socket client;
- String uri;
+ private Socket client;
+ private String uri;📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||
|
|
||||||||||
| public ConnectionHandler(Socket client) { | ||||||||||
| this.client = client; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| public void runConnectionHandler() throws IOException { | ||||||||||
| StaticFileHandler sfh = new StaticFileHandler(); | ||||||||||
| HttpParser parser = new HttpParser(); | ||||||||||
| parser.setReader(client.getInputStream()); | ||||||||||
| parser.parseRequest(); | ||||||||||
| parser.parseHttp(); | ||||||||||
| resolveTargetFile(parser.getUri()); | ||||||||||
| sfh.sendGetRequest(client.getOutputStream(), uri); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| private void resolveTargetFile(String uri) { | ||||||||||
| if (uri.matches("/$")) { //matches(/) | ||||||||||
| this.uri = "index.html"; | ||||||||||
| } else if (uri.matches("^(?!.*\\.html$).*$")) { | ||||||||||
| this.uri = uri.concat(".html"); | ||||||||||
| } else { | ||||||||||
| this.uri = uri; | ||||||||||
| } | ||||||||||
|
|
||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
♻️ Proposed fix private void resolveTargetFile(String uri) {
- if (uri.matches("/$")) { //matches(/)
- this.uri = "index.html";
- } else if (uri.matches("^(?!.*\\.html$).*$")) {
- this.uri = uri.concat(".html");
- } else {
- this.uri = uri;
- }
+ // Strip leading slash so new File(WEB_ROOT, uri) resolves relative to WEB_ROOT
+ String normalized = uri.startsWith("/") ? uri.substring(1) : uri;
+ if (normalized.isEmpty()) {
+ this.uri = "index.html";
+ } else if (!normalized.endsWith(".html")) {
+ this.uri = normalized + ".html";
+ } else {
+ this.uri = normalized;
+ }
}🤖 Prompt for AI Agents |
||||||||||
| } | ||||||||||
|
|
||||||||||
| @Override | ||||||||||
| public void close() throws Exception { | ||||||||||
| client.close(); | ||||||||||
| } | ||||||||||
| } | ||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| package org.example; | ||
|
|
||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
|
|
||
| public class FileCache { | ||
| private final Map<String, byte []> cache = new HashMap<>(); | ||
|
|
||
| public boolean contains (String key) { | ||
| return cache.containsKey(key); | ||
| } | ||
|
|
||
| public byte[] get(String key) { | ||
| return cache.get(key); | ||
| } | ||
|
|
||
| public void put(String key, byte[] value){ | ||
| cache.put(key, value); | ||
| } | ||
| public void clear() { | ||
| cache.clear(); | ||
| } | ||
|
|
||
| public int size() { | ||
| return cache.size(); | ||
| } | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| package org.example; | ||
|
|
||
| import org.example.http.HttpResponseBuilder; | ||
|
|
||
| import java.io.File; | ||
| import java.io.IOException; | ||
| import java.io.OutputStream; | ||
| import java.io.PrintWriter; | ||
| import java.nio.file.Files; | ||
| import java.util.Map; | ||
|
|
||
| public class StaticFileHandler { | ||
| private static final String WEB_ROOT = "www"; | ||
| private byte[] fileBytes; | ||
| private final FileCache cache = new FileCache(); | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| public StaticFileHandler(){} | ||
|
|
||
| private void handleGetRequest(String uri) throws IOException { | ||
| if (cache.contains(uri)) { | ||
| System.out.println("✓ Cache hit for: " + uri); | ||
| fileBytes = cache.get(uri); | ||
| return; | ||
| } | ||
| System.out.println("✗ Cache miss for: " + uri); | ||
| File file = new File(WEB_ROOT, uri); | ||
| fileBytes = Files.readAllBytes(file.toPath()); | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| cache.put(uri, fileBytes); | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| public void sendGetRequest(OutputStream outputStream, String uri) throws IOException{ | ||
| handleGetRequest(uri); | ||
| HttpResponseBuilder response = new HttpResponseBuilder(); | ||
| response.setHeaders(Map.of("Content-Type", "text/html; charset=utf-8")); | ||
| response.setBody(fileBytes); | ||
| PrintWriter writer = new PrintWriter(outputStream, true); | ||
| writer.println(response.build()); | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,36 @@ | ||||||||||||||||||||||||||||||||||||||||||||
| package org.example; | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| import java.io.IOException; | ||||||||||||||||||||||||||||||||||||||||||||
| import java.net.ServerSocket; | ||||||||||||||||||||||||||||||||||||||||||||
| import java.net.Socket; | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| public class TcpServer { | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| private final int port; | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| public TcpServer(int port) { | ||||||||||||||||||||||||||||||||||||||||||||
| this.port = port; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| public void start() { | ||||||||||||||||||||||||||||||||||||||||||||
| System.out.println("Starting TCP server on port " + port); | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| try (ServerSocket serverSocket = new ServerSocket(port)) { | ||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Static analysis: plain The static analysis tool flags this as "Cleartext Transmission of Sensitive Information." For an internal/educational HTTP server this is likely intentional, but if the server ever handles any credentials or sensitive content in production, 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||
| while (true) { | ||||||||||||||||||||||||||||||||||||||||||||
| Socket clientSocket = serverSocket.accept(); // block | ||||||||||||||||||||||||||||||||||||||||||||
| System.out.println("Client connected: " + clientSocket.getRemoteSocketAddress()); | ||||||||||||||||||||||||||||||||||||||||||||
| Thread.ofVirtual().start(() -> handleClient(clientSocket)); | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| } catch (IOException e) { | ||||||||||||||||||||||||||||||||||||||||||||
| throw new RuntimeException("Failed to start TCP server", e); | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+24
to
+31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any transient The single Consider distinguishing startup failure from runtime accept errors: ♻️ Proposed fix try (ServerSocket serverSocket = new ServerSocket(port)) {
while (true) {
- Socket clientSocket = serverSocket.accept();
- System.out.println("Client connected: " + clientSocket.getRemoteSocketAddress());
- Thread.ofVirtual().start(() -> handleClient(clientSocket));
+ try {
+ Socket clientSocket = serverSocket.accept();
+ System.out.println("Client connected: " + clientSocket.getRemoteSocketAddress());
+ Thread.ofVirtual().start(() -> handleClient(clientSocket));
+ } catch (IOException e) {
+ if (serverSocket.isClosed()) break; // intentional shutdown
+ System.err.println("Transient accept error, continuing: " + e.getMessage());
+ }
}
} catch (IOException e) {
throw new RuntimeException("Failed to start TCP server", e);
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| private void handleClient(Socket client) { | ||||||||||||||||||||||||||||||||||||||||||||
| try (ConnectionHandler connectionHandler = new ConnectionHandler(client)) { | ||||||||||||||||||||||||||||||||||||||||||||
| connectionHandler.runConnectionHandler(); | ||||||||||||||||||||||||||||||||||||||||||||
| } catch (Exception e) { | ||||||||||||||||||||||||||||||||||||||||||||
| throw new RuntimeException("Error handling client connection " + e); | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+45
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Original exception cause and stack trace are silently discarded.
Pass 🐛 Proposed fix } catch (Exception e) {
- throw new RuntimeException("Error handling client connection " + e);
+ throw new RuntimeException("Error handling client connection", e);
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Get full Dockerfile content cat -n DockerfileRepository: ithsjava25/ithsjava25-java-project-webserver-maven-java-template
Length of output: 515
🏁 Script executed:
# Check pom.xml for dependencies and maven plugins cat -n pom.xmlRepository: ithsjava25/ithsjava25-java-project-webserver-maven-java-template
Length of output: 8522
🏁 Script executed:
Repository: ithsjava25/ithsjava25-java-project-webserver-maven-java-template
Length of output: 158
🏁 Script executed:
Repository: ithsjava25/ithsjava25-java-project-webserver-maven-java-template
Length of output: 113
🏁 Script executed:
Repository: ithsjava25/ithsjava25-java-project-webserver-maven-java-template
Length of output: 672
🏁 Script executed:
# Examine StaticFileHandler to understand how it resolves the www directory path cat -n src/main/java/org/example/StaticFileHandler.javaRepository: ithsjava25/ithsjava25-java-project-webserver-maven-java-template
Length of output: 1680
🏁 Script executed:
Repository: ithsjava25/ithsjava25-java-project-webserver-maven-java-template
Length of output: 287
🏁 Script executed:
Repository: ithsjava25/ithsjava25-java-project-webserver-maven-java-template
Length of output: 209
Runtime image is missing dependency JARs and the
www/static content directory.The build stage only runs
mvn compile, which puts compiled classes undertarget/classes/but does not download dependency JARs into that directory. The runtime stage copies onlytarget/classes/to/app/and sets the classpath to/app. Sincepom.xmldeclares non-test dependencies (bucket4j, jackson-databind, jackson-dataformat-yaml), the application will fail at runtime withClassNotFoundException.Additionally,
StaticFileHandleruses a relative path"www"to locate static content. The Dockerfile does not copy thewww/directory into the image, so the application will fail to serve static files at runtime.Consider using
mvn packagewith themaven-dependency-plugin:copy-dependenciesgoal (or the shade/assembly plugin to create a fat JAR), add aCOPY www/ /app/www/step in the runtime stage, and setWORKDIR /appto ensure the relative path resolves correctly.🤖 Prompt for AI Agents