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

Funnies ain't so funny. #304

Closed
IanTrudel opened this issue Jan 27, 2017 · 27 comments
Closed

Funnies ain't so funny. #304

IanTrudel opened this issue Jan 27, 2017 · 27 comments
Assignees
Labels
Milestone

Comments

@IanTrudel
Copy link
Collaborator

The sample code Funnies in both Shoes3 and HacketyHack is not working. The download stalling on (possibly) redirections. It loads query on google but certainly not XKCD. Directly invoking HTTPS URL of XKCD will not help. Even famously standard w3c will not work.

Redirect checker reports:

http://xkcd.com/rss.xml
301 Moved Permanently
https://xkcd.com/rss.xml
200 OK
@ccoupe
Copy link

ccoupe commented Jan 27, 2017

According to XKCD.com About page you're supposed to use json. I subscribe to the atom feed and it's OK but perhaps he dropped rss.xml . Same question about w3c does it have and rss.xml? No visual indication that it does.

I would not be surprised it there are https problems in Shoes. I suspect packaging has one.

@ccoupe
Copy link

ccoupe commented Jan 27, 2017

wget https://xkcd.com/rss.xml does produce something reasonable. I get an hpricot error so thats a bug to fix in expert-funnies.rb

@IanTrudel
Copy link
Collaborator Author

Hpricot? Both are using Nogokiri. I also tested download on its own, hence the mention about other non-rss URLs.

@ccoupe
Copy link

ccoupe commented Jan 27, 2017

I didn't have hpricot or nokogiri installed in that ruby (Loose Shoes). Download is actually done in Ruby, a change I made long long ago. lib/shoes/download.rb if you want see what is doing. I also remember a recent commit to Shoes 4 and I made a note to see if Shoes 3.x needs it.

@ccoupe
Copy link

ccoupe commented Jan 27, 2017

An Shoes app that knew how to get the daily comics for various sites (lots of web server issues there) would be good project for someone.

@dredknight
Copy link
Contributor

I can try this one.

@dredknight dredknight assigned dredknight and unassigned dredknight Jan 28, 2017
@IanTrudel
Copy link
Collaborator Author

Good luck @dredknight !

@ccoupe ccoupe added this to the 3.3.3 milestone Jan 28, 2017
@ccoupe ccoupe added the Normal label Jan 28, 2017
@ccoupe
Copy link

ccoupe commented Jan 28, 2017

@dredknight - did you volunteer to fix the bug or write a more general comics reader?

@ccoupe ccoupe self-assigned this Jan 29, 2017
ccoupe pushed a commit that referenced this issue Jan 29, 2017
* Copy https://github.com/obfusk/open_uri_w_redirect_to_https
  copy to lib/shoes/open-uri-patch
* modify lib/shoes/download.rb to use the patch
@ccoupe
Copy link

ccoupe commented Jan 29, 2017

Fixed the http->https redirect see commit . Works for me and samples/expert-funnies.rb. If you don't want to wait for 3.3.3 beta (I wouldn't), you can fix your Shoes by replacing one file from the github repo lib/shoes/download.rb and add one file: lib/shoes/open-uri-patch.rb

@ccoupe
Copy link

ccoupe commented Jan 29, 2017

The Shoes4 code appears to handle the problem and might be worth backporting into Shoes 3.3.3

@dredknight
Copy link
Contributor

@ccoupe currently writing the shoes-exe GUI. I will take this one after that. Currently I am not sure of approach because adding and using external libs is not my strongest shoes skill but I believe it will be good thing to learn.
I will update you when I have some progress. Thanks for the proposals!

@IanTrudel
Copy link
Collaborator Author

For reference only:

Shoes 4 approach does work in Shoes 3 with some changes. It would need download.rb with some minor modifications and http_request.rb with an additional require "net/https" and change @started_proc&.call(response) to @started_proc.call(response) unless @started_proc.nil?.

Your approach @ccoupe is much lighter. Some changes inspired by Shoes 4 might be in order but it's yet unclear whether open_uri with redirection or http_request is better.

@dredknight don't be shy to share with us, we sure can give you feedback. Your contribution is more than welcomed! :)

@ccoupe
Copy link

ccoupe commented Jan 30, 2017

I suspect the Shoe4 code is better but the easy way works, so far.

@IanTrudel
Copy link
Collaborator Author

I suspect the Shoe4 code is better but the easy way works, so far.

Shoes 4 is clean and considerably easier to understand. Sometimes a little over-engineered and the final rendering isn't as good as Shoes 3. For example, the Shoes 4 manual is not rendered properly and looks rather retro.

