Add YAML.safe_load config option #1668
Conversation
WalkthroughThe pull request introduces enhancements to the internationalization (i18n) configuration in the React on Rails library. A new attribute Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
spec/react_on_rails/locales_to_js_spec.rb (1)
101-106: Ensure consistency in test setup.This block successfully configures the new
i18n_yml_safe_load_options. Consider verifying that other test contexts also reset or override these values to prevent side effects across tests.lib/react_on_rails/configuration.rb (1)
73-73: Keyword parameter inclusion.Accepting
i18n_yml_safe_load_optionsin the initializer helps centralize configuration withinConfiguration. Make sure to document this new parameter in the project's README or CHANGELOG.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
lib/react_on_rails/configuration.rb(3 hunks)lib/react_on_rails/locales/base.rb(1 hunks)spec/react_on_rails/fixtures/i18n/locales_symbols/de.yml(1 hunks)spec/react_on_rails/fixtures/i18n/locales_symbols/en.yml(1 hunks)spec/react_on_rails/locales_to_js_spec.rb(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- spec/react_on_rails/fixtures/i18n/locales_symbols/de.yml
🔇 Additional comments (7)
spec/react_on_rails/locales_to_js_spec.rb (3)
100-100: Clean separation of test context.
Introducing a dedicated example group describing "with symbols in yaml" is a clear, well-structured way to isolate this test scenario.
108-113: Good teardown practice.
Restoring the configuration to its previous state helps maintain independence between test examples.
115-117: Effective test coverage for symbol-based YAML.
This test confirms that loading locale files with symbol keys does not raise errors, ensuring the new option is functional.
lib/react_on_rails/locales/base.rb (1)
118-119: Leverage safe Load Options.
The introduction of safe_load_options effectively integrates user-defined YAML loading parameters. Consider adding a fallback or exception handling strategy if a needed class is not permitted.
lib/react_on_rails/configuration.rb (2)
55-55: New configuration attribute recognized.
Adding :i18n_yml_safe_load_options to the attr_accessor allows flexible YAML loading settings.
84-84: Storing the YAML safe load options.
Assigning the incoming i18n_yml_safe_load_options to the instance variable ensures that user-defined loaders can be honored throughout the application.
spec/react_on_rails/fixtures/i18n/locales_symbols/en.yml (1)
1-8: Useful YAML file for symbol testing.
Providing example keys (symbols) validates real-world usage of the new safe_load_options. The content is clear and sufficient for verifying symbol-based locales.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/guides/i18n.md (1)
34-37: Enhance documentation with security context and examplesThe documentation would benefit from additional context about YAML safety. Consider expanding it to:
- Define what constitutes "unsafe YAML"
- Provide examples of unsafe YAML content
- Add a security note about carefully choosing permitted classes
Here's a suggested expansion:
-4. If your locale files (or one of the gems locale files) contains unsafe YAML, you may need to configure `config.i18n_yml_safe_load_options` if you can't fix such yamls properly. +4. If your locale files (or one of the gems locale files) contains unsafe YAML (e.g., YAML with Ruby symbols or custom objects), you may need to configure `config.i18n_yml_safe_load_options` if you can't fix such yamls properly. + + Example of unsafe YAML content: + ```yaml + en: + status: !ruby/symbol success + custom_object: !ruby/object:MyClass {} + ``` + ```rb config.i18n_yml_safe_load_options = { permitted_classes: [Symbol] } ``` + + Note: Be cautious when permitting classes as it may pose security risks. Only permit classes that you trust and require.docs/guides/configuration.md (1)
216-218: Add context and link to i18n guideThe configuration option would benefit from a brief explanation and a reference to the detailed documentation.
- # Possible YAML.safe_load options pass-through for locales + # Options for YAML.safe_load when parsing locale files + # See docs/guides/i18n.md for detailed usage and security considerations # config.i18n_yml_safe_load_options = { permitted_classes: [Symbol] }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
CHANGELOG.md(1 hunks)docs/guides/configuration.md(1 hunks)docs/guides/i18n.md(1 hunks)lib/react_on_rails/locales/base.rb(1 hunks)spec/react_on_rails/locales_to_js_spec.rb(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- spec/react_on_rails/locales_to_js_spec.rb
- lib/react_on_rails/locales/base.rb
🔇 Additional comments (1)
CHANGELOG.md (1)
36-36: LGTM! Changelog entry is well-formatted and properly positioned.
The changelog entry follows the contribution guidelines, clearly describes the new feature, and includes proper attribution.
Summary
YAML.safe_loadto handle special yaml files.Pull Request checklist
Remove this line after checking all the items here. If the item is not applicable to the PR, both check it out and wrap it by
~.Add the CHANGELOG entry at the top of the file.
Other Information
Remove this paragraph and mention any other important and relevant information such as benchmarks.
This change is
Summary by CodeRabbit
New Features
stream_react_componentImprovements
registerStoremethod toregisterStoreGeneratorsDocumentation