Avoid unnecessary double string split in ActiveSupport::MessageVerifier#verified #42919

jcmfernandes · 2021-07-30T17:21:53Z

Summary

ActiveSupport::MessageVerifier#verified is causing signed_message to be split twice: first inside #valid_message? and then inside #verified. This is ultimately unnecessary.

Benchmark

I renamed the current implementation of #valid_message? and #verified to #valid_message_old? and #verified_old respectively, with the latter modified to call #valid_message_old?. I then ran the following benchmark:

data = SecureRandom.alphanumeric(SIZE)
signed_message = ActiveSupport::MessageVerifier.new("Hey, I'm a secret!").generate(data)

GC.start

Benchmark.ips do |x|
  x.time = 10
  x.warmup = 2

  x.report('new verified') do
    @verifier.verified(signed_message)
  end
  x.report('old verified') do
    @verifier.verified_old(signed_message)
  end

  x.compare!
end

Benchmark.ips do |x|
  x.time = 10
  x.warmup = 2

  x.report('new valid message') do
    @verifier.valid_message?(signed_message)
  end
  x.report('old valid message') do
    @verifier.valid_message_old?(signed_message)
  end

  x.compare!
end

With SIZE = 2**12 (4 kB, approximately the maximum size of a cookie):

Comparison:
        new verified:    28808.8 i/s
        old verified:    25400.1 i/s - 1.13x  (± 0.00) slower

Comparison:
        new verified:    37774.6 i/s
        old verified:    32482.2 i/s - 1.16x  (± 0.00) slower

Comparison:
        new verified:    38709.1 i/s
        old verified:    33416.9 i/s - 1.16x  (± 0.00) slower

Comparison:
        new verified:    38629.5 i/s
        old verified:    32879.7 i/s - 1.17x  (± 0.00) slower

Comparison:
   old valid message:    70262.7 i/s
   new valid message:    69663.3 i/s - same-ish: difference falls within error

Comparison:
   old valid message:    69522.6 i/s
   new valid message:    68408.8 i/s - same-ish: difference falls within error

Comparison:
   new valid message:    69642.7 i/s
   old valid message:    69283.1 i/s - same-ish: difference falls within error

Comparison:
   new valid message:    69779.8 i/s
   old valid message:    69510.5 i/s - same-ish: difference falls within error

With SIZE = 2**22 (4 MB):

Comparison:
        new verified:       75.0 i/s
        old verified:       49.9 i/s - 1.50x  (± 0.00) slower

Comparison:
        new verified:       70.0 i/s
        old verified:       49.1 i/s - 1.43x  (± 0.00) slower

Comparison:
        new verified:       74.6 i/s
        old verified:       49.7 i/s - 1.50x  (± 0.00) slower

Comparison:
        new verified:       40.1 i/s
        old verified:       32.2 i/s - 1.25x  (± 0.00) slower

Comparison:
   new valid message:      133.3 i/s
   old valid message:      131.3 i/s - same-ish: difference falls within error

Comparison:
   old valid message:      131.5 i/s
   new valid message:      131.4 i/s - same-ish: difference falls within error

Comparison:
   new valid message:      133.3 i/s
   old valid message:      132.1 i/s - same-ish: difference falls within error

Comparison:
   new valid message:      132.6 i/s
   old valid message:      130.7 i/s - same-ish: difference falls within error

As expected, with a larger signed message, a greater speedup.

Other Information

None.

activesupport/lib/active_support/message_verifier.rb

p8 · 2021-08-25T19:17:45Z

activesupport/lib/active_support/message_verifier.rb

+        signed_message.split("--")
+      end
+
+      def _valid_message?(data, digest)


As we no longer have a message here, what do you think about something like:

Suggested change

def _valid_message?(data, digest)

def verified_digest?(data, digest)

Makes absolute sense! With that said, I'm not the biggest fan of the way it reads. It seems to suggest that we already did something with the digest. What do you think of #verifiable_digest? or #matching_digest? or #digest_matches_data?? I must confess that I have a slight preference towards the last.

#digest_matches_data? looks good to me!

Awesome, it's done! 😉

p8 · 2021-08-31T07:54:57Z

@jcmfernandes I was looking at a sample message and the digest is always at the end.
It would be nice if we could search from the end of the message instead of going through all the data first.

require 'benchmark/ips'
signed_message = "BAhJIgIAEDQ4UEFlbFZFTmJGcUVJZWJNWnQ3azdwVGp6N0lpdkNTTzdob1kxcDBvZlltbWJYUks2bGVUNnMza2V4U0hRVEVpNkYwclhiWmZnb2k5NTkzVEdqeFFSdlQ2RkE1bmg0RlN0cGgxYzFIM3hOaHRwUXlvcDJHdGZQY3lycHlWa1ZodVI1VkZTUmpvTUN6TEZTcGxaVjV4OEloVzk0OUZMa1lqbm5Oc1gybFNQekJIeEVrOUpnQjUzZHFqRnhhM1FKMENYVUlkQ3Q5NDJ2bFpDNkE5T05ITnAxNTU2YTFjSVpDRjdyU2NpZ21JTmVMV0M5Y1c4Y3hWOXVWQVZZU254b3plYVlTSUt3SEVBVUtROFNVdUVvdXZaM0NuVm1tdWIwQ1MwNnJwQXBwS2FQSmE5Nm5Db0Jhb3dodmR0QW5ucG82cDdNbEJtRzZ6MG5JWEVTV1ZkaFhGRGl2MG1QaFNZR3JsR055clVzNThxelJUbUFPZ21VaTM1TTBDaG82QUxzY0ViQjAxaEZ5WTVzc2pyd3Z6TVlLb3k3ak1jNGVvV1dLZGYwM2tRQjdyZHpLanRqUnRaYzZYTUVVRGRrNUpuWG1kZlI4cEw5NzY5MnkyTFVnQVdFQkJrWTMwTEYyMlFROVhTVGhXU0MxVzkwWWd4OTBVWm5aTEFDMkZ0SjBQc0ZxWFQ5Tk8zOGhmZGZaM3p0bXVBNm5seUNhYkdOTlZFTnFwbmVYMDNFT0UyaEZJUEJZdnFFaDRMSVl5Nk94YlZBajRNd2dIRGZ0WGluaGRudFpyWUFLaXpOQ0ZqcTc5aU1nTTdqcTNWV1ZGcm1VQWdON1dkdnZjVG0wanY5ZWU5bzAydmhkbDVHZURuTDI2VzcyQ1lnc3R6NGl4RDhYV3RRMUxCcXdsMFg4QzlpMkRKUGdYY0VSUkZ6QkhFNDFQRm1NY1ozb0V0bTM2bm8wMkQwSmlWM3UxWnVMaWlLRmVybjFIMmRSeHQ2dzkxM01HVlVaRTVhZDlUb3JuSUpTQUVHb2NQV2R2SWxGcUxMeEE1UGNBSTVJT25yemRhSU1ZRDdxQTNUMVJIenl0RFJxdFlvZTF0QVpXSjM5UTNIUENOdlZOcElPd1lBaHdudWFRd2JKVU5RN3JENmRyUkVpdzdWbEJwSFk4NjhQdzN0d2l6bVdsZnVIT0R3SThhQkxtdUliTjBLOEl4SGM4dUJPTTFjWXkwdFh1QjAwRm5uUmJaVm1tUlpjS2xrQkpCU1poYkVhWGFvZ3cxQVVZalppTVM0TXNMdUNlV2JWN2J5RVFCNWJuQzhlcGJiOEdVaUVPTFdnUTJUSUl6czZ4eEpXZWR0U3NMZk9IbTlzcmhXSGxkeWlNUms5ZWVPZTNyUmhIMDZyc0l0VUtSNW9oRXowTlJVcTExTXg0cDluTmtjMXNxZk9IVWVBcVBCOWVpVnZNdWhudTB1RXYydENramo0Wm5YVnd2V2Y1TmNoR3lWaFdkRzdseEhHaTUzVWxnSGY1M2V3bmliQnRCU00yWHlJaG9YYmQ2Y1F1eGJFOUhaWVJFbHFmUFZBSzJmTG5kY3JsVU5XTVdNNGp1czc3dnZVb0ZtNVNkUkJyVVlmSFN1MkNpb0F0Um50UDNKamxmZDVnUWZER21TS202d21PSEM0S2d4RUh5TXhTdmlNTVZFR1J1UEpPbUFDZ2kyTVYxbms1d3doM3NyR0k5MG5ZdUZ4VUJVdTFPNWxQSUJtYXYyY0EyTXpvOEVLWjVseWlscjNnRlZKbW9ZTDFvZml4Y1ZMQUNxcUFpQkxmaTd4Tk1HQmZSSTlMdDA0TDFOYVpwVHJ4MHlpOWczaE00Q2JuMmlYeUlNMTBJZ1h1YkxlUVJTYlpDeHhnTDZUWGd1bERGOVcya21oMjRlWEl0UmRPWkxXRWNadTBuWkg1T3pWTTBZd25sdEpERldNTG9jWjF3QTJNOFJkSzlmdHpiMTdXWmhnM2RYSkM0QU1rRzBpeXc1bVdjUWVweko0V2hpRnNlVng3QnJJd1ZqZFZTSDRYU1V6R0lnMDZuN2NtVmJ2Z0tDaHhDYlNnZ3JGMHc1blI1Y1hkajNQVVdreFRPYlFsOFBTcFR1S1pIZWp5NVBNVW9zWWdZbVpZaVg2N0VXQk9VbElQNWJhNnB6Vm02UWt3MWNPbzQxUjIxcHFoZ3RWRlU3T3NLekd6YU5lQzFCRnVNYWFONWNNZ01LcnNTUUY1Mm5rUTRUR2dFN0FENTBScXdsS255UHJBVjVBdGwyNEJaV3JWaWFIbTEwSTNPQW1xaE56cXlCelozTHlNb1pvNkZwRjlxUEZSa09zbldGNzFQVUQ5ZGdQQWd3T3FnT2tzaUN2VG0wVUhMenBnUnVVcXZCZW10MjRKTlB5QVhnS0FZV3h6R3ZydXVyemc2cEZhZDI3dWthYWIzckVVUHhxdzdqTU5YSHRWZ05lSnR4cGNMVDB6dTBuTFdGNkROalhXeVNZUEV6Q1YyZ3NXQkRzRHpvellHN3NDUXdtTnB4dExRd21XM3RsQ09MMmZZMVQzU0dQcUZqSzkxMXZOUERhRE5xb3NxTzUzZDlvMVA4ZHJPV3J4WVYzaGxlSWowTUxGbUQ1d0kyQkxJMmxBeHRnMlRWV0hCT1FESGF3allDbDJvRjN3Q05seXZzekltZjk4R3VNMFcyd1p6YWdhQ1NQWE1NSVZSeURWMGpaeHEwam5PR2E0OXhnQTdUM1lHWnBxSlg2ajJlWThhVFVTVFp0TDRzNkJiRUU4ZmxGYUJDQ1hnQ0dIdmdwR1RKSWdXRTVKUGlkc0IwdER2WjgxemF3RFVNUXVLR3NZRFVKNjNta0c2NjlNZEcwZUxxT0lKODU2UkNQeG43VGxyTjBPN0RyM3J3ZVJTSFJOZlc3eVhYbTlLSEZUWjJQSEllaGtOUGNtRTBydFc0MXdXMWRZblRWMnYzZnNtVVZLWFFlckhOY0YxdVJvNFhQdExxeU5iN2JTSXZKa0tpQ2JraEFCY2V1ajlYb1hjZkFzOUVmOWJ4WURYSko4bU1vOFJvdEFZNE9sZ1Q5dmVpVW4xZzNZUUMyUnhtT0NQQkVGd2ZsN2pqTmg5ZDNwNzBCTnk2d1NoeERMM2pwZmhNdVE2VkQ4bE1XdVlKVXNlczdYNFlXN1drTFI5VXpzVmFmV0w3V0d4SWNneWRXU3M0MjlxeDlpMGhVcWRGb2RwNDA5ODFxc0h2YUo3eHhRVW5LQ0M1N05tdlJjeERwdmN0OFhxUTZkMFE3b1F6dGY3VEJaOEd2VldxV2VpWkFDRWliWUZ0ZWljYnB3Y29xcm9VRjdvVnIxcVVuaEwycUFtQVlsZEtzVVBQWHpNN0MxYmtjTTJ0QzVhMlJPdzRrR090Zk5TU09ERE0zaU5EdnE2Y3N2TzB3WTdOcDhqcHJUaDRDekFEM3hhTFM3UkNmVkx6bFEzc1d6NUt3Qko5ZGxRQnZqZVp3MXFDc2F0UnhpZDl2aXY3b0g2U2JoaEpJdVFLaGhleG80bHc1TlI4TmRhc0Z3RXBiTkl6NEZRcE9uS2dnYWNzNDQxbmEyNm9oTXd5SHBZd0JHY09KMEtDYkNMZ0UzTnZwZWxoVFRGRXpPUG5heVV6SmdtcHEybFZsWmV1d1d4dXB4MENHUWRxdjVsNXVVcGxuMHNmelNBaEQxWW9qcFRaRGZtVmFwanNMMm1NM2dyQjZ4Nmh2VUl5c0c1Mk5pRkowT2hLa2lIYlVldE5CU2lieEd5dDN3Q1N6bEhpdnVSQjFOa0lmVmgxT2x6SWh4cGZXdGlZNEdwUHlkdEFQcFZid2NLQUh6WTZxSEdlaWVnamFkaTJweVY4bDBFcWxLM3JVNDVIa3VhWnNjcnhCNjUwUTlJQzNQWnRKOEtRd3pqQk50d1piYlcwcHc2RENQeHpURXJlOEVZNzQ2OXZXSHJtTXlwN0d3S0VxdDJ4bWZTQXNjV0I5TzlaaXBhWk05ZmVNaUtFdjZmdlhwUlFGbVdPcGgycVFaNXhEMjlQRUZHU3daREtGNUJlbVhvUzF2SGVWZE9iSU1EbldIUk1RY1VkRWZLa3RMOUpRVkk3WXRlUUhnTENrNTU1MThET2tYY0FaNUpFS0w0aEc4eWlGQWZGdWljeHZuaE45ZnZHZXhwbENzY1ZmTGdSS0pTMlZ3c203MHpsT1BZSXowN0FUNDJuRGdBQzZkZ2ZnbHBlTnE2bHI1Wm1lTnpjMTE2UzB4RXFhMHFTcmljdW1HVFIwTVpzWWV1emFyTEV2OFpsS3QzUzhEQmdSNG40QlJNWHZVbkpyT2Y2aUJZMmVFcTRoTEFYTDVVQjVCVFpQYzRNdWVISmRMWjBSNEFqU0NLUmxWamJnbjdaVUZKcTRwRUtjZVlCNWpUTmZPU2Q2OUxlZjdERVZnSzlOUjZqelJ4ZHRmdU10cmFsYlFYbTVKeUhUbzh5VmFkWHM2V2lRZUxaR0pUenhiM2ZJdm5SbWgybnN5cHkwc0I4Z0dMeVNNQVQ4R2xxc3BVbjZ4R1N2bHlPV0JNajBJV0hCaTRZd2NpaWZ0eXhyN0syamVpMEdxb1RURlRwYTRLNVhBQ3dhd0JwSzYwZXViek95SkR4TFFTT090MXA3UnJWWnJISFozSVZuU1Ywc1N2Uk1rU0lBNmNwNzhLcFFnNzVQM3NKWnZucmhWamphRjNqWFE2aDB5cWVoZVE3dWJlWGhmQmdFVHNsbHRaRFg2bXZwTkJKUUUwREthSWJ6QmFLaFlJQnBncXBhd1p0cnJRUGY4YVZwa0RIODU3VU8zZXpWanVBVE0zVDNRcTJqZ1BZNW1GSHlwUGlpTzJSc2ZYQU9FQndZRW94RHpwM1o2NkZQMEEzYzhRaDZ4VnVOdkdxQXdQSFI1YlhWY3ZHUndoYXRieTRydXNzSmUwNDc3SDZKVVY0UW9WeHBPQUxmQTdVT3BSN3FDQUdDcXpUSXRFUDhleVNFZWRpcVhyeUh1OGo0N2UwUUJkTzI4MFBQM2J5aXdiSmRmcXY3bmRFNFRIQk5QNjBXbTBlY0ZUbVI0M21Gcm8yMVM1emV2b0ZJWm5mQURNRzNVcHdZemFoYUFMdFFsQmUwQUV5Z21xeE9XQllDNWUxRjNUUXNPb1ZKbW9SWERxVlVzaWxrZGFaMW9pTVdhNFVTNTlZdnBIREpSZ1RXSEFKbDNVVjZWYlZrc0FDWnc2YXFCeEdPUkxEdW8wMDBlbkI4aGlXMHBKMlZESHlCNzJPOVVSWmFscktUZFVRb0pBT2EyQlQ0SU9HWnVoSFRNbmN1NzREbWpHUFFGcWJNYU9Da1RDc3JMZmZMVWxyS3dWZDBTVXVTU1lJZ3J5enlUVkdCc2lDWkRObmE3Um5QUlVQSTJ5QU9sYU54b3JHbnlWeTJLeDhFQkVDZDVmbjJ6bnBwWmxJYlB6RTczbUF5Y3J4b1VnamIGOgZFRg==--60ae7b3ca695673426771dfb5e817d43718f3b92"

GC.start

Benchmark.ips do |x|
  x.time = 10
  x.warmup = 2

  x.report('split') do
    signed_message.split('==', 2)
  end

  x.report('reverse split') do
    signed_message.reverse.split('==', 2).map(&:reverse)
  end

  x.report('split with regexp') do
    signed_message.split(/(.+)--([^-]+)$/)
  end

  x.report('rindex') do
    index = signed_message.rindex("==")
    signed_message[0, index]
    signed_message[index+2, signed_message.length]
  end

  x.report('rindex with ranges') do
    index = signed_message.rindex("==")
    signed_message[0...index]
    signed_message[index+2..-1]
  end

  x.compare!
end

Warming up --------------------------------------
               split    30.599k i/100ms
       reverse split    20.604k i/100ms
   split with regexp     3.620k i/100ms
              rindex    61.198k i/100ms
  rindex with ranges    49.064k i/100ms
Calculating -------------------------------------
               split    304.998k (± 5.9%) i/s -      3.060M in  10.069927s
       reverse split    232.232k (±13.2%) i/s -      2.287M in  10.061747s
   split with regexp     25.875k (±17.6%) i/s -    249.780k in  10.081676s
              rindex    380.131k (±20.4%) i/s -      3.672M in  10.135841s
  rindex with ranges    375.177k (±19.1%) i/s -      3.631M in  10.120255s

