Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Image upload failing #9438

Closed
cesswairimu opened this issue Apr 4, 2021 · 35 comments · Fixed by #9506
Closed

Image upload failing #9438

cesswairimu opened this issue Apr 4, 2021 · 35 comments · Fixed by #9506
Labels
bug the issue is regarding one of our programs which faces problems when a certain task is executed help wanted requires help by anyone willing to contribute high-priority

Comments

@cesswairimu
Copy link
Collaborator

cesswairimu commented Apr 4, 2021

Image upload fails with this error in the body section "Section 4" of /post route
Screenshot from 2021-04-04 03-28-07 this is on the https://publiclab.org/, was able to replicate it on https://unstable.publiclab.org and locally

Note: Image upload is working fine on "Section 2" of /post
Template: https://github.com/publiclab/plots2/blob/main/app/views/editor/rich.html.erb

@cesswairimu cesswairimu added bug the issue is regarding one of our programs which faces problems when a certain task is executed high-priority help wanted requires help by anyone willing to contribute labels Apr 4, 2021
@waridrox
Copy link
Member

waridrox commented Apr 4, 2021

Also referencing #9442 for a minor problem.

@cesswairimu
Copy link
Collaborator Author

thanks @waridrox 👍

@waridrox
Copy link
Member

waridrox commented Apr 4, 2021

Attempting to debug the problem further, the error that I got when uploading an image from local file storage was
Paperclip::Errors::CommandNotFoundError: Could not run the 'identify' command. Please install ImageMagick.
under local dev environment. This I believe is a new dependency for paperclip gem that deals with file uploads.

Screenshot 2021-04-04 at 8 13 15 PM

After installing imagemagick with the command sudo apt-get install imagemagick or brew install imagemagick, I added these 2 lines of code in development.rb file.

Paperclip.options[:image_magick_path] = "/opt/ImageMagick/bin"
Paperclip.options[:command_path] = "/opt/ImageMagick/bin"

After that I simply restarted the server and the file upload was functional again with file types like jpeg, png and gif. Here's a glimpse of the same -

test.mp4

From this thread on stackoverflow, we need to add these 2 lines in production.rb as well to reflect the changes in the production app.

Also due to optional parameter to install image-magick in https://github.com/publiclab/plots2/blob/main/doc/PREREQUISITES.md#image-libraries-optional, this behaviour was observed in the first place.

@cesswairimu
Copy link
Collaborator Author

Thanks @waridrox, I pulled your changes and I am still getting the error....could someone else please try on their end. Thanks

@waridrox
Copy link
Member

waridrox commented Apr 4, 2021

Oh 😅 really sorry about that, pls revert the pull then. Does it work locally though ? Maybe trying sudo apt-get update first and then sudo apt-get install imagemagick can help...

@jywarren
Copy link
Member

jywarren commented Apr 4, 2021

Hi all, locally it makes some sense that it's an imagemagick but (or absence of imagemagick) but on stable it should be installed. I wonder if it's a distinct bug on stable? We should cross check on Sentry.io. Cess do you have access?

Thanks for finding and documenting this!!

@jywarren
Copy link
Member

jywarren commented Apr 4, 2021

I'm not seeing sentry errors on image upload except in relation to profile pics (which is often the case due to spam and attacks).

Screenshot_20210404-174638
Screenshot_20210404-174706

What is the path for the error? It's a 500 error?

@jywarren
Copy link
Member

jywarren commented Apr 4, 2021

Sorry I mean /recent/ errors

@cesswairimu
Copy link
Collaborator Author

cesswairimu commented Apr 4, 2021

@waridrox it didn't work for me locally maybe I have the wrong version..@jywarren it was more of a 302 error on my end

@cesswairimu
Copy link
Collaborator Author

Screenshot from 2021-04-05 00-54-02

@jywarren
Copy link
Member

Aha, 302 error indeed... 🕵️

I'll look at why that may be the case. Thank you!

@jywarren
Copy link
Member

I'm going to try running it in GitPod to see if i can reproduce the 302...

@jywarren
Copy link
Member

OK, got a log from GitPod!

Started POST "/images" for 10.4.6.182 at 2021-04-13 18:36:40 +0000
Cannot render console from 10.4.0.249! Allowed networks: 127.0.0.1, ::1, 127.0.0.0/127.255.255.255
Processing by ImagesController#create as JSON
  Parameters: {"nid"=>"null", "image"=>{"photo"=>#<ActionDispatch::Http::UploadedFile:0x00007f444ce23c30 @tempfile=#<Tempfile:/tmp/RackMultipart20210413-4879-sla487.jpg>, @original_filename="11aa-love-ryan2-720.jpg", @content_type="image/jpeg", @headers="Content-Disposition: form-data; name=\"image[photo]\"; filename=\"11aa-love-ryan2-720.jpg\"\r\nContent-Type: image/jpeg\r\n">}}
Can't verify CSRF token authenticity.
Redirected to https://3000-aquamarine-bass-qsunlo8p.ws-us03.gitpod.io/login?return_to=/images
Filter chain halted as :require_user rendered or redirected
Completed 302 Found in 2ms (ActiveRecord: 0.0ms)


Started GET "/login?return_to=/images" for 10.4.6.182 at 2021-04-13 18:36:40 +0000
Cannot render console from 10.4.0.249! Allowed networks: 127.0.0.1, ::1, 127.0.0.0/127.255.255.255
Processing by UserSessionsController#new as JSON
  Parameters: {"return_to"=>"/images"}
  User Load (0.5ms)  SELECT  `rusers`.* FROM `rusers` WHERE `rusers`.`id` = 1 LIMIT 1
   (0.2ms)  BEGIN
  User Update (0.5ms)  UPDATE `rusers` SET `last_request_at` = '2021-04-13 18:36:40', `updated_at` = '2021-04-13 18:36:40' WHERE `rusers`.`id` = 1
   (28.6ms)  COMMIT
Redirected to https://localhost/home?return_to=%2Flogin
Filter chain halted as :require_no_user rendered or redirected
Completed 302 Found in 35ms (ActiveRecord: 29.7ms)

OK, so why are we being redirected to log in... looking...

@jywarren
Copy link
Member

Looks like CSRF token is not working... let's see...

editor = new PL.Editor({
data: {
token: $('meta[name="csrf-token"]').attr('content')
},

we pass it in here.

@jywarren
Copy link
Member

$('meta[name="csrf-token"]').attr('content') correctly fetches the token, so that's not it...

@jywarren
Copy link
Member

Noting that in GitPod, the main image uploader works. It uses the same controller, so the issue seem likely to be in the Editor's rich text module code itself:


Started POST "/images" for 10.4.0.248 at 2021-04-13 18:47:45 +0000
Cannot render console from 10.4.6.181! Allowed networks: 127.0.0.1, ::1, 127.0.0.0/127.255.255.255
Processing by ImagesController#create as JSON
  Parameters: {"authenticity_token"=>"5Z5plBlzFXbu2qCKPTDZYa54DWaNn556hs9FzyvH2U8NDMabePLVh1RlBuhkRR+Ej+kgI/hLIAm9LcdrEZv4lg==", "uid"=>"1", "image"=>{"photo"=>#<ActionDispatch::Http::UploadedFile:0x000055644bef1898 @tempfile=#<Tempfile:/tmp/RackMultipart20210413-5664-bsht8f.jpg>, @original_filename="11aa-love-ryan2-720.jpg", @content_type="image/jpeg", @headers="Content-Disposition: form-data; name=\"image[photo]\"; filename=\"11aa-love-ryan2-720.jpg\"\r\nContent-Type: image/jpeg\r\n">}}

@jywarren
Copy link
Member

And actually the main image uploader also uses the token in its request:

https://github.com/publiclab/PublicLab.Editor/blob/59e5bf6fd1ac2b6f9108e522baa7cdab36e0bb64/src/modules/PublicLab.MainImageModule.js#L94

It correctly fetches it with:

editor.options.mainImageModule.token
"5Z5plBlzFXbu2qCKPTDZYa54DWaNn556hs9FzyvH2U8NDMabePLVh1RlBuhkRR+Ej+kgI/hLIAm9LcdrEZv4lg=="

@jywarren
Copy link
Member

Confirmed. The main image upload has this set of params in the form request:

authenticity_token: 5Z5plBlzFXbu2qCKPTDZYa54DWaNn556hs9FzyvH2U8NDMabePLVh1RlBuhkRR+Ej+kgI/hLIAm9LcdrEZv4lg==
uid: 1
image[photo]: (binary)

Whereas the inline image upload has only these:

nid: null
image[photo]: (binary)

checking recent changes to that section of code, as well as the module options itself to ensure it's getting properly stored/passed in from the constructor.

@jywarren
Copy link
Member

jywarren commented Apr 13, 2021

Wow, deep bug. Chasing back through jywarren/woofmark#2 to woofmark library: bevacqua/woofmark#44

@jywarren
Copy link
Member

It's present in our branch of woofmark:

https://github.com/jywarren/woofmark/blob/plots2/src/prompts/prompt.js#L133

@jywarren
Copy link
Member

I believe this PR from 28 days ago may be related. the last change to the branch of woofmark we are using happened in September 2020.

https://github.com/jywarren/woofmark/pull/76/files

@jywarren
Copy link
Member

I'm not seeing anything in the code in woofmark -- 2 routes now:

  1. tracing the upload.xhrOptions param through Woofmark and confirming it's correctly got it
  2. seeing if the xhr npm module has changed

@jywarren
Copy link
Member

https://www.npmjs.com/package/xhr published 2.6 5 months ago but has been at 2.2.1 for many many months in our projects. confirmed in package-lock.json

@jywarren
Copy link
Member

jywarren commented Apr 13, 2021

OK, so the upload.xhrOptions param beforeSend does make it into woofmark, as tested in the JS console.

Does it actually generate the extra header?

xhrOptions: { 
        beforeSend: function(xhr) { xhr.setRequestHeader('X-CSRF-Token', $('meta[name="csrf-token"]').attr('content')) }
      },

@jywarren
Copy link
Member

Gosh i don't know when this stopped working but I think we could potentially just add another formData param and do it that way??? But not sure if it'll get passed up with the others, or if the xhr lib will ignore it?

Noting this would have happened either November 5 2020 in https://github.com/publiclab/PublicLab.Editor/releases/tag/v3.0 or 28 days ago in #9323

@jywarren
Copy link
Member

According to this line, i believe formData should get passed in normally:

https://github.com/bevacqua/woofmark/pull/44/files#diff-b70752c6e4fb751c6aa381f57afac66c25cc2d401b141b03e1751d81e60efcd2R216

@jywarren
Copy link
Member

will try in GitPod:

      formData: {nid: null, authenticity_token: _module.options.token},

... THAT DID IT:

image

Will fix in publiclab/PublicLab.Editor#712, release 3.0.3, and also add the extra formData token in #9504.

Whew!

jywarren added a commit to publiclab/PublicLab.Editor that referenced this issue Apr 13, 2021
…, v3.0.3 (#712)

* move csrf token into formData for bureaucracy

re publiclab/plots2#9438

* bumped version and built dist
jywarren added a commit that referenced this issue Apr 13, 2021
)

* move csrf token into formdata for PublicLab.Editor richTextModule

ref #9438

* remove ", :fr" from application.rb re #9481
@jywarren
Copy link
Member

Done and confirmed in stable!

image

@ebarry
Copy link
Member

ebarry commented Apr 19, 2021

@publiclab-mimi reported today that

On the research note editor I was unable to upload a JPEG, PDF, or PNG.
On Google Chrome and Safari both drag-and-drop as well as browse options

@ebarry ebarry reopened this Apr 19, 2021
@waridrox
Copy link
Member

waridrox commented Apr 19, 2021

@ebarry, I think the current version still doesn't have the above changes due to buffer time required to push the changes to the live version. That's because image / file upload works on the stable version and the legacy version.

Screenshot 2021-04-20 at 2 54 03 AM

@jywarren
Copy link
Member

The new code went live last night! Taking a look, i'm seeing a new error - a 500 error:

starting with this one, which is the same 302 as Cess found:

image

Then redirected to this new 500 error:

image

which is actually upon a successful login, i believe, but it's weird because i'm already logged in?

Sentry logged it here, i think, but i'm not sure if the error happens before or after the login attempt.

https://sentry.io/share/issue/13e10e65210c4ceb9860ec687482d9b7/

ActionView::MissingTemplate
Missing template home/home with {:locale=>[:en], :formats=>[:json], :variants=>[], :handlers=>[:raw, :erb, :html, :builder, :ruby, :coffee, :jbuilder]}. Searched in:
  * "/app/app/views"
  * "/usr/local/bundle/gems/grape-swagger-ui-2.2.8/app/views"
  * "/usr/local/bundle/gems/grape-swagger-rails-0.3.1/app/views"
  * ```

@jywarren
Copy link
Member

Huh, having trouble tracing this so i'll try reproducing in GitPod again???

@jywarren
Copy link
Member

OK, @icarito and I were able to resolve a yarn update issue to pull in the real 3.0.3 editor code, confirmed it, then fixed one more nginx config issue related to SSL and it is now working on the live site. Thanks, everyone!

image

@cesswairimu
Copy link
Collaborator Author

🎉 🎉 🎉

@ebarry
Copy link
Member

ebarry commented Apr 23, 2021

awesome!!! thank you

reginaalyssa pushed a commit to reginaalyssa/plots2 that referenced this issue Oct 16, 2021
…bliclab#9504)

* move csrf token into formdata for PublicLab.Editor richTextModule

ref publiclab#9438

* remove ", :fr" from application.rb re publiclab#9481
billymoroney1 pushed a commit to billymoroney1/plots2 that referenced this issue Dec 28, 2021
…bliclab#9504)

* move csrf token into formdata for PublicLab.Editor richTextModule

ref publiclab#9438

* remove ", :fr" from application.rb re publiclab#9481
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug the issue is regarding one of our programs which faces problems when a certain task is executed help wanted requires help by anyone willing to contribute high-priority
Projects
None yet
4 participants