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.

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

Allow idiomatic usage of middlewares in gorilla/mux #9802

Merged
merged 1 commit into from Jun 11, 2020

Conversation

harshavardhana
Copy link
Member

Description

Historically due to lack of support for middlewares
we ended up writing wrapped handlers for all
middlewares on top of the gorilla/mux, this causes
multiple issues when we want to let's say

  • Overload r.Body with some custom implementation
    to track the incoming Reads()
  • Add other sort of top level checks to avoid
    DDOSing the server with large incoming HTTP
    bodies.

Since 1.7.x release gorilla/mux provides proper
use of middlewares, which are honored by the muxer
directly. This makes sure that Go can honor its
own internal ServeHTTP(w, r) implementation where
Go net/http can wrap into its own customer readers.

This PR as a side-affect fixes rare issues of client
hangs which were reported in the wild but never really
understood or fixed in our codebase.

Motivation and Context

Fixes #9759
Fixes #7266
Fixes #6540
Fixes #5455
Fixes #5150

Refer boto/botocore#1328 for
one variation of the same issue in #9759

How to test this PR?

As per #9759

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Fixes a regression probably a long time ago
  • Documentation needed
  • Unit tests needed
  • Functional tests needed (If yes, add mint PR # here: )

Historically due to lack of support for middlewares
we ended up writing wrapped handlers for all
middlewares on top of the gorilla/mux, this causes
multiple issues when we want to let's say

- Overload r.Body with some custom implementation
  to track the incoming Reads()
- Add other sort of top level checks to avoid
  DDOSing the server with large incoming HTTP
  bodies.

Since 1.7.x release gorilla/mux provides proper
use of middlewares, which are honored by the muxer
directly. This makes sure that Go can honor its
own internal ServeHTTP(w, r) implementation where
Go net/http can wrap into its own customer readers.

This PR as a side-affect fixes rare issues of client
hangs which were reported in the wild but never really
understood or fixed in our codebase.

Fixes minio#9759
Fixes minio#7266
Fixes minio#6540
Fixes minio#5455
Fixes minio#5150

Refer boto/botocore#1328 for
one variation of the same issue in minio#9759
@minio-trusted
Copy link
Contributor

Mint Automation

Test Result
mint-xl.sh ✔️
mint-large-bucket.sh ✔️
mint-fs.sh ✔️
mint-dist-xl.sh ✔️
mint-gateway-s3.sh ✔️
mint-gateway-azure.sh ✔️
mint-gateway-nas.sh ✔️
Deleting image on docker hub
Deleting image locally

Copy link
Contributor

@kannappanr kannappanr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested. LGTM

Copy link
Member

@vadmeste vadmeste left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@harshavardhana harshavardhana merged commit a42df3d into minio:master Jun 11, 2020
@harshavardhana harshavardhana deleted the fix-expect branch June 11, 2020 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment