[Bug]: Fix for bytesize on Array doesn't work when aws-core is installed from debian

SleeplessByte commented 1 year ago

Report

The issue in #492 was resolved using the following conditional:

Gem::Version.new(Aws::CORE_GEM_VERSION) >= Gem::Version.new("3.104.0")

Unfortunately, the Debian version (which is frozen for 2 years) has a faulty implementation. Whilst this is an upstream issue, this means that the fix doesn't work for those relying on debian to provide the gems.

Unfortunately I don't think shrine.rb can fix this except for:

inversing the check
someone setting the ENV variable (which is sigh as it's a super generic name)
adding some option to shrine to manually trigger this fix (preferred?)

I understand this is an upstream issue and I am mostly opening this for visibility, but since bullseye was released only a month ago, this issue is going to persist for 2 years.

Expected Behavior

The fix works

Actual Behavior

Debian's installation of aws-sdk-core will have Aws::CORE_GEM_VERSION returns '' or worse, whatever is in ENV['VERSION'] which makes the fix not work.

The fix does not work.

Steps to Reproduce the Problem

Source gems locally like so:

gem 'shrine', path: File.join(__dir__, 'vendor', 'shrine-3.4.0')
gem 'aws-sdk-s3', '~> 1.48'

now install the package

apt-get install ruby-aws-sdk-s3

and force bundler to use system packages:

bundle install --local

Ruby Version

ruby 2.7.4p191

Shrine Version

3.4.0

Anything else?

There are two workarounds.

Set the VERSION in ENV. I recommend against this, but it works.
make sure require: false for the gem, and then in shrine.rb:

Aws.send(:remove_const, 'CORE_GEM_VERSION')
module Aws
  CORE_GEM_VERSION = '3.104.3'
end

require 'shrine'

janko commented 1 year ago

It might be my lack of knowledge on Linux distributions, but why are you installing a gem through apt-get and not through RubyGems.org?

SleeplessByte commented 1 year ago

No worries, it's not super common.

TL;DR: security

Linux distributions in general are capable of downloading gems from Rubygems, but just like how many enterprises will not download JavaScript packages from NPM but rather from their own packages registry (usually because there is a SecOps team involved or to protect against yanking / pollution / phising / etc.), one can use debian to install packages as system gems.

On Debian systems, especially in production, you normally have one ruby version and one version per package. This creates friction when developing applications (less packages to pick from, sometimes being stuck with old version that don't have the feature you want), but you gain a lot.

Installing through apt-get means you'll always get compatible packages and security backports (even through unattended-upgrades) which is great for production systems. On many of our production servers, we cannot download from rubygems at all.
Installing through apt-get means that on the same Debian version (current stable is called bullseye), you'll have the exact same upgrade path when you move to the next version. This means that we need to figure out upgrades for each gem only once, and can then codemod 100% of our applications and libraries, greatly reducing the cost to upgrade to the next version.

There are plenty of people who use Debian in conjuction with:

self built ruby
multiple ruby version
rubygems (e.g. via bundler)

There are also plenty of people however that strictly adhere to the packages from debian.

The way we make shrine work is by vendoring those. Because you always keep your dependencies to a minimum (thank you!!) this is extremely easy for us to do, and allows us to use non-debian packages where an author has ensured it's not a dependency hell. Unfortunately, aws-sdk-* instead that easy to vendor. Vendoring does increase the time to upgrade and adds a requirement to periodically do manual security updates. The more gems in a dependency tree when vendoring, the more painfull that is.

jrochkind commented 1 year ago

Hm.

Shrine has to keep working well for the majority of people who aren't using debian of course. I agree that the shrine gem automatically setting an ENV['version'] is a bad idea and not possible.

And Shrine shouldn't change it's logic in the default case generally, it should not break things for the 99% of users, or require them to set some configuration if and only if they are using aws-core gem >= 3.104.0, that would not make any senes.

If there was some configuration to be used only by debian-release-aws-core users that they had to opt into... I'm not sure that would be that much preferable to your number 2 workaround, just manually overriding the version setting for the debian-release-aws-core in a local app initializer file. (I think you mean ./config/initializers/shrine.rb, or a similar initializer file in local app?). If you have to know that you have to do something special as a debian-release-gem user to use shrine with AWS, and then know what that thing is -- that thing might as well be that initializer workaround.

I think your workaround number 2 is probably the appropriate thing for debian-release-aws-core users to do -- it's working around either a bug in debian-release-aws-core, or an idiosyncracy where debian release gem forks intentionally do not have a version? Either way, it seems a strangeness in the debian release that is likely to break a lot of things -- I suspect that people using this debian forked releases of gem system often run into problems like this?

The only other thing I can think of is if shrine can somehow do a "feature check" of the aws gem -- see what methods with what arguments are actually available in actual loaded aws gem, instead of checking version number as a proxy for "what features are available behaving how." I'm not sure if that's feasible in this case.

SleeplessByte commented 1 year ago

Yeah. I think the workaround is fine, and indeed I meant the initializer. Furthermore I agree with everything else you've said.

If feature checking is possible that would be preferable, but at least now there is something people can search for and find if they run i to the same issue.

SleeplessByte commented 1 year ago

A bug report was filed, accepted, and completed. This means that eventually this problem will be solved.

I think that the workaround version 2 is the way to go for people in the meanwhile.

shrinerb / shrine