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.

Sign up for GitHub

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

Jump to bottom

Empty markup returned when testing HTTPS pages. #38

Closed
Chapabu opened this issue Feb 22, 2015 · 6 comments
Closed

Empty markup returned when testing HTTPS pages. #38

Chapabu opened this issue Feb 22, 2015 · 6 comments

Comments

@Chapabu
Copy link
Contributor

Chapabu commented Feb 22, 2015

If your configuration is set up to hit a page over HTTPS, then you get the following markup:

CasperJS: CREATING NEW REFERENCE FILES
CasperJS: LOG> CASPER IS RUNNING
CasperJS: LOG> PHANTOM IS RUNNING
CasperJS: 
==================
Current location is https://www.twitter.com
==================

CasperJS: 

<html><head></head><body></body></html>
CasperJS: 
======================
echoFiles has completed 
=======================


Echo files completed.

This isn't an issue with BackstopJS exactly, but how PhantomJS handles pages running under HTTPS. Following advice on StackOverflow here, it can be fixed by passing --ssl-protocol=tlsv1 in with the tests array prior to spawning the CasperJS process (e.g. echo.js).

Obviously this doesn't make sense to add this in globally, but perhaps we could add an option for which SSL protocol to use (if any) in the backstop.json config file? I guess it'd be most suitable in the scenarios section?

I.e.

...
 "scenarios": [
    {
      "label": "Twitter",
      "url": "https://www.twitter.com",
      "ssl-protocol": "tlsv1",
      "hideSelectors": []
    }
 ]

I'm happy to work on a pull request for this, but does the suggested approach make sense?
@garris
Copy link
Owner

garris commented Feb 22, 2015

Hi @Chapabu thank you for filing this and researching the issue! This suggestion makes sense. Question tho -- is there any case where we'd need to set ssl-protocol if the url didn't contian "https://"? My guess is no -- and if that guess is correct then my instinct is to handle this internally with a switch on if (/^https:\/\//.test(url)) {...} just prior to the spawn block. Maybe this is a nice convienience for nearly every (if not all) use cases? What do you think?

@Chapabu
Copy link
Contributor Author

Chapabu commented Feb 22, 2015

Good call. My only concern with that would be that PhantomJS defaults to SSLv3 (which POODLE kinda borked), so we'd need to allow people to pass in the protocol that should be used. PhantomJS can take one of four arguments for ssl-protocol (namely sslv3, sslv2, tlsv1, any - source), so if we were to guess then there's a 75% chance we'd be wrong (I've not had success with any when using CasperJS for tests, but it my be OK in this case). So I think either way we'd need to add a configuration option somewhere.

@Chapabu
Copy link
Contributor Author

Chapabu commented Feb 22, 2015

Actually, I've just checked against Twitter and Facebook, and it appears that passing any works fine. So yeah, perhaps checking for https:// in the url to be tested and just pushing --ssl-protocol=any onto the Casper arguments would work fine. That kinda makes things a LOT easier.

@Chapabu
Copy link
Contributor Author

Chapabu commented Feb 22, 2015

One last thing (sorry, I keep thinking just after posting my comments - this'll be the last one, I promise). If a site doesn't use HTTPS, then it doesn't even matter what option we pass to Casper/Phantom. We should just be able to always pass ssl-protocol=any, and it should work across the board.

@garris
Copy link
Owner

garris commented Feb 22, 2015

That sounds great. Go ahead and add that parameter if you get a chance. I'm happy to pull that in. Thanks.

@garris garris closed this as completed Feb 22, 2015
@garris garris reopened this Feb 22, 2015
@Chapabu Chapabu mentioned this issue Feb 23, 2015
Merged
@garris
Copy link
Owner

garris commented Mar 3, 2015

This was resolved. Issue only effects older phantom installs.

@garris garris closed this as completed Mar 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@garris @Chapabu