Skip to content

Commit

Permalink
Change ForceSSLHandler#secure? to use == instead of regex
Browse files Browse the repository at this point in the history
It's more clear and 52x faster in majority of scenarios (matches) and
24x faster on redirects (mismatches)

It also does remove case insensitive searching, but it discussion on the
PR #1662 stated this should be okay functionality to change

Benchmark:

```crystal
require "benchmark"

puts "Matches"
Benchmark.ips do |x|
  x.report("String ==") do
    "https" == "https"
  end

  x.report("!! String =~ /https/i") do
    !!("https" =~ /https/i)
  end
end

puts "\n\nMismatches"
Benchmark.ips do |x|
  x.report("String == mismatch") do
    "http" == "https"
  end

  x.report("!! String =~ /https/i mismatch") do
    !!("http" =~ /https/i)
  end
end
```

```plaintext
➜  tmp crystal build --release downcase-includes-vs-regex.cr && ./downcase-includes-vs-regex
Matches
            String == 897.53M (  1.11ns) (± 6.76%)   0.0B/op        fastest
!! String =~ /https/i  17.13M ( 58.36ns) (±16.32%)  16.0B/op  52.38× slower

Mismatches
            String == mismatch 881.32M (  1.13ns) (± 8.78%)   0.0B/op        fastest
!! String =~ /https/i mismatch  36.42M ( 27.46ns) (± 1.47%)  16.0B/op  24.20× slower
```
  • Loading branch information
grepsedawk committed Feb 9, 2022
1 parent ecf19a0 commit b0ae752
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/lucky/force_ssl_handler.cr
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class Lucky::ForceSSLHandler
end

private def secure?(context) : Bool
!!(context.request.headers["X-Forwarded-Proto"]? =~ /https/i)
context.request.headers["X-Forwarded-Proto"]? == "https"
end

private def redirect_to_secure_version(context : HTTP::Server::Context)
Expand Down

0 comments on commit b0ae752

Please sign in to comment.