Skip to content
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

Fix escaping in OptimizedFileSystemResolver #37119

Merged

Conversation

jonathanhefner
Copy link
Member

@jonathanhefner jonathanhefner commented Sep 3, 2019

OptimizedFileSystemResolver builds a regular expression to match view template paths. Prior to this patch, only file globbing special characters were escaped when building this regular expression, leaving other regular expression special characters unescaped.

This patch properly escapes all regular expression special characters, and adds test coverage for paths that include these characters.

Fixes #37107.

Copy link
Member

@jhawthorn jhawthorn left a comment

Choose a reason for hiding this comment

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

Thanks! ❤️

Left one suggestion for tests.

actionview/test/template/resolver_shared_tests.rb Outdated Show resolved Hide resolved
`OptimizedFileSystemResolver` builds a regular expression to match view
template paths.  Prior to this patch, only file globbing special
characters were escaped when building this regular expression, leaving
other regular expression special characters unescaped.

This patch properly escapes all regular expression special characters,
and adds test coverage for paths that include these characters.

Fixes rails#37107.
@jonathanhefner jonathanhefner force-pushed the fix-escaping-in-view-path-resolver branch from 18100e0 to 897cdc7 Compare September 9, 2019 17:20
@jonathanhefner
Copy link
Member Author

@jhawthorn Thank you for the review! I've made the suggested change.

@eugeneius
Copy link
Member

I backported this to 6-0-stable in 4e3fd92, since it fixes a regression from 5.2 that has been reported again twice since it was merged (#37641, #38415).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants