From 64af65b0e71acc855566a43f0fd50ad91ffc58af Mon Sep 17 00:00:00 2001 From: Julien Neuhart Date: Thu, 14 Dec 2023 21:39:36 +0100 Subject: [PATCH] feat(chromium): return 409 Conflict if HTTP status code from main page is not acceptable (#755) --- pkg/modules/chromium/browser.go | 18 +++++++++++ pkg/modules/chromium/browser_test.go | 33 ++++++++++++++++++++ pkg/modules/chromium/chromium.go | 12 +++++++- pkg/modules/chromium/events.go | 32 +++++++++++++++++++ pkg/modules/chromium/routes.go | 29 ++++++++++++++++-- pkg/modules/chromium/routes_test.go | 46 ++++++++++++++++++++++++++++ 6 files changed, 166 insertions(+), 4 deletions(-) diff --git a/pkg/modules/chromium/browser.go b/pkg/modules/chromium/browser.go index adfb774c..96f1829e 100644 --- a/pkg/modules/chromium/browser.go +++ b/pkg/modules/chromium/browser.go @@ -243,6 +243,16 @@ func (b *chromiumBrowser) pdf(ctx context.Context, logger *zap.Logger, url, outp // If a request does not pass the validation, we make it fail. listenForEventRequestPaused(taskCtx, logger, b.arguments.allowList, b.arguments.denyList) + var ( + invalidHttpStatusCode error + invalidHttpStatusCodeMu sync.RWMutex + ) + + // See https://github.com/gotenberg/gotenberg/issues/613. + if len(options.FailOnHttpStatusCodes) != 0 { + listenForEventResponseReceived(taskCtx, logger, url, options.FailOnHttpStatusCodes, &invalidHttpStatusCode, &invalidHttpStatusCodeMu) + } + var ( consoleExceptions error consoleExceptionsMu sync.RWMutex @@ -287,6 +297,14 @@ func (b *chromiumBrowser) pdf(ctx context.Context, logger *zap.Logger, url, outp return fmt.Errorf("print to PDF: %w", err) } + // See https://github.com/gotenberg/gotenberg/issues/613. + invalidHttpStatusCodeMu.RLock() + defer invalidHttpStatusCodeMu.RUnlock() + + if invalidHttpStatusCode != nil { + return fmt.Errorf("%v: %w", invalidHttpStatusCode, ErrInvalidHttpStatusCode) + } + // See https://github.com/gotenberg/gotenberg/issues/262. consoleExceptionsMu.RLock() defer consoleExceptionsMu.RUnlock() diff --git a/pkg/modules/chromium/browser_test.go b/pkg/modules/chromium/browser_test.go index afe9e214..59b99395 100644 --- a/pkg/modules/chromium/browser_test.go +++ b/pkg/modules/chromium/browser_test.go @@ -415,6 +415,39 @@ func TestChromiumBrowser_pdf(t *testing.T) { "skipping network idle event", }, }, + { + scenario: "ErrInvalidHttpStatusCode", + browser: newChromiumBrowser( + browserArguments{ + binPath: os.Getenv("CHROMIUM_BIN_PATH"), + wsUrlReadTimeout: 5 * time.Second, + allowList: regexp.MustCompile(""), + denyList: regexp.MustCompile(""), + }, + ), + fs: func() *gotenberg.FileSystem { + fs := gotenberg.NewFileSystem() + + err := os.MkdirAll(fs.WorkingDirPath(), 0o755) + if err != nil { + t.Fatalf(fmt.Sprintf("expected no error but got: %v", err)) + } + + err = os.WriteFile(fmt.Sprintf("%s/index.html", fs.WorkingDirPath()), []byte("

ErrInvalidHttpStatusCode

"), 0o755) + if err != nil { + t.Fatalf("expected no error but got: %v", err) + } + + return fs + }(), + options: Options{ + FailOnHttpStatusCodes: []int64{299}, + }, + noDeadline: false, + start: true, + expectError: true, + expectedError: ErrInvalidHttpStatusCode, + }, { scenario: "ErrConsoleExceptions", browser: newChromiumBrowser( diff --git a/pkg/modules/chromium/chromium.go b/pkg/modules/chromium/chromium.go index ba9c3f19..640c1e19 100644 --- a/pkg/modules/chromium/chromium.go +++ b/pkg/modules/chromium/chromium.go @@ -48,6 +48,10 @@ var ( // ChromeDevTools are larger than 100 MB. ErrRpccMessageTooLarge = errors.New("rpcc message too large") + // ErrInvalidHttpStatusCode happens when the status code from the main page + // matches with one of the entry in [Options.FailOnHttpStatusCodes]. + ErrInvalidHttpStatusCode = errors.New("invalid HTTP status code") + // ErrConsoleExceptions happens when there are exceptions in the Chromium // console. It also happens only if the [Options.FailOnConsoleExceptions] // is set to true. @@ -76,6 +80,11 @@ type Options struct { // Optional. SkipNetworkIdleEvent bool + // FailOnHttpStatusCodes sets if the conversion should fail if the status + // code from the main page matches with one of its entries. + // Optional. + FailOnHttpStatusCodes []int64 + // FailOnConsoleExceptions sets if the conversion should fail if there are // exceptions in the Chromium console. // Optional. @@ -179,6 +188,7 @@ type Options struct { func DefaultOptions() Options { return Options{ SkipNetworkIdleEvent: false, + FailOnHttpStatusCodes: []int64{499, 599}, FailOnConsoleExceptions: false, WaitDelay: 0, WaitWindowStatus: "", @@ -436,7 +446,7 @@ func (mod *Chromium) Routes() ([]api.Route, error) { // Pdf converts a URL to PDF. func (mod *Chromium) Pdf(ctx context.Context, logger *zap.Logger, url, outputPath string, options Options) error { - // FIXME: no error wrapping because it leaks on console exceptions output. + // Note: no error wrapping because it leaks on console exceptions output. return mod.supervisor.Run(ctx, logger, func() error { return mod.browser.pdf(ctx, logger, url, outputPath, options) }) diff --git a/pkg/modules/chromium/events.go b/pkg/modules/chromium/events.go index 257821b2..e5128c34 100644 --- a/pkg/modules/chromium/events.go +++ b/pkg/modules/chromium/events.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "regexp" + "slices" "sync" "github.com/chromedp/cdproto/cdp" @@ -60,6 +61,37 @@ func listenForEventRequestPaused(ctx context.Context, logger *zap.Logger, allowL }) } +// listenForEventResponseReceived listens for an invalid HTTP status code is +// returned by the main page. +// See https://github.com/gotenberg/gotenberg/issues/613. +func listenForEventResponseReceived(ctx context.Context, logger *zap.Logger, url string, failOnHttpStatusCodes []int64, invalidHttpStatusCode *error, invalidHttpStatusCodeMu *sync.RWMutex) { + for _, code := range []int64{199, 299, 399, 499, 599} { + if slices.Contains(failOnHttpStatusCodes, code) { + for i := code - 99; i <= code; i++ { + failOnHttpStatusCodes = append(failOnHttpStatusCodes, i) + } + } + } + + chromedp.ListenTarget(ctx, func(ev interface{}) { + switch ev := ev.(type) { + case *network.EventResponseReceived: + if ev.Response.URL != url { + return + } + + logger.Debug(fmt.Sprintf("event EventResponseReceived fired for main page: %+v", ev.Response)) + + if slices.Contains(failOnHttpStatusCodes, ev.Response.Status) { + invalidHttpStatusCodeMu.Lock() + defer invalidHttpStatusCodeMu.Unlock() + + *invalidHttpStatusCode = fmt.Errorf("%d: %s", ev.Response.Status, ev.Response.StatusText) + } + } + }) +} + // listenForEventExceptionThrown listens for exceptions in the console and // appends those exceptions to the given error pointer. // See https://github.com/gotenberg/gotenberg/issues/262. diff --git a/pkg/modules/chromium/routes.go b/pkg/modules/chromium/routes.go index 1d5038ab..3d4d6300 100644 --- a/pkg/modules/chromium/routes.go +++ b/pkg/modules/chromium/routes.go @@ -28,6 +28,7 @@ func FormDataChromiumPdfOptions(ctx *api.Context) (*api.FormData, Options) { var ( skipNetworkIdleEvent bool + failOnHttpStatusCodes []int64 failOnConsoleExceptions bool waitDelay time.Duration waitWindowStatus string @@ -44,6 +45,19 @@ func FormDataChromiumPdfOptions(ctx *api.Context) (*api.FormData, Options) { form := ctx.FormData(). Bool("skipNetworkIdleEvent", &skipNetworkIdleEvent, defaultOptions.SkipNetworkIdleEvent). + Custom("failOnHttpStatusCodes", func(value string) error { + if value == "" { + failOnHttpStatusCodes = defaultOptions.FailOnHttpStatusCodes + return nil + } + + err := json.Unmarshal([]byte(value), &failOnHttpStatusCodes) + if err != nil { + return fmt.Errorf("unmarshal failOnHttpStatusCodes: %w", err) + } + + return nil + }). Bool("failOnConsoleExceptions", &failOnConsoleExceptions, defaultOptions.FailOnConsoleExceptions). Duration("waitDelay", &waitDelay, defaultOptions.WaitDelay). String("waitWindowStatus", &waitWindowStatus, defaultOptions.WaitWindowStatus). @@ -51,13 +65,12 @@ func FormDataChromiumPdfOptions(ctx *api.Context) (*api.FormData, Options) { Custom("extraHttpHeaders", func(value string) error { if value == "" { extraHttpHeaders = defaultOptions.ExtraHttpHeaders - return nil } err := json.Unmarshal([]byte(value), &extraHttpHeaders) if err != nil { - return fmt.Errorf("unmarshal extra HTTP headers: %w", err) + return fmt.Errorf("unmarshal extraHttpHeaders: %w", err) } return nil @@ -65,7 +78,6 @@ func FormDataChromiumPdfOptions(ctx *api.Context) (*api.FormData, Options) { Custom("emulatedMediaType", func(value string) error { if value == "" { emulatedMediaType = defaultOptions.EmulatedMediaType - return nil } @@ -94,6 +106,7 @@ func FormDataChromiumPdfOptions(ctx *api.Context) (*api.FormData, Options) { options := Options{ SkipNetworkIdleEvent: skipNetworkIdleEvent, + FailOnHttpStatusCodes: failOnHttpStatusCodes, FailOnConsoleExceptions: failOnConsoleExceptions, WaitDelay: waitDelay, WaitWindowStatus: waitWindowStatus, @@ -369,6 +382,16 @@ func convertUrl(ctx *api.Context, chromium Api, engine gotenberg.PdfEngine, url ) } + if errors.Is(err, ErrInvalidHttpStatusCode) { + return api.WrapError( + fmt.Errorf("convert to PDF: %w", err), + api.NewSentinelHttpError( + http.StatusConflict, + fmt.Sprintf("Invalid HTTP status code from the main page: %s", strings.ReplaceAll(err.Error(), fmt.Sprintf(": %s", ErrInvalidHttpStatusCode.Error()), "")), + ), + ) + } + if errors.Is(err, ErrConsoleExceptions) { return api.WrapError( fmt.Errorf("convert to PDF: %w", err), diff --git a/pkg/modules/chromium/routes_test.go b/pkg/modules/chromium/routes_test.go index a5d5a740..2222c170 100644 --- a/pkg/modules/chromium/routes_test.go +++ b/pkg/modules/chromium/routes_test.go @@ -28,6 +28,40 @@ func TestFormDataChromiumPdfOptions(t *testing.T) { ctx: &api.ContextMock{Context: new(api.Context)}, expectedOptions: DefaultOptions(), }, + { + scenario: "invalid failOnHttpStatusCodes form field", + ctx: func() *api.ContextMock { + ctx := &api.ContextMock{Context: new(api.Context)} + ctx.SetValues(map[string][]string{ + "failOnHttpStatusCodes": { + "foo", + }, + }) + return ctx + }(), + expectedOptions: func() Options { + options := DefaultOptions() + options.FailOnHttpStatusCodes = nil + return options + }(), + }, + { + scenario: "valid failOnHttpStatusCodes form field", + ctx: func() *api.ContextMock { + ctx := &api.ContextMock{Context: new(api.Context)} + ctx.SetValues(map[string][]string{ + "failOnHttpStatusCodes": { + `[399,499,599]`, + }, + }) + return ctx + }(), + expectedOptions: func() Options { + options := DefaultOptions() + options.FailOnHttpStatusCodes = []int64{399, 499, 599} + return options + }(), + }, { scenario: "invalid extraHttpHeaders form field", ctx: func() *api.ContextMock { @@ -639,6 +673,18 @@ func TestConvertUrl(t *testing.T) { expectHttpStatus: http.StatusBadRequest, expectOutputPathsCount: 0, }, + { + scenario: "ErrInvalidHttpStatusCode", + ctx: &api.ContextMock{Context: new(api.Context)}, + api: &ApiMock{func(ctx context.Context, logger *zap.Logger, url, outputPath string, options Options) error { + return ErrInvalidHttpStatusCode + }}, + options: DefaultOptions(), + expectError: true, + expectHttpError: true, + expectHttpStatus: http.StatusConflict, + expectOutputPathsCount: 0, + }, { scenario: "ErrConsoleExceptions", ctx: &api.ContextMock{Context: new(api.Context)},