Apply suggestions from code review

Co-authored-by: Julien Neuhart <j.neuhart@thecodingmachine.com>
This commit is contained in:
tomjvdberg
2020-06-08 08:31:57 +02:00
committed by GitHub
parent 1e6926ee2d
commit 4d46b98e7f
6 changed files with 39 additions and 39 deletions

View File

@@ -58,9 +58,11 @@ The hard limit is 100 MB and is defined by Google Chrome itself.
> The default Google Chrome rpcc buffer size may also be overridden per request thanks to the form field `googleChromeRpccBufferSize`. > The default Google Chrome rpcc buffer size may also be overridden per request thanks to the form field `googleChromeRpccBufferSize`.
> See the [rpcc buffer size section](#html.rpcc_buffer_size). > See the [rpcc buffer size section](#html.rpcc_buffer_size).
## Ignore certificate errors ## Google Chrome ignore certificate errors
By default Chrome will not accept certificate errors when using the URL print method. Setting this environment variable to `"1"` will allow insecure connections to be used. In dev environment for example. Be careful with this! Do not use in production. When performing a [URL](#url) conversion, Google Chrome will not accept certificate errors .
You may allow insecure connections by setting `GOOGLE_CHROME_IGNORE_CERTIFICATE_ERRORS` variable to `"1"`. **You should be careful with this feature and only enable it in your development environment.**
## Disable LibreOffice (unoconv) ## Disable LibreOffice (unoconv)

View File

@@ -28,7 +28,7 @@ func main() {
systemLogger.DebugOpf(op, "configuration: %+v", config) systemLogger.DebugOpf(op, "configuration: %+v", config)
if !config.DisableGoogleChrome() { if !config.DisableGoogleChrome() {
// start Google Chrome headless. // start Google Chrome headless.
if err := chrome.Start(systemLogger, config.IgnoreCertificateErrors()); err != nil { if err := chrome.Start(systemLogger, config.GoogleChromeIgnoreCertificateErrors()); err != nil {
systemLogger.FatalOp(op, err) systemLogger.FatalOp(op, err)
} }
} }

View File

@@ -16,11 +16,11 @@ import (
) )
// Start starts Google Chrome headless in background. // Start starts Google Chrome headless in background.
func Start(logger xlog.Logger, urlIgnoreCertificateErrors bool) error { func Start(logger xlog.Logger, ignoreCertificateErrors bool) error {
const op string = "chrome.Start" const op string = "chrome.Start"
logger.DebugOp(op, "starting new Google Chrome headless process on port 9222...") logger.DebugOp(op, "starting new Google Chrome headless process on port 9222...")
resolver := func() error { resolver := func() error {
cmd, err := cmd(logger, urlIgnoreCertificateErrors) cmd, err := cmd(logger, ignoreCertificateErrors)
if err != nil { if err != nil {
return err return err
} }
@@ -32,7 +32,7 @@ func Start(logger xlog.Logger, urlIgnoreCertificateErrors bool) error {
// if the process failed to start correctly, // if the process failed to start correctly,
// we have to restart it. // we have to restart it.
if !isViable(logger) { if !isViable(logger) {
return restart(logger, cmd.Process, urlIgnoreCertificateErrors) return restart(logger, cmd.Process, ignoreCertificateErrors)
} }
return nil return nil
} }
@@ -42,7 +42,7 @@ func Start(logger xlog.Logger, urlIgnoreCertificateErrors bool) error {
return nil return nil
} }
func cmd(logger xlog.Logger, urlIgnoreCertificateErrors bool) (*exec.Cmd, error) { func cmd(logger xlog.Logger, ignoreCertificateErrors bool) (*exec.Cmd, error) {
const op string = "chrome.cmd" const op string = "chrome.cmd"
binary := "google-chrome-stable" binary := "google-chrome-stable"
args := []string{ args := []string{
@@ -67,7 +67,7 @@ func cmd(logger xlog.Logger, urlIgnoreCertificateErrors bool) (*exec.Cmd, error)
"--no-first-run", "--no-first-run",
} }
if urlIgnoreCertificateErrors { if ignoreCertificateErrors {
args = append(args, "--ignore-certificate-errors") args = append(args, "--ignore-certificate-errors")
} }
@@ -98,7 +98,7 @@ func kill(logger xlog.Logger, proc *os.Process) error {
return nil return nil
} }
func restart(logger xlog.Logger, proc *os.Process, urlIgnoreCertificateErrors bool) error { func restart(logger xlog.Logger, proc *os.Process, ignoreCertificateErrors bool) error {
const op string = "chrome.restart" const op string = "chrome.restart"
logger.DebugOp(op, "restarting Google Chrome headless process using port 9222...") logger.DebugOp(op, "restarting Google Chrome headless process using port 9222...")
resolver := func() error { resolver := func() error {
@@ -106,7 +106,7 @@ func restart(logger xlog.Logger, proc *os.Process, urlIgnoreCertificateErrors bo
if err := kill(logger, proc); err != nil { if err := kill(logger, proc); err != nil {
return err return err
} }
cmd, err := cmd(logger, urlIgnoreCertificateErrors) cmd, err := cmd(logger, ignoreCertificateErrors)
if err != nil { if err != nil {
return err return err
} }
@@ -118,7 +118,7 @@ func restart(logger xlog.Logger, proc *os.Process, urlIgnoreCertificateErrors bo
// if the process failed to restart correctly, // if the process failed to restart correctly,
// we have to restart it again. // we have to restart it again.
if !isViable(logger) { if !isViable(logger) {
return restart(logger, cmd.Process, urlIgnoreCertificateErrors) return restart(logger, cmd.Process, ignoreCertificateErrors)
} }
return nil return nil
} }

View File

@@ -40,8 +40,9 @@ const (
// DefaultGoogleChromeRpccBufferSizeEnvVar contains the name // DefaultGoogleChromeRpccBufferSizeEnvVar contains the name
// of the environment variable "DEFAULT_GOOGLE_CHROME_RPCC_BUFFER_SIZE". // of the environment variable "DEFAULT_GOOGLE_CHROME_RPCC_BUFFER_SIZE".
DefaultGoogleChromeRpccBufferSizeEnvVar string = "DEFAULT_GOOGLE_CHROME_RPCC_BUFFER_SIZE" DefaultGoogleChromeRpccBufferSizeEnvVar string = "DEFAULT_GOOGLE_CHROME_RPCC_BUFFER_SIZE"
// Allow self signed certificates when using a remote url // GoogleChromeIgnoreCertificateErrorsEnvVar contains the name
URLIgnoreCertificateErrorsEnvVar string = "URL_IGNORE_CERTIFICATE_ERRORS" // of the environment variable "GOOGLE_CHROME_IGNORE_CERTIFICATE_ERRORS".
GoogleChromeIgnoreCertificateErrorsEnvVar string = "GOOGLE_CHROME_IGNORE_CERTIFICATE_ERRORS"
) )
// Config contains the application // Config contains the application
@@ -55,7 +56,7 @@ type Config struct {
defaultListenPort int64 defaultListenPort int64
disableGoogleChrome bool disableGoogleChrome bool
disableUnoconv bool disableUnoconv bool
urlIgnoreCertificateErrors bool googleChromeIgnoreCertificateErrors bool
logLevel xlog.Level logLevel xlog.Level
rootPath string rootPath string
maximumGoogleChromeRpccBufferSize int64 maximumGoogleChromeRpccBufferSize int64
@@ -78,7 +79,7 @@ func DefaultConfig() Config {
rootPath: "/", rootPath: "/",
maximumGoogleChromeRpccBufferSize: 104857600, // ~100 MB maximumGoogleChromeRpccBufferSize: 104857600, // ~100 MB
defaultGoogleChromeRpccBufferSize: 1048576, // 1 MB defaultGoogleChromeRpccBufferSize: 1048576, // 1 MB
urlIgnoreCertificateErrors: false, googleChromeIgnoreCertificateErrors: false,
} }
} }
@@ -192,11 +193,11 @@ func FromEnv() (Config, error) {
if err != nil { if err != nil {
return c, err return c, err
} }
urlIgnoreCertificateErrors, err := xassert.BoolFromEnv( googleChromeIgnoreCertificateErrors, err := xassert.BoolFromEnv(
URLIgnoreCertificateErrorsEnvVar, GoogleChromeIgnoreCertificateErrorsEnvVar,
c.urlIgnoreCertificateErrors, c.googleChromeIgnoreCertificateErrors,
) )
c.urlIgnoreCertificateErrors = urlIgnoreCertificateErrors c.googleChromeIgnoreCertificateErrors = googleChromeIgnoreCertificateErrors
if err != nil { if err != nil {
return c, err return c, err
} }
@@ -287,9 +288,6 @@ func (c Config) DefaultGoogleChromeRpccBufferSize() int64 {
return c.defaultGoogleChromeRpccBufferSize return c.defaultGoogleChromeRpccBufferSize
} }
// IgnoreCertificateErrors returns true if func (c Config) GoogleChromeIgnoreCertificateErrors() bool {
// Google Chrome should ignore certificate errors
// in case of self signed certificates for example.
func (c Config) IgnoreCertificateErrors() bool {
return c.urlIgnoreCertificateErrors return c.urlIgnoreCertificateErrors
} }

View File

@@ -380,38 +380,38 @@ func TestDefaultGoogleChromeRpccBufferSizeFromEnv(t *testing.T) {
os.Unsetenv(DefaultGoogleChromeRpccBufferSizeEnvVar) os.Unsetenv(DefaultGoogleChromeRpccBufferSizeEnvVar)
} }
func TestIgnoreCertificateErrorsFromEnv(t *testing.T) { func TestGoogleChromeIgnoreCertificateErrorsFromEnv(t *testing.T) {
var ( var (
expected Config expected Config
result Config result Config
err error err error
) )
// URL_IGNORE_CERTIFICATE_ERRORS correctly set to true. // GOOGLE_CHROME_IGNORE_CERTIFICATE_ERRORS correctly set to true.
os.Setenv(URLIgnoreCertificateErrorsEnvVar, "1") os.Setenv(GoogleChromeIgnoreCertificateErrorsEnvVar, "1")
expected = DefaultConfig() expected = DefaultConfig()
expected.urlIgnoreCertificateErrors = true expected.googleChromeIgnoreCertificateErrors = true
result, err = FromEnv() result, err = FromEnv()
assert.Nil(t, err) assert.Nil(t, err)
assert.Equal(t, expected, result) assert.Equal(t, expected, result)
os.Unsetenv(URLIgnoreCertificateErrorsEnvVar) os.Unsetenv(GoogleChromeIgnoreCertificateErrorsEnvVar)
// URL_IGNORE_CERTIFICATE_ERRORS correctly set to false. // GOOGLE_CHROME_IGNORE_CERTIFICATE_ERRORS correctly set to false.
os.Setenv(URLIgnoreCertificateErrorsEnvVar, "0") os.Setenv(GooleChromeIgnoreCertificateErrorsEnvVar, "0")
expected = DefaultConfig() expected = DefaultConfig()
expected.urlIgnoreCertificateErrors = false expected.googleChromeIgnoreCertificateErrors = false
result, err = FromEnv() result, err = FromEnv()
assert.Nil(t, err) assert.Nil(t, err)
assert.Equal(t, expected, result) assert.Equal(t, expected, result)
os.Unsetenv(URLIgnoreCertificateErrorsEnvVar) os.Unsetenv(GoogleChromeIgnoreCertificateErrorsEnvVar)
// URL_IGNORE_CERTIFICATE_ERRORS wrongly set. // GOOGLE_CHROME_IGNORE_CERTIFICATE_ERRORS wrongly set.
os.Setenv(URLIgnoreCertificateErrorsEnvVar, "foo") os.Setenv(GoogleChromeIgnoreCertificateErrorsEnvVar, "foo")
expected = DefaultConfig() expected = DefaultConfig()
result, err = FromEnv() result, err = FromEnv()
test.AssertError(t, err) test.AssertError(t, err)
assert.Equal(t, expected, result) assert.Equal(t, expected, result)
os.Unsetenv(URLIgnoreCertificateErrorsEnvVar) os.Unsetenv(GoogleChromeIgnoreCertificateErrorsEnvVar)
// URL_IGNORE_CERTIFICATE_ERRORS not set at all. // GOOGLE_CHROME_IGNORE_CERTIFICATE_ERRORS not set at all.
expected = DefaultConfig() expected = DefaultConfig()
expected.urlIgnoreCertificateErrors = false expected.googleChromeIgnoreCertificateErrors = false
result, err = FromEnv() result, err = FromEnv()
assert.Nil(t, err) assert.Nil(t, err)
assert.Equal(t, expected, result) assert.Equal(t, expected, result)
@@ -431,5 +431,5 @@ func TestGetters(t *testing.T) {
assert.Equal(t, result.rootPath, result.RootPath()) assert.Equal(t, result.rootPath, result.RootPath())
assert.Equal(t, result.maximumGoogleChromeRpccBufferSize, result.MaximumGoogleChromeRpccBufferSize()) assert.Equal(t, result.maximumGoogleChromeRpccBufferSize, result.MaximumGoogleChromeRpccBufferSize())
assert.Equal(t, result.defaultGoogleChromeRpccBufferSize, result.DefaultGoogleChromeRpccBufferSize()) assert.Equal(t, result.defaultGoogleChromeRpccBufferSize, result.DefaultGoogleChromeRpccBufferSize())
assert.Equal(t, result.urlIgnoreCertificateErrors, result.IgnoreCertificateErrors()) assert.Equal(t, result.googleChromeIgnoreCertificateErrors, result.GoogleChromeIgnoreCertificateErrors())
} }

View File

@@ -14,7 +14,7 @@ func main() {
systemLogger.FatalOp(op, err) systemLogger.FatalOp(op, err)
} }
// start Google Chrome headless. // start Google Chrome headless.
if err := chrome.Start(systemLogger, config.IgnoreCertificateErrors()); err != nil { if err := chrome.Start(systemLogger, config.GoogleChromeIgnoreCertificateErrors()); err != nil {
systemLogger.FatalOp(op, err) systemLogger.FatalOp(op, err)
} }
} }