Comparison:
              rindex:   380131.4 i/s
  rindex with ranges:   375177.3 i/s - same-ish: difference falls within error
               split:   304998.4 i/s - same-ish: difference falls within error
       reverse split:   232231.5 i/s - 1.64x  (± 0.00) slower
   split with regexp:    25874.9 i/s - 14.69x  (± 0.00) slower

jcmfernandes · 2021-08-31T10:49:40Z

You're absolutely right @p8! In fact, I believe we can do better. We know what digest is being used, so we can know how many chars can it take to be represented in base64 (worst case). That means that we can use #index with an offset of something like signed_message.length - maximum_digest_length - 2. Makes sense? I'm going to investigate.

EDIT: I was forgetting that the HMAC is being represented in hex but the logic is the same.

activesupport/lib/active_support/message_verifier.rb

jcmfernandes · 2021-09-01T15:17:39Z

activesupport/lib/active_support/message_verifier.rb

+        return if signed_message.nil? || !signed_message.valid_encoding? || signed_message.empty?
+
+        separator_index = separator_index_for(signed_message)
+        return unless signed_message[separator_index, SEPARATOR_LENGTH] == SEPARATOR


Here we check for the presence of the separator. Worth extracting to a method?

activesupport/lib/active_support/message_verifier.rb

jcmfernandes · 2021-09-01T16:37:20Z

First, ignore the ugly commit history as I'll fix that before we merge.

What the benchmarks show is the much-increased resistance to invalid messages, those that don't feature the separator. The new code is not sensitive to the size of the input whenever it doesn't feature the separator.

Using #rindex to find the separator provides a similar performance when the separator is there. This makes sense, as it will have to cover at most 128 bytes when using SHA512. However, it's much slower when dealing with invalid messages that don't feature the separator.

So, we are showing a ~1.4x speedup for 4 kB valid messages and a ~1.8x speedup for 4 MB valid messages. When dealing with invalid messages, we are looking at ~3x to ~10x speedup for 4 kB messages and over 1000x speedup for 4 MB messages.

I dropped a few comments in the PR and would appreciate your feedback @p8. Thank you! 🙌

4 kB

-- OLD
Warming up --------------------------------------
           #verified     3.554k i/100ms
     #valid_message?     7.414k i/100ms
Calculating -------------------------------------
           #verified     36.783k (± 1.4%) i/s -    369.616k in  10.050797s
     #valid_message?     75.039k (± 1.7%) i/s -    756.228k in  10.080698s

-- NEW
Warming up --------------------------------------
           #verified     5.081k i/100ms
     #valid_message?    10.109k i/100ms
