fzaninotto / screenshot-as-a-service

Website screenshot service powered by node.js and phantomjs
1.1k stars 243 forks source link

Better support for HTTPS websites #66

Open slorber opened 9 years ago

slorber commented 9 years ago

See https://github.com/fzaninotto/screenshot-as-a-service/issues/54

The screenshot service does not work well with most HTTPS urls. Here is a cause: http://stackoverflow.com/questions/12021578/phantomjs-failing-to-open-https-site

Some exemples:

OK: http://localhost:3000/?url=http://en.wikipedia.org/wiki/User_agent KO: http://localhost:3000/?url=https://en.wikipedia.org/wiki/User_agent

OK: http://localhost:3000/?url=http://leverich.github.io/swiftislikescala/ KO: http://localhost:3000/?url=https://leverich.github.io/swiftislikescala/

slorber commented 9 years ago

Note the pull request solves your TODO Allow to configure phantomjs options through YAML config

mikej165 commented 9 years ago

I generate about 20,000 screenshots a day and was having a problem with some of the sites that were using https-hosted images. Applying the --ssl-protocol=any option completely resolved it.

PS-HaejinChoi commented 9 years ago

mikej165,

Per your comment above, I created the file called development.yaml under the config folder, and wrote as follow:

rasterizer: command: 'phantomjs ----ssl-protocol=any'

But it fails to run with the following error message: [uncaughtException] { [Error: spawn ENOENT] code: 'ENOENT', errno: 'ENOENT', syscall: 'spawn' }

It would be greatly appreciated if you could share where and how you've added the Phantom options above.

mikej165 commented 9 years ago

Have you tried using the full path to the binary. i.e. /usr/bin/phantomjs?

slorber commented 9 years ago

@mspoodle the current code does not permit to pass options imho, this is why I made a fork/PR

With my fork, the option is automatically included by default and is not part of the command attribute but a new option attribute. See https://github.com/slorber/screenshot-as-a-service/blob/master/config/default.yaml

See also the related diff: https://github.com/slorber/screenshot-as-a-service/commit/eb5aa5460601129b2e38a5cbca0aae2a47501d5b

When passing options to phantomJS, I discovered that these options must be passed at the beginning. I mean phantomjs options xxx.js works, but not phantomjs xxx.js options (as far as I remember).

The original code before my PR is

spawn(this.config.command, ['scripts/rasterizer.js', this.config.path, this.config.port, this.config.viewport]);

I have also tried to use the command: 'phantomjs ----ssl-protocol=any' but it did not work,

It seems you can't call: spawn("phantomjs option1 option2", ['scripts/rasterizer.js', ...])

But instead you can call: spawn("phantomjs", ['option1","option2","scripts/rasterizer.js', ...])

I'm a node beginner and never used spawn before but this is what I noticed, and this is what my fork fixes

Maybe @fzaninotto will merge it soon

PS-HaejinChoi commented 9 years ago

Thank you @slorber and @mikej165 for the suggestions.

I will try them and see how that works.

PS-HaejinChoi commented 9 years ago

@slorber Your solution worked like a charm. Thank you so much.