Bug - Images: SHA is changed, if I change uploaded image

[prokopsimek]

I uploaded image called "prokop.jpg" and I added it in my page content via WYM. But now I want to change this image without to have to changing content of page, I want only edit image and to change the source file... But the SHA is changed, if I change the source file, so there is 404 image on my page...

How to resolve? Any ideas?

[bricesanchez]

It's a missing feature.
It could be great to find a solution :)

[prokopsimek]

What about to substitute image url with something like {{IMG:5}} in HTML source code of a page and this would by substituted for an image url? (this example returns image with id == 5)

[anitagraham]

How did you do this?
I just tried to duplicate it, and ended up with the original image still on my page.
To get the second image on the page I had to insert it.

Only one Bullwinkle in the images list, but two on the page:

[prokopsimek]

It's only idea, I don't know how to do it.

[anitagraham]

If you are going to start offering templating in the page you will end up having to offer a lot more than that!
(Useful things always grow)

Calling it a bug is a little harsh if you can't make it happen!

[prokopsimek]

@anitagraham You don't understand me... Try this steps:

1. upload img (your first img)
2. add img into page part content (your second img)
3. edit your uploaded img
4. change source file of this img
5. SHA is changed, so your img in page part content returns err404

[anitagraham]

@prokopsimek off to singing practise now. Will try when I get back.

Can you label the later steps so that I can tell which one is image 1 and which is image 2?
Thanks.

[bsodmike]

@parndt @anitagraham I did some code spelunking through the latest commit on master 29ab036 and find that a rather old version of dragonfly's still the dependency of Refinery, v1 from 2013-11-24. This version is so old it still has protect_from_dos_attacks as the option for enabling its SHA-based protection, rather than the newer verify_urls (ref).

Of course, there's chatter of SHA related issues with version 1.0.7 (markevans/dragonfly#387) but the latest version is 1.0.10. Any chance we could use the latest version?

A recent project of mine was based off the master branch, and for the time being I've had to kill the DOS protection in Dragonfly from causing a mess in production (the usual 'invalid SHA' provided message, even though a secret has been set).

These are from master 29ab036

images/refinerycms-images.gemspec
22:  s.add_dependency 'dragonfly',               '~> 1.0.0'

resources/refinerycms-resources.gemspec
23:  s.add_dependency 'dragonfly',               '~> 1.0.0'

[parndt]

That dependency specification should allow 1.0.10 just fine?

Run bundle update dragonfly.

[bsodmike]

Nutters, I must have a stale Gemfile.lock! Thanks @parndt

[brooks]

@bsodmike , how were you able to remove the DOS protection? We're having the same problem in a production app. Is there a configuration setting in refinery core config or do I need to fork refinery and patch the fork?

[bsodmike]

hey @brooks,

I added an initialiser to override both the Refinery Images and Resources modules - as they both use Dragonfly. The upgrade from 1.0.7 to 1.0.10 Dragonfly didn't help the SHA issue, so I went with this

require 'dragonfly'

Rails.application.config.before_initialize do

  module Refinery
    module Images
      module Dragonfly

        class << self
          def configure!
            # copy the method's code over from your version of Refinery
          end
        end

      end
    end
  end

  module Refinery
    module Resources
      module Dragonfly

        class << self
          def configure!
            # copy the method's code over from your version of Refinery
          end
        end

      end
    end
  end

end

Inside each configure! call, you need to set verify_urls false. I feel this is somewhat better than adding a monkey patched dependency to your codebase.

[brooks]

Thanks @bsodmike. Much appreciated.

[anitagraham]

Refinery could offer verify_urls as a config option for any dragonfly plugin. An initializer would only have to set that to false.

[bsodmike]

@anitagraham +1

[bsodmike]

Pleasure @brooks

[parndt]

OK, so who is keen to write the patch?

[anitagraham]

I thought I had done that! Let me investigate...
Yes, code written, but stopped midst test-writing.

I will submit a PR.

[anitagraham]

PR #3017. However this test fails:

      context "when Dragonfly.verify_urls is false" do
        Refinery::Images.dragonfly_verify_urls = false
        it "returns a url without an SHA parameter" do
          expect(created_image.url).not_to match(/\?sha=[\da-fA-F]{16}\z/)
        end
      end

I think because you can't change a config value mid-stream, so to speak.

The url returned has got an SHA parameter. Testing in an application did work though.

I was investigating how to get this to work about a month ago, but have been too busy to follow up.

[parndt]

When mocking config values, you might want to use RSpec's allow feature:

context "when Dragonfly.verify_urls is false" do
  before { allow(Refinery::Images).to receive(:dragonfly_verify_urls).and_return(false) }
  it "returns a url without an SHA parameter" do
    expect(created_image.url).not_to match(/\?sha=[\da-fA-F]{16}\z/)
  end
end

[parndt]

@anitagraham hopefully my previous comment helps you?

[bsodmike]

@parndt +1 RSpec's stub/mocking also allows use of expect; allow is technically a stub, whereas expect itself is an assertion, and would work just as well. Cheers for the patch @anitagraham.

[bricesanchez]

Is it fixed ? @parndt and @anitagraham

[parndt]

@bricesanchez the commit f444811 was merged which should mean this is fixed.

[parndt]

@bricesanchez though, the commit just gives you a config option, so I'm not sure if that qualifies as fixing the underlying issue.

[anitagraham]

That's what I was going to say.

[simi]

Just to summarize this: It is not bug in Refinery, but Dragonfly feature and it can be bypassed with new config in Refinery configuring Dragonfly to not use SHA in URL. Am I right and can we close this?

/cc @anitagraham @bsodmike @prokopsimek

[bsodmike]

@simi correct. The change in f444811 allows by-passing the underlying issue in Dragonfly, at least until Refinery's dependency is upgraded once it's sorted in Dragonfly itself a patched version is released. Until such time, the config option is your friend. Correction

I've not had time to dig into this further, but I hope to do so soon (markevans/dragonfly#387)

[bricesanchez]

"... at least until Refinery's dependency is upgraded...."

@bsodmike if you run bundle update, you will be able to use the latest version of dragonfly 1.0.10

The proof in my Gemfile.lock :

[...]
    dragonfly (1.0.10)
      addressable (~> 2.3)
      multi_json (~> 1.0)
      rack (>= 1.3.0)
[...]

[bsodmike]

As I recall the issue has yet to be fixed, even in the latest tagged
release of Dragonfly. Issue is not the use of the pessimistic version
constraint in Refinery. Sorry for the confusion!

On Wednesday, 19 August 2015, Brice Sanchez notifications@github.com
wrote:

"... at least until Refinery's dependency is upgraded...."

@bsodmike https://github.com/bsodmike if you run bundle update, you
will be able to use the latest version of dragonfly 1.0.10

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

Mike's on the move!

[simi]

@bsodmike do you mean https://github.com/refinery/refinerycms/blob/master/resources/refinerycms-resources.gemspec#L23 and https://github.com/refinery/refinerycms/blob/master/images/refinerycms-images.gemspec#L22? Why we need to lock it to specific minimal version?

[bsodmike]

Oh right, my mistake. I should have said

"The change in f444811 allows by-passing the underlying issue in Dragonfly, at least until Refinery's dependency is upgraded once it's sorted in Dragonfly itself a patched version is released. Until such time, the config option is your friend."

@bricesanchez good catch mate, thanks for clarifying that!
@simi Apologies for the confusion

[simi]

@bsodmike as I said I think it is not bug, but it is feature of Dragonfly.