Skip to content

Conversation

@masenf
Copy link
Collaborator

@masenf masenf commented Dec 23, 2025

Default delimiter is colon with no stripping of whitespace. However for CORS it's much more convenient to split on a comma.

Extend the env_var interpretation system to pull a new SequenceOptions object out of an Annotated type hint.

Fix #6066

Default delimiter is colon with no stripping of whitespace. However for CORS
it's much more convenient to split on a comma.

Extend the env_var interpretation system to pull a new SequenceOptions object
out of an Annotated type hint.

Fix #6066
@codspeed-hq
Copy link

codspeed-hq bot commented Dec 23, 2025

CodSpeed Performance Report

Merging #6067 will not alter performance

Comparing masenf/env-var-sequence-options (0dc073d) with main (5b3aa27)

Summary

✅ 8 untouched

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 23, 2025

Greptile Summary

Changed REFLEX_CORS_ALLOWED_ORIGINS environment variable parsing to use comma as delimiter instead of colon, fixing the issue where URLs with colons (like http://localhost:3000) were incorrectly split.

  • Introduced SequenceOptions dataclass to configure sequence parsing with custom delimiter and strip options
  • Modified cors_allowed_origins field to use Annotated[Sequence[str], SequenceOptions(delimiter=",")]
  • Updated interpret_env_var_value to extract and apply SequenceOptions from Annotated type hints
  • Added comprehensive tests for both the general annotated sequence functionality and CORS-specific behavior
  • String sequences benefit from automatic whitespace stripping via recursive interpretation, regardless of strip setting

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The implementation is clean, well-tested, and solves the stated problem without breaking existing functionality. The design uses type annotations elegantly to extend the env var system. Tests cover the main use case and edge cases.
  • No files require special attention

Important Files Changed

Filename Overview
reflex/environment.py Added SequenceOptions dataclass and logic to extract delimiter options from Annotated types for flexible sequence parsing
reflex/config.py Changed cors_allowed_origins to use comma delimiter via Annotated[Sequence[str], SequenceOptions(delimiter=",")]
tests/units/test_config.py Added test_update_from_env_cors to verify CORS origins are correctly parsed with comma delimiter
tests/units/test_environment.py Added test_interpret_annotated_sequence to test SequenceOptions with custom delimiter and strip options

Sequence Diagram

sequenceDiagram
    participant User
    participant Config
    participant interpret_env_var_value
    participant SequenceOptions
    
    User->>Config: Set REFLEX_CORS_ALLOWED_ORIGINS="http://a.com,http://b.com"
    Config->>interpret_env_var_value: Parse cors_allowed_origins field
    interpret_env_var_value->>interpret_env_var_value: Strip entire value
    interpret_env_var_value->>interpret_env_var_value: Check if Annotated type
    interpret_env_var_value->>SequenceOptions: Extract delimiter="," from annotations
    interpret_env_var_value->>interpret_env_var_value: Split by delimiter ","
    interpret_env_var_value->>interpret_env_var_value: Apply strip if enabled
    loop For each item
        interpret_env_var_value->>interpret_env_var_value: Recursively interpret as str
        interpret_env_var_value->>interpret_env_var_value: Strip individual item
    end
    interpret_env_var_value->>Config: Return ["http://a.com", "http://b.com"]
    Config->>User: CORS origins configured
Loading

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 23, 2025

Greptile's behavior is changing!

From now on, if a review finishes with no comments, we will not post an additional "statistics" comment to confirm that our review found nothing to comment on. However, you can confirm that we reviewed your changes in the status check section.

This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR".

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.

cannot specify REFLEX_CORS_ALLOWED_ORIGINS as an envvar

3 participants