Skip to content

UpdateMergeRequestWorker silently fails with TypeError: can't quote String

https://sentry.gitlab.net/gitlab/gitlabcom/issues/4091009/?referrer=gitlab_plugin

TypeError: can't quote String
  lib/gitlab/database/load_balancing/connection_proxy.rb:127:in `public_send'
    connection.public_send(...)
  lib/gitlab/database/load_balancing/connection_proxy.rb:127:in `block in write_using_load_balancer'
    connection.public_send(...)
  lib/gitlab/database/load_balancing/load_balancer.rb:127:in `block in read_write'
    yield connection
  lib/gitlab/database/load_balancing/load_balancer.rb:205:in `retry_with_backoff'
    return yield attempt # Yield the current attempt count
  lib/gitlab/database/load_balancing/load_balancer.rb:116:in `read_write'
    retry_with_backoff(attempts: attempts) do |attempt|
...
(198 additional frame(s) were not displayed)

can't quote String

Explanation of bug

It's rather subtle, but I think it all has to do with force_encoding and how the second iteration returns a BatchLoader instead of a String:

  1. When this commit is loaded, Gitaly retrieves the message and returns it in string with an ASCII-8BIT encoding.
  2. When the Git commit message is longer than 10 * 1024 bytes, @message is a lazy BatchLoader object.
  3. When Commit#message is called, this gets delegated to Gitlab::Git::Commit#message, which calls encode! @message in https://gitlab.com/gitlab-org/gitlab/-/blob/7ffd1ef11ed639bfbc72952981e89df1504f5025/lib/gitlab/git/commit.rb#L331.
  4. This calls force_encode_utf8 in https://gitlab.com/gitlab-org/gitlab/-/blob/7ffd1ef11ed639bfbc72952981e89df1504f5025/lib/gitlab/encoding_helper.rb#L22.
  5. Let's look at force_encode_utf8 in detail:
1    def force_encode_utf8(message)
2      raise ArgumentError unless message.respond_to?(:force_encoding)
3      return message if message.encoding == Encoding::UTF_8 && message.valid_encoding?
4
5      message = message.dup if message.respond_to?(:frozen?) && message.frozen?
6
7      message.force_encoding("UTF-8")
8    end
  1. The first time force_encode_utf8 runs, the message will be a valid encoding in ASCII-8BIT. That means the last line (7) of the method will be called: message.force_encoding("UTF-8"). A String will be returned.
  2. As https://tenderlovemaking.com/2020/01/13/guide-to-string-encoding-in-ruby.html explains, force_encoding modifies the associated encoding data for that String.
  3. The second time this is run, line 3 is reached. This time the encoding is UTF-8, so line 3 returns message. However, message is the BatchLoader object!

The solution: we need to call to_s to avoid exposing BatchLoader.

Edited by Stan Hu