Calculating -------------------------------------
           #verified     53.090k (± 1.7%) i/s -    533.505k in  10.052230s
     #valid_message?     98.483k (± 1.1%) i/s -    990.682k in  10.060535s

4 MB

-- OLD
Warming up --------------------------------------
           #verified     6.000  i/100ms
     #valid_message?    15.000  i/100ms
Calculating -------------------------------------
           #verified     68.867  (± 2.9%) i/s -    690.000  in  10.030612s
     #valid_message?    158.845  (± 1.9%) i/s -      1.590k in  10.014114s

-- NEW
Warming up --------------------------------------
           #verified    12.000  i/100ms
     #valid_message?    27.000  i/100ms
Calculating -------------------------------------
           #verified    127.487  (± 2.4%) i/s -      1.284k in  10.079559s
     #valid_message?    271.107  (± 3.0%) i/s -      2.727k in  10.067583s

4 kB INVALID MESSAGE

-- OLD
Warming up --------------------------------------
           #verified    26.000k i/100ms
     #valid_message?    36.977k i/100ms
Calculating -------------------------------------
           #verified    259.917k (± 1.8%) i/s -      2.600M in  10.006742s
     #valid_message?    368.768k (± 0.4%) i/s -      3.698M in  10.027311s

-- NEW
Warming up --------------------------------------
           #verified    67.852k i/100ms
     #valid_message?   249.353k i/100ms
Calculating -------------------------------------
           #verified    676.537k (± 1.8%) i/s -      6.785M in  10.032982s
     #valid_message?      2.529M (± 2.1%) i/s -     25.434M in  10.061480s

4 MB INVALID MESSAGE

-- OLD
Warming up --------------------------------------
           #verified    48.000  i/100ms
     #valid_message?    48.000  i/100ms
Calculating -------------------------------------
           #verified    483.897  (± 0.8%) i/s -      4.848k in  10.019468s
     #valid_message?    481.756  (± 2.3%) i/s -      4.848k in  10.068306s

-- NEW
Warming up --------------------------------------
           #verified    66.592k i/100ms
     #valid_message?   255.351k i/100ms
Calculating -------------------------------------
           #verified    677.241k (± 2.2%) i/s -      6.792M in  10.034678s
     #valid_message?      2.503M (± 2.5%) i/s -     25.024M in  10.004185s

rails-bot · 2021-12-01T08:59:57Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

jcmfernandes · 2021-12-06T14:42:17Z

Hey @p8 any changes I should make here to get this over the line?

activesupport/lib/active_support/message_verifier.rb

railties/test/application/middleware/cookies_test.rb

ActiveSupport::MessageVerifier#verified is causing signed_message to be split twice: first inside #valid_message? and then inside #verified. This is ultimately unnecessary. We can avoid String#split all together by calculating the indexes of the data and the digest in the payload. This increases the resistance of the solution against ill-formed payloads that don't include the separator.

…-message-verifier Avoid unnecessary double string split in ActiveSupport::MessageVerifier#verified

p8 · 2021-12-09T12:37:57Z