We sure can stick with open_uri with redirection for now. My previous comment is meant to be a reference in case something comes up, then it will be possible to dig into issue tracker for alternative solution.

@IanTrudel
Copy link
Collaborator Author

The code committed on the repository is not working here. It takes a while and then two windows open up.

image

@ccoupe
Copy link

ccoupe commented Jan 30, 2017

Did you run that with the old Hacketyhack shoes installed?

@IanTrudel
Copy link
Collaborator Author

Did you run that with the old Hacketyhack shoes installed?

Sorry? I made a portable installation of HacketyHack. Shoes 3.3.2 is by default.

> cshoes -v
Shoes walkabout 3.3.2 r2741 i386-mingw32 2.2.4
``

@ccoupe
Copy link

ccoupe commented Jan 30, 2017

I just package up a beta exe walkabout.mvmanila.com/public/shoes/ for 3.3.3 and it does not have the strangeness you show. However it doesn't fix expert-funnies.rb.

@ccoupe
Copy link

ccoupe commented Feb 1, 2017

There's more going on here than I first thought. It's easy to change the xkcd feed to use the proper https: url , and without the open-uri-patch it still hangs on Windows. Digging deeper it seems that nothing that uses a https works. Also in image.rb we don't have the patch but it's hanging getting the rss.xml, not the image.

Updated to include: nothing that uses a https works.

ccoupe pushed a commit that referenced this issue Feb 1, 2017
* did someone try to invoke a Shoes alert method in a rescue? Dumb Cecil.
* create HttpResponce.rb since it's used in both image.rb and download.rb
  Helps. Doesn't fix much.
* Does not fix window ssl/https problems.
@ccoupe
Copy link

ccoupe commented Feb 1, 2017

Now we have something to dig into. From the shoes.log on windows (cross compiled)

Error in <unknown> line 0 | 2017-02-01 00:13:12 -0700
Image download failed for https://s3.amazonaws.com/static.garfield.com/comics/garfield/2017/2017-01-30.gif because: SSL_connect returned=1 errno=0 state=SSLv3 read server certificate B: certificate verify failed
C:/Program Files (x86)/Shoes/lib/shoes/image.rb:39:in `rescue in image_download_sync'
C:/Program Files (x86)/Shoes/lib/shoes/image.rb:26:in `image_download_sync'
foo.rb:4:in `image'
foo.rb:4:in `block in <main>'
eval:1:in `instance_eval'
eval:1:in `block in <main>'
-e:1:in `call'

Certs!

@IanTrudel
Copy link
Collaborator Author

It's possible to disable certificate verification.

output = open(request_uri, {ssl_verify_mode: OpenSSL::SSL::VERIFY_NONE})

@ccoupe
Copy link

ccoupe commented Feb 2, 2017

That does appear to help with some things. Be aware that image caching can fool us!

ccoupe pushed a commit that referenced this issue Feb 2, 2017
@ccoupe
Copy link

ccoupe commented Feb 2, 2017

Ok. I think I have something that works for Linux and Windows. Two test scripts, samples/expert-funnies.rb (xkcd with https redirect) and this test

Shoes.app {
  stack do
    image "https://shoes.mvmanila.com/public/images/2017-01-30.gif"
    download "https://xkcd.com/rss.xml" do |dl|
     puts "got xkcd rss.xml"
    end
    download "https://shoes.mvmanila.com/public/images/dino.jpg", save: "dino.jpg" do |f|
      puts "got dino image"
    end
  end
  #image "https://s3.amazonaws.com/static.garfield.com/comics/garfield/2017/2017-01-30.gif"
}

I think this beta will work for windows. It's 22MB because it has debugging symbols - might be helpful in a crash. It has some puts status messages from inside download.rb. Don't forget to clear the image cache with cobbler when testing the image method.

The beta does not verify the ssl certificates but thats OK for now. I suspect a deeper bug between ruby, a possibility out-dated openssl version, and the two Windows ssl dlls included with Shoes.

ccoupe pushed a commit that referenced this issue Feb 2, 2017
* might work well enough for now.
@dredknight
Copy link
Contributor

Heya. I am back now. Anything I can help with here?

@IanTrudel
Copy link
Collaborator Author

Welcome back @dredknight !

You have plenty of choices. Let's see...

@dredknight
Copy link
Contributor

Good :)

@ccoupe
Copy link

ccoupe commented May 17, 2017

Closing. Does not fix that download can be very slow. - different issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants