From 24f4d43a0f874f87321c897899ac88ec6c66e87b Mon Sep 17 00:00:00 2001 From: DarthSim Date: Tue, 21 Mar 2023 20:58:16 +0300 Subject: [PATCH] use "clean" HTTP transport; Use context for downloading timeout control --- imagedata/download.go | 70 ++++++++++++++++++++++++++--------------- imagedata/error.go | 43 +++++++++++++++++++++++++ imagedata/image_data.go | 8 ++--- imagedata/read.go | 9 ++++-- imagedata/timeout.go | 14 --------- processing_handler.go | 2 +- stream.go | 8 +++-- 7 files changed, 104 insertions(+), 50 deletions(-) create mode 100644 imagedata/error.go delete mode 100644 imagedata/timeout.go diff --git a/imagedata/download.go b/imagedata/download.go index cd18e006..2b364550 100644 --- a/imagedata/download.go +++ b/imagedata/download.go @@ -2,9 +2,11 @@ package imagedata import ( "compress/gzip" + "context" "crypto/tls" "fmt" "io" + "net" "net/http" "net/http/cookiejar" "time" @@ -55,16 +57,25 @@ func (e *ErrorNotModified) Error() string { } func initDownloading() error { - transport := http.DefaultTransport.(*http.Transport).Clone() - transport.DisableCompression = true + transport := &http.Transport{ + Proxy: http.ProxyFromEnvironment, + DialContext: (&net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + DualStack: true, + }).DialContext, + MaxIdleConns: 100, + MaxIdleConnsPerHost: config.Concurrency + 1, + IdleConnTimeout: time.Duration(config.ClientKeepAliveTimeout) * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + ForceAttemptHTTP2: true, + DisableCompression: true, + } - if config.ClientKeepAliveTimeout > 0 { - transport.MaxIdleConns = config.Concurrency - transport.MaxIdleConnsPerHost = config.Concurrency - transport.IdleConnTimeout = time.Duration(config.ClientKeepAliveTimeout) * time.Second - } else { - transport.MaxIdleConns = 0 - transport.MaxIdleConnsPerHost = 0 + if config.ClientKeepAliveTimeout <= 0 { + transport.MaxIdleConnsPerHost = -1 + transport.DisableKeepAlives = true } if config.IgnoreSslVerification { @@ -113,7 +124,6 @@ func initDownloading() error { } downloadClient = &http.Client{ - Timeout: time.Duration(config.DownloadTimeout) * time.Second, Transport: transport, CheckRedirect: func(req *http.Request, via []*http.Request) error { redirects := len(via) @@ -139,14 +149,18 @@ func headersToStore(res *http.Response) map[string]string { return m } -func BuildImageRequest(imageURL string, header http.Header, jar *cookiejar.Jar) (*http.Request, error) { - req, err := http.NewRequest("GET", imageURL, nil) +func BuildImageRequest(ctx context.Context, imageURL string, header http.Header, jar *cookiejar.Jar) (*http.Request, context.CancelFunc, error) { + reqCtx, reqCancel := context.WithTimeout(ctx, time.Duration(config.DownloadTimeout)*time.Second) + + req, err := http.NewRequestWithContext(reqCtx, "GET", imageURL, nil) if err != nil { - return nil, ierrors.New(404, err.Error(), msgSourceImageIsUnreachable) + reqCancel() + return nil, func() {}, ierrors.New(404, err.Error(), msgSourceImageIsUnreachable) } if _, ok := enabledSchemes[req.URL.Scheme]; !ok { - return nil, ierrors.New( + reqCancel() + return nil, func() {}, ierrors.New( 404, fmt.Sprintf("Unknown scheme: %s", req.URL.Scheme), msgSourceImageIsUnreachable, @@ -167,37 +181,41 @@ func BuildImageRequest(imageURL string, header http.Header, jar *cookiejar.Jar) } } - return req, nil + return req, reqCancel, nil } func SendRequest(req *http.Request) (*http.Response, error) { res, err := downloadClient.Do(req) if err != nil { - return nil, ierrors.New(500, checkTimeoutErr(err).Error(), msgSourceImageIsUnreachable) + return nil, wrapError(err) } return res, nil } -func requestImage(imageURL string, opts DownloadOptions) (*http.Response, error) { - req, err := BuildImageRequest(imageURL, opts.Header, opts.CookieJar) +func requestImage(ctx context.Context, imageURL string, opts DownloadOptions) (*http.Response, context.CancelFunc, error) { + req, reqCancel, err := BuildImageRequest(ctx, imageURL, opts.Header, opts.CookieJar) if err != nil { - return nil, err + reqCancel() + return nil, func() {}, err } res, err := SendRequest(req) if err != nil { - return nil, err + reqCancel() + return nil, func() {}, err } if res.StatusCode == http.StatusNotModified { res.Body.Close() - return nil, &ErrorNotModified{Message: "Not Modified", Headers: headersToStore(res)} + reqCancel() + return nil, func() {}, &ErrorNotModified{Message: "Not Modified", Headers: headersToStore(res)} } if res.StatusCode != 200 { body, _ := io.ReadAll(res.Body) res.Body.Close() + reqCancel() status := 404 if res.StatusCode >= 500 { @@ -205,19 +223,21 @@ func requestImage(imageURL string, opts DownloadOptions) (*http.Response, error) } msg := fmt.Sprintf("Status: %d; %s", res.StatusCode, string(body)) - return nil, ierrors.New(status, msg, msgSourceImageIsUnreachable) + return nil, func() {}, ierrors.New(status, msg, msgSourceImageIsUnreachable) } - return res, nil + return res, reqCancel, nil } -func download(imageURL string, opts DownloadOptions, secopts security.Options) (*ImageData, error) { +func download(ctx context.Context, imageURL string, opts DownloadOptions, secopts security.Options) (*ImageData, error) { // We use this for testing if len(redirectAllRequestsTo) > 0 { imageURL = redirectAllRequestsTo } - res, err := requestImage(imageURL, opts) + res, reqCancel, err := requestImage(ctx, imageURL, opts) + defer reqCancel() + if res != nil { defer res.Body.Close() } diff --git a/imagedata/error.go b/imagedata/error.go new file mode 100644 index 00000000..5e942b3e --- /dev/null +++ b/imagedata/error.go @@ -0,0 +1,43 @@ +package imagedata + +import ( + "context" + "errors" + "fmt" + "net/http" + + "github.com/imgproxy/imgproxy/v3/ierrors" +) + +type httpError interface { + Timeout() bool +} + +func wrapError(err error) error { + isTimeout := false + + if errors.Is(err, context.Canceled) { + return ierrors.New( + 499, + fmt.Sprintf("The image request is cancelled: %s", err), + msgSourceImageIsUnreachable, + ) + } else if errors.Is(err, context.DeadlineExceeded) { + isTimeout = true + } else if httpErr, ok := err.(httpError); ok { + isTimeout = httpErr.Timeout() + } + + if !isTimeout { + return err + } + + ierr := ierrors.New( + http.StatusGatewayTimeout, + fmt.Sprintf("The image request timed out: %s", err), + msgSourceImageIsUnreachable, + ) + ierr.Unexpected = true + + return ierr +} diff --git a/imagedata/image_data.go b/imagedata/image_data.go index 7a83b089..17d1a1bf 100644 --- a/imagedata/image_data.go +++ b/imagedata/image_data.go @@ -70,7 +70,7 @@ func loadWatermark() (err error) { } if len(config.WatermarkURL) > 0 { - Watermark, err = Download(config.WatermarkURL, "watermark", DownloadOptions{Header: nil, CookieJar: nil}, security.DefaultOptions()) + Watermark, err = Download(context.Background(), config.WatermarkURL, "watermark", DownloadOptions{Header: nil, CookieJar: nil}, security.DefaultOptions()) return } @@ -84,7 +84,7 @@ func loadFallbackImage() (err error) { case len(config.FallbackImagePath) > 0: FallbackImage, err = FromFile(config.FallbackImagePath, "fallback image", security.DefaultOptions()) case len(config.FallbackImageURL) > 0: - FallbackImage, err = Download(config.FallbackImageURL, "fallback image", DownloadOptions{Header: nil, CookieJar: nil}, security.DefaultOptions()) + FallbackImage, err = Download(context.Background(), config.FallbackImageURL, "fallback image", DownloadOptions{Header: nil, CookieJar: nil}, security.DefaultOptions()) default: FallbackImage, err = nil, nil } @@ -130,8 +130,8 @@ func FromFile(path, desc string, secopts security.Options) (*ImageData, error) { return imgdata, nil } -func Download(imageURL, desc string, opts DownloadOptions, secopts security.Options) (*ImageData, error) { - imgdata, err := download(imageURL, opts, secopts) +func Download(ctx context.Context, imageURL, desc string, opts DownloadOptions, secopts security.Options) (*ImageData, error) { + imgdata, err := download(ctx, imageURL, opts, secopts) if err != nil { if nmErr, ok := err.(*ErrorNotModified); ok { nmErr.Message = fmt.Sprintf("Can't download %s: %s", desc, nmErr.Message) diff --git a/imagedata/read.go b/imagedata/read.go index 49d53faf..103c1aae 100644 --- a/imagedata/read.go +++ b/imagedata/read.go @@ -42,13 +42,14 @@ func readAndCheckImage(r io.Reader, contentLength int, secopts security.Options) return nil, ErrSourceImageTypeNotSupported } - return nil, checkTimeoutErr(err) + return nil, wrapError(err) } if err = security.CheckDimensions(meta.Width(), meta.Height(), 1, secopts); err != nil { buf.Reset() cancel() - return nil, err + + return nil, wrapError(err) } if contentLength > buf.Cap() { @@ -56,8 +57,10 @@ func readAndCheckImage(r io.Reader, contentLength int, secopts security.Options) } if err = br.Flush(); err != nil { + buf.Reset() cancel() - return nil, checkTimeoutErr(err) + + return nil, wrapError(err) } return &ImageData{ diff --git a/imagedata/timeout.go b/imagedata/timeout.go deleted file mode 100644 index 7225f871..00000000 --- a/imagedata/timeout.go +++ /dev/null @@ -1,14 +0,0 @@ -package imagedata - -import "errors" - -type httpError interface { - Timeout() bool -} - -func checkTimeoutErr(err error) error { - if httpErr, ok := err.(httpError); ok && httpErr.Timeout() { - return errors.New("The image request timed out") - } - return err -} diff --git a/processing_handler.go b/processing_handler.go index fddb1b94..8f5d2da2 100644 --- a/processing_handler.go +++ b/processing_handler.go @@ -303,7 +303,7 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) { checkErr(ctx, "download", err) } - return imagedata.Download(imageURL, "source image", downloadOpts, po.SecurityOptions) + return imagedata.Download(ctx, imageURL, "source image", downloadOpts, po.SecurityOptions) }() if err == nil { diff --git a/stream.go b/stream.go index a6e76064..692e81a9 100644 --- a/stream.go +++ b/stream.go @@ -71,14 +71,16 @@ func streamOriginImage(ctx context.Context, reqID string, r *http.Request, rw ht checkErr(ctx, "streaming", err) } - req, err := imagedata.BuildImageRequest(imageURL, imgRequestHeader, cookieJar) + req, reqCancel, err := imagedata.BuildImageRequest(r.Context(), imageURL, imgRequestHeader, cookieJar) + defer reqCancel() checkErr(ctx, "streaming", err) res, err := imagedata.SendRequest(req) + if res != nil { + defer res.Body.Close() + } checkErr(ctx, "streaming", err) - defer res.Body.Close() - for _, k := range streamRespHeaders { vv := res.Header.Values(k) for _, v := range vv {