Nice work @jcmfernandes !

jcmfernandes · 2021-12-10T11:40:17Z

Thank you for your help @p8! 🙌

We can avoid using String#split by calculating the indexes of the encrypted data, IV, and auth tag in the payload. This increases the resistance of the solution against ill-formed payloads that don't include the separator. This is a follow up to the work in PR rails#42919.

rails-bot bot added the activesupport label Jul 30, 2021

p8 reviewed Aug 25, 2021

View reviewed changes

activesupport/lib/active_support/message_verifier.rb Outdated Show resolved Hide resolved

p8 reviewed Aug 25, 2021

View reviewed changes

jcmfernandes requested a review from p8 August 30, 2021 20:04

rails-bot bot added the railties label Sep 1, 2021

jcmfernandes commented Sep 1, 2021

View reviewed changes

activesupport/lib/active_support/message_verifier.rb Show resolved Hide resolved

jcmfernandes commented Sep 1, 2021

View reviewed changes

activesupport/lib/active_support/message_verifier.rb Show resolved Hide resolved

rails-bot bot added the stale label Dec 1, 2021

rails-bot bot removed the stale label Dec 6, 2021

rafaelfranca reviewed Dec 6, 2021

View reviewed changes

jcmfernandes force-pushed the avoid-double-string-split-message-verifier branch from 22f44f5 to d9558e7 Compare December 7, 2021 09:42

jcmfernandes force-pushed the avoid-double-string-split-message-verifier branch from d9558e7 to 02eda1b Compare December 7, 2021 09:52

jcmfernandes requested a review from rafaelfranca December 7, 2021 10:31

rafaelfranca merged commit de065c9 into rails:main Dec 7, 2021

rafaelfranca added a commit that referenced this pull request Dec 8, 2021

Merge pull request #42919 from jcmfernandes/avoid-double-string-split…

7e3bd8b

…-message-verifier Avoid unnecessary double string split in ActiveSupport::MessageVerifier#verified

jcmfernandes mentioned this pull request Dec 17, 2021

Improve the performance of ActiveSupport::MessageEncryptor #43924

Merged

miquelbarba mentioned this pull request Aug 17, 2022

activestorage debug miquelbarba/rails#1

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avoid unnecessary double string split in ActiveSupport::MessageVerifier#verified #42919

Avoid unnecessary double string split in ActiveSupport::MessageVerifier#verified #42919

jcmfernandes commented Jul 30, 2021 •

edited

p8 Aug 25, 2021

jcmfernandes Aug 26, 2021

p8 Aug 30, 2021

jcmfernandes Aug 30, 2021

p8 commented Aug 31, 2021

jcmfernandes commented Aug 31, 2021 •

edited

jcmfernandes Sep 1, 2021

jcmfernandes commented Sep 1, 2021

rails-bot bot commented Dec 1, 2021

jcmfernandes commented Dec 6, 2021

p8 commented Dec 9, 2021

jcmfernandes commented Dec 10, 2021

	def _valid_message?(data, digest)
	def verified_digest?(data, digest)

Avoid unnecessary double string split in ActiveSupport::MessageVerifier#verified #42919

Avoid unnecessary double string split in ActiveSupport::MessageVerifier#verified #42919

Conversation

jcmfernandes commented Jul 30, 2021 • edited

Summary

Benchmark

Other Information

p8 Aug 25, 2021

Choose a reason for hiding this comment

jcmfernandes Aug 26, 2021

Choose a reason for hiding this comment

p8 Aug 30, 2021

Choose a reason for hiding this comment

jcmfernandes Aug 30, 2021

Choose a reason for hiding this comment

p8 commented Aug 31, 2021

jcmfernandes commented Aug 31, 2021 • edited

jcmfernandes Sep 1, 2021

Choose a reason for hiding this comment

jcmfernandes commented Sep 1, 2021

rails-bot bot commented Dec 1, 2021

jcmfernandes commented Dec 6, 2021

p8 commented Dec 9, 2021

jcmfernandes commented Dec 10, 2021

jcmfernandes commented Jul 30, 2021 •

edited

jcmfernandes commented Aug 31, 2021 •

edited