Change filter on /rails/info/routes to use an actual path regexp from rails

[brainopia]

Change filter on /rails/info/routes to use an actual path regexp from rails
and not approximate javascript version. Oniguruma supports much more
extensive list of features than javascript regexp engine.

Fixes #18402 (by removing json_regexp which breaks down on complex regexp patterns)

cc @schneems @rafaelfranca

[brainopia]

Example of filtering on a complex pattern (which breaks /rails/info/routes page and rake routes on master)

[arthurnn]

@schneems what do you think?

[rohitarondekar]

I've only given a cursory look at the PR but in general I agree with the direction. The conversion of Ruby compatible regexp to JS compatible is fragile and will be prone to errors. Also rake routes is an important command that ought not to break just because there is a complex regexp.

Another alternative will be to catch the error during conversion and revert back to original regexp. Basically if you get till the conversion the original regexp is fine but converting it is not working out so print the regex as-is. Of course this leads to inconsistency and might be surprising.

[schneems]

Thanks for the work, and sorry for the delay. This is a pretty big change for a pretty small bug. As you noted, this page doesn't have any tests, so i've hesitant to merge in large changes. Moving the processing to the server side is a bit heavy handed. I'm 👎 on it as is. Can we modify the regex sanitizer to remove comments?

[brainopia]

The ruby regexp engine supports much more features than javascript version.
Why try to achieve a working approximation when exact match can be achieved?

If you worry about complex logic, then you shouldn't. I've removed more lines of code than I've added and the logic is straightforward.

[rafaelfranca]

Also we want to make it possible to use the fuzzy match in the rake tasks
so it is better to have to logic at Ruby side.

On Wed, Feb 18, 2015, 21:33 Ravil Bayramgalin notifications@github.com
wrote:

The ruby regexp engine supports much more features than javascript version.
Why try to achieve a working approximation when exact match can be
achieved?

If you worry about complex logic, then you shouldn't. I've removed more
lines of code than I've added (including a big changelog entry) and the
logic is straightforward.

—
Reply to this email directly or view it on GitHub
#18434 (comment).

[schneems]

I'm convinced. I'll test out the implementation locally in the next day or so.

Is there anyway to test any of this? Are we doing integration style tests on any of the other internal controllers?

[rafaelfranca]

Maybe unit test in the fuzzy match class? But I think we have tests to the
info controller

On Wed, Feb 18, 2015, 21:42 Richard Schneeman notifications@github.com
wrote:

I'm convinced. I'll test out the implementation locally in the next day or
so.

Is there anyway to test any of this? Are we doing integration style tests
on any of the other internal controllers?

—
Reply to this email directly or view it on GitHub
#18434 (comment).

[schneems]

I verified and it works as promised. We should add a test to https://github.com/rails/rails/blob/master/railties/test/rails_info_controller_test.rb to test the results of querying for a partial and full match return as expected.

This commit did break the routing error page: https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.html.erb

Basically whenever you get a routing exception you get a very similar view to rails/info/routes that lets you debug the problem on the same page. It needs to be updated to use the new server architecture

[brainopia]

@schneems Thank you for feedback!

I've fixed the issue with routing_error.html.erb page (I was using a relative path in javascript, changed it to absolute to work from arbitrary locations).

And I've added tests.

[schneems]

The error page works, your page works. I agree this is cleaner and simpler than my first version. Thanks for the PR, merging in ❤️ ❤️ ❤️

[brainopia]

Thank you 🙇