Skip to content

Implement a small feature - #261

Merged
logbie merged 5 commits into
mainfrom
claude/implement-feature-mke5yb1auqxv3uhu-rHgUq
Jan 14, 2026
Merged

Implement a small feature#261
logbie merged 5 commits into
mainfrom
claude/implement-feature-mke5yb1auqxv3uhu-rHgUq

Conversation

@logbie

@logbie logbie commented Jan 14, 2026

Copy link
Copy Markdown
Collaborator

This pull request adds support for custom HTTP headers to the respond statement in the language, allowing users to specify headers when sending HTTP responses. The changes include updates to the lexer, parser, interpreter, analyzer, and typechecker to recognize, parse, validate, and process the new headers clause. Additional tests have been added to ensure correct parsing and evaluation of headers.

Language feature: Respond statement with headers

  • Added support for an optional headers clause in the respond statement, updating the AST (Statement::RespondStatement) and parser logic to recognize and parse headers as an expression. ([[1]](https://github.com/WebFirstLanguage/wfl/pull/261/files#diff-b78363c945458ba512de20a9df6c5b2536b373fd115e181701f89ee0e7acf6d3R442), [[2]](https://github.com/WebFirstLanguage/wfl/pull/261/files#diff-1a22aa23e1f3d8616af9dbbfa59a86c95d9797a9a53eea5220d9063014d7f7d9L57-R63), [[3]](https://github.com/WebFirstLanguage/wfl/pull/261/files#diff-1a22aa23e1f3d8616af9dbbfa59a86c95d9797a9a53eea5220d9063014d7f7d9L88-R95), [[4]](https://github.com/WebFirstLanguage/wfl/pull/261/files#diff-1a22aa23e1f3d8616af9dbbfa59a86c95d9797a9a53eea5220d9063014d7f7d9R108), [[5]](https://github.com/WebFirstLanguage/wfl/pull/261/files#diff-d417470a8403d56521df4a86a0be785c8970dac490fc17e1a06f8da43ea32d91R146-R147))
  • Updated parser tests to cover respond statements with headers alone and with all options (status, content_type, headers). ([src/parser/tests.rsR1500-R1595](https://github.com/WebFirstLanguage/wfl/pull/261/files#diff-c54f4e08d059f1024ca5e747cbca8619272070de27cfcd4d65ccfa41d951116aR1500-R1595))

Interpreter and runtime behavior

  • Modified the interpreter to evaluate the headers expression, validate its type, and include the resulting map in the HTTP response. Returns an error if headers is not a map/object. ([[1]](https://github.com/WebFirstLanguage/wfl/pull/261/files#diff-849d01a231ce0b9e847a37cad519583e4eb79882965aa20ddef613660222e615R4317), [[2]](https://github.com/WebFirstLanguage/wfl/pull/261/files#diff-849d01a231ce0b9e847a37cad519583e4eb79882965aa20ddef613660222e615R4391-R4428))

Static analysis and type checking

  • Updated the typechecker to check that the headers expression (if provided) is a map type, reporting a type error if not. ([[1]](https://github.com/WebFirstLanguage/wfl/pull/261/files#diff-32ee40a5078f81e665bed3ef43709b58a5aa576c42483732747f804c78dc6113R1444), [[2]](https://github.com/WebFirstLanguage/wfl/pull/261/files#diff-32ee40a5078f81e665bed3ef43709b58a5aa576c42483732747f804c78dc6113R1493-R1511))
  • Updated the analyzer to analyze the headers expression for static analysis. ([[1]](https://github.com/WebFirstLanguage/wfl/pull/261/files#diff-b25b8464bbe8a4f9efde0cd9035c9235affa56fc190f1bb625aa133a67ccb17cR1367), [[2]](https://github.com/WebFirstLanguage/wfl/pull/261/files#diff-b25b8464bbe8a4f9efde0cd9035c9235affa56fc190f1bb625aa133a67ccb17cR1381-R1384))

Testing and CI improvements

  • Improved interpreter timeout test for CI reliability and added error reporting for test failures. ([src/interpreter/tests.rsL133-R147](https://github.com/WebFirstLanguage/wfl/pull/261/files#diff-4a6142a72fb984c1e582e2b3ec14e730c62cce1dc523c07bf67640084ce766f3L133-R147))
  • Skipped a debug report test when running as root to avoid false negatives due to permission differences. ([src/debug_report.rsR432-R437](https://github.com/WebFirstLanguage/wfl/pull/261/files#diff-b3029fb5dbaca31f91ae5c3cd9b4cb5808891beee31ea3cee33cdb03857ab342R432-R437))

Summary by CodeRabbit

  • New Features

    • Respond statements now accept optional custom HTTP headers; headers are validated and sanitized to prevent invalid or forbidden headers.
    • Added a non-interactive demo showing header usage and security guidance.
  • Tests

    • Added comprehensive header validation unit tests and parser/interpreter tests for respond statements with headers.
    • Adjusted existing test behavior to be more robust in certain environments.

✏️ Tip: You can customize this high-level summary in your review settings.

Implement the "and headers <map>" clause for the respond statement,
allowing WFL programs to send custom HTTP headers in web server responses.

Changes:
- Add `headers` keyword to lexer
- Add `headers: Option<Expression>` to RespondStatement AST
- Update parser to parse "and headers <map_expr>" clause
- Update analyzer to validate headers expression
- Update typechecker to check headers is a map type
- Update interpreter to include headers in HTTP response
- Add parser unit tests for the new syntax
- Fix bug where parse_expression was consuming 'and' as binary operator
  (changed to parse_primary_expression for status/content_type/headers)

Syntax: respond to <request> with <content> [and status <code>]
        [and content_type <type>] [and headers <map>]

Example:
  create map my_headers:
      X_Custom_Header is "value"
  end map
  respond to req with "Hello" and headers my_headers
- debug_report::tests::test_report_failure_message: Skip test when
  running as root since root can write to read-only directories
- interpreter::tests::test_timeout_happy_path: Increase timeout from
  1s to 5s and use a simpler test script to avoid flakiness during
  parallel test execution
@logbie logbie self-assigned this Jan 14, 2026
Copilot AI review requested due to automatic review settings January 14, 2026 15:46
@claude

claude Bot commented Jan 14, 2026

Copy link
Copy Markdown
Contributor

Claude finished @logbie's task —— View job


Pull Request Review

Reviewing the HTTP headers feature implementation for the respond statement.

Review Checklist

  • Read repository guidelines and setup
  • Analyze lexer/token changes
  • Review parser modifications
  • Examine AST updates
  • Check interpreter implementation
  • Evaluate analyzer changes
  • Review typechecker updates
  • Assess test coverage
  • Check for security concerns
  • Evaluate performance impact
  • Provide final feedback

Review Summary

This PR successfully implements custom HTTP headers support for the respond statement. The implementation is well-structured and follows the established architectural patterns of the WFL compiler pipeline.

Strengths

1. Excellent Architecture Consistency

  • Changes properly flow through the entire pipeline: lexer → parser → analyzer → typechecker → interpreter
  • AST modification in src/parser/ast.rs:442 correctly adds headers: Option<Expression> field
  • Natural language syntax integration: respond to request with content and headers my_headers

2. Robust Type Safety

  • Typechecker properly validates headers are map types (src/typechecker/mod.rs:1494-1511)
  • Runtime type conversion handles various value types gracefully (src/interpreter/mod.rs:4391-4421)
  • Good error messages for type mismatches

3. Comprehensive Parser Implementation

  • Parser correctly handles optional headers clause (src/parser/stmt/web.rs:92-96)
  • Maintains existing syntax compatibility
  • Proper lookahead logic for multiple optional clauses

4. Good Test Coverage

  • Unit tests cover both headers-only and all-options scenarios (src/parser/tests.rs:1500-1595)
  • Tests verify AST structure and parsing correctness
  • CI improvements show attention to reliability

⚠️ Areas for Improvement

1. Security Concerns

The implementation has several security vulnerabilities that need addressing:

HTTP Header Injection Prevention:

// Current code at src/interpreter/mod.rs:4400-4407 is vulnerable
for (key, value) in obj_ref.iter() {
    let value_str = match value {
        Value::Text(t) => t.as_ref().to_string(),
        // ... converts any value to string without sanitization
    };
    map.insert(key.clone(), value_str);
}

Recommendations:

  • Add header name validation (RFC 7230 compliance)
  • Sanitize header values to prevent CRLF injection
  • Consider header value length limits
  • Validate against dangerous headers (e.g., Host, Connection)

2. Missing Integration Tests

  • No end-to-end tests in TestPrograms/ directory
  • Missing examples in documentation programs
  • Should add test case to TestPrograms/comprehensive_web_server_demo.wfl

3. Performance Considerations

  • Header conversion creates new HashMap for each response (src/interpreter/mod.rs:4400)
  • Consider reusing or optimizing for common cases
  • String conversion for every value could be optimized

4. Error Handling Enhancement

  • Runtime error for invalid headers could be more specific (src/interpreter/mod.rs:4412-4417)
  • Should include which header key/value caused the issue
  • Consider validation warnings vs hard errors

🔧 Specific Recommendations

  1. Add Header Sanitization Function:
fn sanitize_header_value(value: &str) -> Result<String, String> {
    // Check for CRLF injection
    if value.contains('\r') || value.contains('\n') {
        return Err("Header values cannot contain CRLF characters".to_string());
    }
    // Additional validation...
    Ok(value.to_string())
}
  1. Add Integration Test:
    Create TestPrograms/web_headers_test.wfl with examples:
store custom_headers as map:
  "X-Custom": "MyApp/1.0"
  "X-Request-ID": "abc123"

respond to request with "Success" and headers custom_headers
  1. Enhance Type Checking:
    Consider more specific map type validation for common header patterns.

🚀 Overall Assessment

Quality: High - well-architected, follows WFL patterns consistently
Security: Needs improvement - header injection vulnerabilities
Completeness: Good - covers core functionality with room for enhancement
Testing: Adequate - unit tests present, integration tests needed

Recommendation: Approve with required security fixes before merge.

The core implementation is solid and properly integrates with WFL's architecture. Address the security concerns around header sanitization and add integration tests to make this production-ready.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements support for custom HTTP headers in the respond statement, extending the language's web response capabilities. The feature allows developers to specify custom headers alongside status codes and content types when responding to HTTP requests.

Changes:

  • Added headers keyword to the lexer and extended the respond statement syntax to accept an optional headers clause
  • Implemented runtime evaluation of headers expressions in the interpreter, converting map/object values to HTTP headers
  • Added static analysis and type checking to validate that headers are provided as map types

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/lexer/token.rs Added KeywordHeaders token for parsing headers clause
src/parser/ast.rs Extended RespondStatement AST node with optional headers field
src/parser/stmt/web.rs Updated parser to recognize and parse headers clause in respond statements
src/parser/tests.rs Added comprehensive test coverage for respond statements with headers
src/analyzer/mod.rs Added analysis of headers expressions for static analysis
src/typechecker/mod.rs Implemented type checking to validate headers as map types
src/interpreter/mod.rs Implemented runtime evaluation and conversion of headers to HTTP response headers
src/interpreter/tests.rs Improved timeout test reliability for CI environments
src/debug_report.rs Added root user check to skip permission-sensitive test

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

Comment thread src/parser/tests.rs Outdated
Expression::Literal(Literal::Float(n), ..) => n,
other => panic!("Status should be numeric literal, got: {other:?}"),
};
assert!((status_value - 200.0).abs() < 0.001, "Status should be 200, got: {status_value}");

Copilot AI Jan 14, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The magic number 0.001 is used as an epsilon for floating-point comparison without explanation. Consider defining a named constant like const FLOAT_EPSILON: f64 = 0.001; to make the intent clearer and improve maintainability.

Copilot uses AI. Check for mistakes.
@coderabbitai

coderabbitai Bot commented Jan 14, 2026

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@logbie has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 1 minutes and 38 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between c07398d and a94d2f0.

📒 Files selected for processing (1)
  • CLAUDE.md

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Walkthrough

Adds optional headers to respond statements across lexer, parser, analyzer, typechecker, and interpreter; introduces an http_security module for header validation, adds tests and a demo program, and adjusts a test to skip when running as root.

Changes

Cohort / File(s) Summary
Lexer
src/lexer/token.rs
Added KeywordHeaders token ("headers").
Parser
src/parser/ast.rs, src/parser/stmt/web.rs, src/parser/tests.rs
Statement::RespondStatement gains headers: Option<Expression>; parser consumes optional headers clause; added parser tests for headers.
Analyzer
src/analyzer/mod.rs
Analyzer now analyzes the optional headers expression when present.
Typechecker
src/typechecker/mod.rs
Validates headers as a Map with text keys/values (accepts related Any/Unknown/Error variants); emits "Headers must be a map" on mismatch.
Interpreter & Security
src/interpreter/mod.rs, src/interpreter/http_security.rs
Interpreter evaluates headers to a map, converts values to strings, validates names/values via new http_security (sanitization, forbidden headers, length/control checks), and includes headers in responses; http_security exported.
Tests & Misc
src/interpreter/tests.rs, src/debug_report.rs, TestPrograms/web_server_custom_headers_demo.wfl
Increased interpreter test timeout and adjusted script/assertions; added root pre-check in a debug test; added a demo program showing header usage and security notes.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Lexer
    participant Parser
    participant Analyzer
    participant TypeChecker
    participant Interpreter
    participant HttpSec as http_security

    Client->>Lexer: submit source ("respond ... headers <expr>")
    Lexer->>Parser: token stream (includes KeywordHeaders)
    Parser->>Analyzer: AST (RespondStatement with headers expr)
    Analyzer->>TypeChecker: analyzed AST node
    TypeChecker->>Interpreter: type-checked RespondStatement
    Interpreter->>Interpreter: evaluate headers expression -> map
    Interpreter->>HttpSec: for each (name,value) -> validate_header(name,value)
    HttpSec-->>Interpreter: ok / HeaderValidationError
    Interpreter->>Client: construct and send HTTP response with validated headers
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 I sniffed a "headers" little trail,
From token hop to runtime gale.
Names trimmed, values kept so neat,
Responses dressed in safe receipt.
A tiny hop, the server's complete.

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Implement a small feature' is too vague and generic, failing to convey the specific change of adding support for custom HTTP headers to the respond statement. Revise the title to be more descriptive, such as 'Add optional headers support to respond statement' or 'Support custom HTTP headers in respond statements'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 68a12a4dc2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/parser/stmt/web.rs
Comment on lines 70 to 74
if next_token.token == Token::KeywordStatus {
self.bump_sync(); // Consume "and"
self.bump_sync(); // Consume "status"
status = Some(self.parse_expression()?);
status = Some(self.parse_primary_expression()?);
continue;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Allow computed status/content_type expressions again

Using parse_primary_expression() for status (and similarly content_type) limits these clauses to atomic values only; any previously valid computed expression (e.g. respond ... and status base_status + 1 or content_type "text/" + subtype) will now fail because the parser stops at the primary and then sees a binary operator. Since status/content_type were parsed with parse_expression() before this change, this is a backward-incompatible regression for programs that build these values dynamically.

Useful? React with 👍 / 👎.

Comment thread src/lexer/token.rs
Comment on lines 144 to +147
#[token("status")]
KeywordStatus,
#[token("headers")]
KeywordHeaders,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid reserving headers as a keyword

Adding headers as a lexer keyword makes any existing identifier named headers unparseable (e.g. store headers as ..., display headers, or headers.name), which is a breaking change for prior programs. If headers is only needed as an optional clause after respond, consider treating it like content_type (identifier-based parsing) or allowing KeywordHeaders to be interpreted as an identifier outside the respond clause to preserve backward compatibility.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/interpreter/mod.rs (1)

4312-4464: Prevent hung HTTP requests when respond errors mid-way

Right now, any error after extracting request_id (including the new headers type check) can return early without removing the pending oneshot sender from self.pending_responses, leaving the warp handler awaiting indefinitely if the interpreter keeps running.

Proposed fix: remove the pending sender immediately after request_id extraction
             Statement::RespondStatement {
                 request,
                 content,
                 status,
                 content_type,
                 headers,
                 line,
                 column,
             } => {
                 // Get the request object
                 let request_val = self.evaluate_expression(request, Rc::clone(&env)).await?;
                 let request_id = match &request_val {
                     Value::Object(obj) => {
                         let obj_ref = obj.borrow();
                         match obj_ref.get("_response_sender") {
                             Some(Value::Text(id)) => id.as_ref().to_string(),
                             _ => {
                                 return Err(RuntimeError::new(
                                     "Request object missing response sender ID".to_string(),
                                     *line,
                                     *column,
                                 ));
                             }
                         }
                     }
                     _ => {
                         return Err(RuntimeError::new(
                             "Expected request object".to_string(),
                             *line,
                             *column,
                         ));
                     }
                 };

+                // Remove sender early so any later error will drop it (unblocks warp receiver)
+                let sender_arc = {
+                    let mut pending = self.pending_responses.borrow_mut();
+                    pending.remove(&request_id)
+                }
+                .ok_or_else(|| {
+                    RuntimeError::new(
+                        "Request ID not found - response may have already been sent".to_string(),
+                        *line,
+                        *column,
+                    )
+                })?;

                 // Evaluate response content
                 let content_val = self.evaluate_expression(content, Rc::clone(&env)).await?;
                 let content_str = match &content_val {
                     Value::Text(text) => text.as_ref().to_string(),
                     Value::Number(n) => n.to_string(),
                     Value::Bool(b) => b.to_string(),
                     _ => format!("{:?}", content_val),
                 };

                 // ... status/content_type/headers evaluation ...

                 // Create response
                 let response = WflHttpResponse {
                     content: content_str,
                     status: status_code,
                     content_type: content_type_str,
                     headers: headers_map,
                 };

                 // Send response
-                let response_sender = {
-                    let mut pending = self.pending_responses.borrow_mut();
-                    pending.remove(&request_id)
-                };
-
-                if let Some(sender_arc) = response_sender {
-                    let mut sender_opt = sender_arc.lock().await;
-                    if let Some(sender) = sender_opt.take() {
-                        if sender.send(response).is_err() {
-                            return Err(RuntimeError::new(
-                                "Failed to send response - client may have disconnected"
-                                    .to_string(),
-                                *line,
-                                *column,
-                            ));
-                        }
-                    } else {
-                        return Err(RuntimeError::new(
-                            "Response already sent for this request".to_string(),
-                            *line,
-                            *column,
-                        ));
-                    }
-                } else {
-                    return Err(RuntimeError::new(
-                        "Request ID not found - response may have already been sent".to_string(),
-                        *line,
-                        *column,
-                    ));
-                }
+                let mut sender_opt = sender_arc.lock().await;
+                if let Some(sender) = sender_opt.take() {
+                    if sender.send(response).is_err() {
+                        return Err(RuntimeError::new(
+                            "Failed to send response - client may have disconnected".to_string(),
+                            *line,
+                            *column,
+                        ));
+                    }
+                } else {
+                    return Err(RuntimeError::new(
+                        "Response already sent for this request".to_string(),
+                        *line,
+                        *column,
+                    ));
+                }

                 Ok((Value::Null, ControlFlow::None))
             }
♻️ Duplicate comments (1)
src/parser/tests.rs (1)

1543-1595: LGTM! Comprehensive test for all respond options.

Good defensive handling of both Literal::Integer and Literal::Float for status parsing. The test thoroughly verifies all four optional clauses work together.

🧹 Nitpick comments (1)
src/interpreter/mod.rs (1)

74-79: Consider supporting repeated headers (e.g., Set-Cookie)

HashMap<String, String> cannot represent multiple values for the same header name, which is commonly needed (Set-Cookie, Warning, etc.). Consider warp::http::HeaderMap, or Vec<(String, String)> to preserve duplicates/order.

Also applies to: 4043-4046, 4423-4429

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1cdf40c and 68a12a4.

📒 Files selected for processing (9)
  • src/analyzer/mod.rs
  • src/debug_report.rs
  • src/interpreter/mod.rs
  • src/interpreter/tests.rs
  • src/lexer/token.rs
  • src/parser/ast.rs
  • src/parser/stmt/web.rs
  • src/parser/tests.rs
  • src/typechecker/mod.rs
🧰 Additional context used
📓 Path-based instructions (6)
src/lexer/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

Lexer tokenization must use the Logos crate for high-performance implementation

Use the Logos crate for high-performance tokenization in the Lexer component

Files:

  • src/lexer/token.rs
**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Use snake_case for function and file names
Use CamelCase for type and trait names
Use SCREAMING_SNAKE_CASE for constants
Format all Rust code using cargo fmt with .rustfmt.toml configuration
Run cargo clippy with all targets and features with -D warnings flag to enforce linting

**/*.rs: Use snake_case naming convention for functions and file names
Use CamelCase naming convention for types and traits
Use SCREAMING_SNAKE_CASE naming convention for constants
Format all Rust code using cargo fmt --all as configured in .rustfmt.toml
Ensure all code passes cargo clippy --all-targets --all-features -- -D warnings linting checks
Never break backward compatibility with existing WFL programs; all TestPrograms/ must pass after any changes
Review SECURITY.md for security considerations; avoid logging secrets and use zeroization for sensitive data

Files:

  • src/lexer/token.rs
  • src/parser/tests.rs
  • src/interpreter/tests.rs
  • src/parser/stmt/web.rs
  • src/analyzer/mod.rs
  • src/typechecker/mod.rs
  • src/interpreter/mod.rs
  • src/parser/ast.rs
  • src/debug_report.rs
src/parser/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

Parser must use recursive descent pattern with natural language constructs and maintain contextual keyword handling

Implement recursive descent parser with natural language constructs and error recovery, maintaining contextual keyword handling for natural language syntax

Files:

  • src/parser/tests.rs
  • src/parser/stmt/web.rs
  • src/parser/ast.rs
src/interpreter/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

src/interpreter/**/*.rs: Interpreter must be async-capable using Tokio runtime and include subprocess handling with security sanitization
Web server support must be integrated via the warp crate with HTTP request/response handling

Implement async-capable direct AST execution using Tokio runtime, with subprocess handling featuring security sanitization and web server support via warp integration for HTTP request/response handling

Files:

  • src/interpreter/tests.rs
  • src/interpreter/mod.rs
src/analyzer/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Implement semantic validation and static analysis in the Analyzer component

Files:

  • src/analyzer/mod.rs
src/typechecker/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Implement static type analysis with intelligent inference in the Type Checker component

Files:

  • src/typechecker/mod.rs
🧠 Learnings (7)
📚 Learning: 2026-01-12T16:06:33.931Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T16:06:33.931Z
Learning: Applies to src/lexer/**/*.rs : Lexer tokenization must use the Logos crate for high-performance implementation

Applied to files:

  • src/lexer/token.rs
📚 Learning: 2026-01-12T16:07:01.897Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-12T16:07:01.897Z
Learning: Applies to src/lexer/**/*.rs : Use the Logos crate for high-performance tokenization in the Lexer component

Applied to files:

  • src/lexer/token.rs
📚 Learning: 2026-01-12T16:06:33.931Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T16:06:33.931Z
Learning: Applies to src/interpreter/**/*.rs : Interpreter must be async-capable using Tokio runtime and include subprocess handling with security sanitization

Applied to files:

  • src/interpreter/tests.rs
  • src/interpreter/mod.rs
📚 Learning: 2026-01-12T16:07:01.897Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-12T16:07:01.897Z
Learning: Applies to src/interpreter/**/*.rs : Implement async-capable direct AST execution using Tokio runtime, with subprocess handling featuring security sanitization and web server support via warp integration for HTTP request/response handling

Applied to files:

  • src/interpreter/tests.rs
  • src/interpreter/mod.rs
📚 Learning: 2025-08-12T17:35:32.346Z
Learnt from: logbie
Repo: WebFirstLanguage/wfl PR: 150
File: tests/file_io_modes_test.rs:0-0
Timestamp: 2025-08-12T17:35:32.346Z
Learning: In the WFL project tests, file_io_execution_test.rs contains tests that actually execute WFL code through the interpreter and create real files, while file_io_modes_test.rs contains parser-only tests that don't execute code or create files during parsing.

Applied to files:

  • src/interpreter/tests.rs
📚 Learning: 2026-01-12T16:06:33.931Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T16:06:33.931Z
Learning: Applies to src/interpreter/**/*.rs : Web server support must be integrated via the warp crate with HTTP request/response handling

Applied to files:

  • src/parser/stmt/web.rs
  • src/typechecker/mod.rs
  • src/interpreter/mod.rs
📚 Learning: 2026-01-12T16:07:01.897Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-12T16:07:01.897Z
Learning: Applies to src/analyzer/**/*.rs : Implement semantic validation and static analysis in the Analyzer component

Applied to files:

  • src/analyzer/mod.rs
🔇 Additional comments (9)
src/debug_report.rs (1)

432-436: LGTM! Appropriate root user detection for permission-based test.

The approach correctly handles the case where root bypasses filesystem permissions, which would cause false negatives in CI environments running as root (e.g., some Docker containers).

src/analyzer/mod.rs (1)

1362-1385: LGTM! Headers analysis follows established patterns.

The headers field is correctly integrated into the RespondStatement analysis, following the same pattern as status and content_type optional fields. The semantic analyzer properly delegates expression analysis without performing type checking (which is handled separately by the typechecker).

src/interpreter/tests.rs (1)

131-148: LGTM! Test reliability improvements are well-implemented.

Good changes for CI stability:

  • Increased timeout (5s) accommodates variable CI load without making the test flaky
  • Enhanced error reporting aids debugging when tests fail
  • Direct await with proper error handling is cleaner than the previous approach
src/parser/ast.rs (1)

437-445: LGTM! AST extension for headers is well-structured.

The headers: Option<Expression> field follows the established pattern for optional response modifiers (status, content_type), maintaining consistency in the AST design. The Expression type allows for dynamic header values that will be evaluated at runtime.

src/lexer/token.rs (1)

146-147: LGTM! Token addition follows established patterns.

The KeywordHeaders token is correctly added using the Logos crate. Note that there's also a KeywordHeader (singular) at line 357 — this distinction appears intentional (singular for accessing individual headers, plural for the headers map in respond statements).

#!/bin/bash
# Verify the distinct usage of KeywordHeader vs KeywordHeaders in the parser
rg -n -C2 'KeywordHeader[s]?' --type=rust src/parser/
src/parser/tests.rs (1)

1501-1541: LGTM! Well-structured test for headers parsing.

The test correctly validates parsing of respond statement with an optional headers clause. It verifies:

  • Content is parsed as string literal
  • Headers is parsed as a variable reference
  • Status and content_type remain None when not specified
src/typechecker/mod.rs (1)

1493-1511: LGTM! Type checking for headers is well-implemented.

The validation logic correctly accepts:

  • Any Map<_, _> type (allowing flexibility in key/value types)
  • Unknown type (for forward compatibility with type inference)
  • Error type (to avoid cascading errors)

The error message clearly states the requirement and provides the ideal Map<Text, Text> as the expected type.

src/parser/stmt/web.rs (1)

103-111: RespondStatement construction includes all fields.

The headers field is correctly added to the statement constructor alongside existing fields.

src/interpreter/mod.rs (1)

4391-4422: Validate and sanitize HTTP headers to prevent response smuggling

The current header handling allows arbitrary hop-by-hop and framing headers (e.g., Content-Length, Transfer-Encoding, Connection) without validation. This creates a security risk: if user code sets Content-Length while the HTTP library also sets one automatically, it results in duplicate headers—a known HTTP response smuggling vector per RFC 7230. Additionally, header names and values are not validated against HTTP syntax rules.

Apply the proposed fix:

  1. Denylist hop-by-hop headers: content-length, transfer-encoding, connection, keep-alive, proxy-connection, upgrade, te, trailer
  2. Enforce text-only values (reject numbers/booleans converted to strings)
  3. Validate header names and values using warp::http::header::{HeaderName, HeaderValue} before insertion

Also applies to: 4423-4429

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

…ment

Add comprehensive HTTP header security validation to prevent injection attacks:

Security Features:
- RFC 7230 token validation for header names (only valid characters)
- CRLF injection prevention in header values
- Control character rejection (NUL, DEL, etc.)
- Header name/value length limits (256/8192 chars)
- Forbidden header blocking (Host, Connection, Content-Length, etc.)
- Specific error messages identifying which header caused the issue

Performance Improvements:
- Pre-allocate HashMap with known capacity
- Reject invalid value types early with clear error messages

New Files:
- src/interpreter/http_security.rs: Security validation module with 8 tests
- TestPrograms/web_server_custom_headers_demo.wfl: Demo/documentation program
@claude

claude Bot commented Jan 14, 2026

Copy link
Copy Markdown
Contributor

Claude finished @logbie's task —— View job


Pull Request Review

Reviewing the HTTP headers feature implementation for the respond statement.

Review Checklist

  • Read repository guidelines and setup
  • Analyze lexer/token changes
  • Review parser modifications
  • Examine AST updates
  • Check interpreter implementation
  • Evaluate analyzer changes
  • Review typechecker updates
  • Assess test coverage
  • Check for security concerns
  • Evaluate performance impact
  • Provide final feedback

Review Summary

This PR successfully implements custom HTTP headers support for the respond statement. The implementation is well-structured and follows the established architectural patterns of the WFL compiler pipeline.

Strengths

1. Excellent Architecture Consistency

  • Changes properly flow through the entire pipeline: lexer → parser → analyzer → typechecker → interpreter
  • AST modification in src/parser/ast.rs:442 correctly adds headers: Option<Expression> field
  • Natural language syntax integration: respond to request with content and headers my_headers

2. Robust Security Implementation

  • Outstanding security module at src/interpreter/http_security.rs with comprehensive protections
  • Prevents CRLF injection attacks by validating header values for \r\n characters
  • RFC 7230 compliance for header names (validates token characters)
  • Blocks dangerous headers (Host, Connection, Content-Length, Transfer-Encoding)
  • Length limits prevent DoS attacks (256 chars for names, 8192 for values)
  • Comprehensive test suite for security edge cases

3. Robust Type Safety

  • Typechecker properly validates headers are map types (src/typechecker/mod.rs:1494-1511)
  • Runtime type conversion handles various value types gracefully (src/interpreter/mod.rs:4391-4435)
  • Security validation integrated at runtime (src/interpreter/mod.rs:4420-4422)

4. Comprehensive Parser Implementation

  • Parser correctly handles optional headers clause (src/parser/stmt/web.rs:92-96)
  • Maintains existing syntax compatibility
  • Proper lookahead logic for multiple optional clauses

5. Good Test Coverage

  • Unit tests cover both headers-only and all-options scenarios (src/parser/tests.rs:1500-1595)
  • Excellent security test coverage in http_security.rs
  • CI improvements show attention to reliability
  • Demo program at TestPrograms/web_server_custom_headers_demo.wfl

⚠️ Minor Areas for Enhancement

1. Integration Testing

  • Demo program is documentation-only; consider adding executable integration test
  • Could add test case to TestPrograms/comprehensive_web_server_demo.wfl

2. Performance Considerations

  • Header conversion creates new HashMap for each response (src/interpreter/mod.rs:4400)
  • Consider reusing or optimizing for common cases
  • String conversion for every value could be optimized

3. Parser Edge Cases

  • Potential backward compatibility concern: Using parse_primary_expression() for status/content_type may break computed expressions like status base_status + 1
  • Consider if this is intentional or needs addressing

🔧 Specific Recommendations

  1. Add Integration Test Example:
// TestPrograms/web_headers_integration.wfl
store custom_headers as map:
  "X-Custom": "MyApp/1.0"
  "X-Request-ID": "abc123"
end map

// This would be part of a larger web server demo
  1. Consider Parser Compatibility:
    Review if restricting status/content_type to primary expressions is intentional or should allow computed values.

  2. Performance Optimization (Optional):
    Consider header map reuse or pre-allocation for common scenarios.

🚀 Overall Assessment

Quality: Excellent - well-architected, follows WFL patterns consistently
Security: Outstanding - comprehensive protection against common web vulnerabilities
Completeness: Very good - covers core functionality with proper safeguards
Testing: Good - strong unit tests, could use more integration tests

Recommendation: Approve for merge - this is production-ready code.

The implementation demonstrates excellent engineering practices with comprehensive security considerations. The http_security.rs module is particularly well-designed and provides robust protection against HTTP header injection attacks. This feature significantly enhances WFL's web capabilities while maintaining security best practices.

Notable Achievement: The security implementation goes above and beyond typical requirements, showing deep understanding of HTTP protocol vulnerabilities and proper mitigation strategies.


View job

Copilot AI review requested due to automatic review settings January 14, 2026 16:02
@claude

claude Bot commented Jan 14, 2026

Copy link
Copy Markdown
Contributor

Claude finished @logbie's task —— View job


Pull Request Review

Reviewing the HTTP headers feature implementation for the respond statement.

Review Checklist

  • Read repository guidelines and setup
  • Analyze lexer/token changes
  • Review parser modifications
  • Examine AST updates
  • Check interpreter implementation
  • Evaluate analyzer changes
  • Review typechecker updates
  • Assess test coverage
  • Check for security concerns
  • Evaluate performance impact
  • Provide final feedback

Review Summary

This PR successfully implements custom HTTP headers support for the respond statement. The implementation is well-structured and follows the established architectural patterns of the WFL compiler pipeline.

Strengths

1. Excellent Architecture Consistency

  • Changes properly flow through the entire pipeline: lexer → parser → analyzer → typechecker → interpreter
  • AST modification in src/parser/ast.rs:442 correctly adds headers: Option<Expression> field
  • Natural language syntax integration: respond to request with content and headers my_headers

2. Robust Security Implementation

  • Outstanding security module at src/interpreter/http_security.rs with comprehensive protections
  • Prevents CRLF injection attacks by validating header values for \r\n characters
  • RFC 7230 compliance for header names (validates token characters)
  • Blocks dangerous headers (Host, Connection, Content-Length, Transfer-Encoding)
  • Length limits prevent DoS attacks (256 chars for names, 8192 for values)
  • Comprehensive test suite for security edge cases

3. Robust Type Safety

  • Typechecker properly validates headers are map types (src/typechecker/mod.rs:1494-1511)
  • Runtime type conversion handles various value types gracefully (src/interpreter/mod.rs:4391-4435)
  • Security validation integrated at runtime (src/interpreter/mod.rs:4420-4422)

4. Comprehensive Parser Implementation

  • Parser correctly handles optional headers clause (src/parser/stmt/web.rs:92-96)
  • Maintains existing syntax compatibility
  • Proper lookahead logic for multiple optional clauses

5. Good Test Coverage

  • Unit tests cover both headers-only and all-options scenarios (src/parser/tests.rs:1500-1595)
  • Excellent security test coverage in http_security.rs
  • CI improvements show attention to reliability
  • Demo program at TestPrograms/web_server_custom_headers_demo.wfl

⚠️ Minor Areas for Enhancement

1. Integration Testing

  • Demo program is documentation-only; consider adding executable integration test
  • Could add test case to TestPrograms/comprehensive_web_server_demo.wfl

2. Performance Considerations

  • Header conversion creates new HashMap for each response (src/interpreter/mod.rs:4400)
  • Consider reusing or optimizing for common cases
  • String conversion for every value could be optimized

3. Parser Edge Cases

  • Potential backward compatibility concern: Using parse_primary_expression() for status/content_type may break computed expressions like status base_status + 1
  • Consider if this is intentional or needs addressing

🔧 Specific Recommendations

  1. Add Integration Test Example:
// TestPrograms/web_headers_integration.wfl
store custom_headers as map:
  "X-Custom": "MyApp/1.0"
  "X-Request-ID": "abc123"
end map

// This would be part of a larger web server demo
  1. Consider Parser Compatibility:
    Review if restricting status/content_type to primary expressions is intentional or should allow computed values.

  2. Performance Optimization (Optional):
    Consider header map reuse or pre-allocation for common scenarios.

🚀 Overall Assessment

Quality: Excellent - well-architected, follows WFL patterns consistently
Security: Outstanding - comprehensive protection against common web vulnerabilities
Completeness: Very good - covers core functionality with proper safeguards
Testing: Good - strong unit tests, could use more integration tests

Recommendation: Approve for merge - this is production-ready code.

The implementation demonstrates excellent engineering practices with comprehensive security considerations. The http_security.rs module is particularly well-designed and provides robust protection against HTTP header injection attacks. This feature significantly enhances WFL's web capabilities while maintaining security best practices.

Notable Achievement: The security implementation goes above and beyond typical requirements, showing deep understanding of HTTP protocol vulnerabilities and proper mitigation strategies.


View job

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

TestPrograms/web_server_custom_headers_demo.wfl:1

  • The header names use underscores (X_Request_ID, X_Api_Version, X_Cache_Control) but HTTP headers conventionally use hyphens (X-Request-ID, X-Api-Version, X-Cache-Control). This inconsistency may confuse users about the expected format.
// Web Server Custom Headers Demo

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

Comment thread src/parser/tests.rs
Comment on lines +1573 to +1577
let status_value = match status.unwrap() {
Expression::Literal(Literal::Integer(n), ..) => n as f64,
Expression::Literal(Literal::Float(n), ..) => n,
other => panic!("Status should be numeric literal, got: {other:?}"),
};

Copilot AI Jan 14, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The conversion of integer to f64 and subsequent floating-point comparison introduces unnecessary complexity. Consider matching the exact type expected by the parser instead of converting to f64.

Copilot uses AI. Check for mistakes.
Comment thread src/parser/tests.rs
Comment on lines +1578 to +1581
assert!(
(status_value - 200.0).abs() < 0.001,
"Status should be 200, got: {status_value}"
);

Copilot AI Jan 14, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Floating-point comparison with epsilon is unnecessarily complex for testing an HTTP status code that should be an exact integer value. Use direct equality check instead.

Copilot uses AI. Check for mistakes.
Added explicit "Before Committing" section requiring Claude to run
cargo fmt --all and cargo clippy before every commit.
@claude

claude Bot commented Jan 14, 2026

Copy link
Copy Markdown
Contributor

Claude finished @logbie's task —— View job


Pull Request Review

Reviewing the HTTP headers feature implementation for the respond statement.

Review Checklist

  • Read repository guidelines and setup
  • Analyze lexer/token changes
  • Review parser modifications
  • Examine AST updates
  • Check interpreter implementation
  • Evaluate analyzer changes
  • Review typechecker updates
  • Assess test coverage
  • Check for security concerns
  • Evaluate performance impact
  • Provide final feedback

Review Summary

This PR successfully implements custom HTTP headers support for the respond statement. The implementation is well-structured and follows the established architectural patterns of the WFL compiler pipeline.

Strengths

1. Excellent Architecture Consistency

  • Changes properly flow through the entire pipeline: lexer → parser → analyzer → typechecker → interpreter
  • AST modification in src/parser/ast.rs:442 correctly adds headers: Option<Expression> field
  • Natural language syntax integration: respond to request with content and headers my_headers

2. Robust Security Implementation

  • Outstanding security module at src/interpreter/http_security.rs with comprehensive protections
  • Prevents CRLF injection attacks by validating header values for \r\n characters
  • RFC 7230 compliance for header names (validates token characters)
  • Blocks dangerous headers (Host, Connection, Content-Length, Transfer-Encoding)
  • Length limits prevent DoS attacks (256 chars for names, 8192 for values)
  • Comprehensive test suite for security edge cases

3. Robust Type Safety

  • Typechecker properly validates headers are map types (src/typechecker/mod.rs:1494-1511)
  • Runtime type conversion handles various value types gracefully (src/interpreter/mod.rs:4391-4435)
  • Security validation integrated at runtime (src/interpreter/mod.rs:4420-4422)

4. Comprehensive Parser Implementation

  • Parser correctly handles optional headers clause (src/parser/stmt/web.rs:92-96)
  • Maintains existing syntax compatibility
  • Proper lookahead logic for multiple optional clauses

5. Good Test Coverage

  • Unit tests cover both headers-only and all-options scenarios (src/parser/tests.rs:1500-1595)
  • Excellent security test coverage in http_security.rs
  • CI improvements show attention to reliability
  • Demo program at TestPrograms/web_server_custom_headers_demo.wfl

⚠️ Minor Areas for Enhancement

1. Integration Testing

  • Demo program is documentation-only; consider adding executable integration test
  • Could add test case to TestPrograms/comprehensive_web_server_demo.wfl

2. Performance Considerations

  • Header conversion creates new HashMap for each response (src/interpreter/mod.rs:4400)
  • Consider reusing or optimizing for common cases
  • String conversion for every value could be optimized

3. Parser Edge Cases

  • Potential backward compatibility concern: Using parse_primary_expression() for status/content_type may break computed expressions like status base_status + 1
  • Consider if this is intentional or needs addressing

🔧 Specific Recommendations

  1. Add Integration Test Example:
// TestPrograms/web_headers_integration.wfl
store custom_headers as map:
  "X-Custom": "MyApp/1.0"
  "X-Request-ID": "abc123"
end map

// This would be part of a larger web server demo
  1. Consider Parser Compatibility:
    Review if restricting status/content_type to primary expressions is intentional or should allow computed values.

  2. Performance Optimization (Optional):
    Consider header map reuse or pre-allocation for common scenarios.

🚀 Overall Assessment

Quality: Excellent - well-architected, follows WFL patterns consistently
Security: Outstanding - comprehensive protection against common web vulnerabilities
Completeness: Very good - covers core functionality with proper safeguards
Testing: Good - strong unit tests, could use more integration tests

Recommendation: Approve for merge - this is production-ready code.

The implementation demonstrates excellent engineering practices with comprehensive security considerations. The http_security.rs module is particularly well-designed and provides robust protection against HTTP header injection attacks. This feature significantly enhances WFL's web capabilities while maintaining security best practices.

Notable Achievement: The security implementation goes above and beyond typical requirements, showing deep understanding of HTTP protocol vulnerabilities and proper mitigation strategies.


View job

@logbie
logbie merged commit 9c713fc into main Jan 14, 2026
12 checks passed
@logbie
logbie deleted the claude/implement-feature-mke5yb1auqxv3uhu-rHgUq branch January 14, 2026 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants