handle large item client redirects (#3350)
adds the khttp redirectMiddleware to the http wrapper used for large item downloads, to ensure that proxy servers and other 3xx class responses appropriately follow their redirection. --- #### Does this PR need a docs update or release note? - [x] ✅ Yes, it's included #### Type of change - [x] 🐛 Bugfix #### Issue(s) * #3344 #### Test Plan - [x] ⚡ Unit test - [x] 💚 E2E
This commit is contained in:
parent
3b1a71902b
commit
dbb3bd486d
@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||||||
- Improve restore time on large restores by optimizing how items are loaded from the remote repository.
|
- Improve restore time on large restores by optimizing how items are loaded from the remote repository.
|
||||||
- Remove exchange item filtering based on m365 item ID via the CLI.
|
- Remove exchange item filtering based on m365 item ID via the CLI.
|
||||||
- OneDrive backups no longer include a user's non-default drives.
|
- OneDrive backups no longer include a user's non-default drives.
|
||||||
|
- OneDrive and SharePoint file downloads will properly redirect from 3xx responses.
|
||||||
|
|
||||||
## [v0.7.0] (beta) - 2023-05-02
|
## [v0.7.0] (beta) - 2023-05-02
|
||||||
|
|
||||||
|
|||||||
@ -140,13 +140,20 @@ func defaultTransport() http.RoundTripper {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func internalMiddleware(cc *clientConfig) []khttp.Middleware {
|
func internalMiddleware(cc *clientConfig) []khttp.Middleware {
|
||||||
return []khttp.Middleware{
|
mw := []khttp.Middleware{
|
||||||
&RetryMiddleware{
|
&RetryMiddleware{
|
||||||
MaxRetries: cc.maxRetries,
|
MaxRetries: cc.maxRetries,
|
||||||
Delay: cc.minDelay,
|
Delay: cc.minDelay,
|
||||||
},
|
},
|
||||||
|
khttp.NewRedirectHandler(),
|
||||||
&LoggingMiddleware{},
|
&LoggingMiddleware{},
|
||||||
&ThrottleControlMiddleware{},
|
&ThrottleControlMiddleware{},
|
||||||
&MetricsMiddleware{},
|
&MetricsMiddleware{},
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if len(cc.appendMiddleware) > 0 {
|
||||||
|
mw = append(mw, cc.appendMiddleware...)
|
||||||
|
}
|
||||||
|
|
||||||
|
return mw
|
||||||
}
|
}
|
||||||
|
|||||||
@ -2,9 +2,11 @@ package graph
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"net/http"
|
"net/http"
|
||||||
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"github.com/alcionai/clues"
|
"github.com/alcionai/clues"
|
||||||
|
khttp "github.com/microsoft/kiota-http-go"
|
||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
"github.com/stretchr/testify/suite"
|
"github.com/stretchr/testify/suite"
|
||||||
|
|
||||||
@ -43,3 +45,70 @@ func (suite *HTTPWrapperIntgSuite) TestNewHTTPWrapper() {
|
|||||||
require.NotNil(t, resp)
|
require.NotNil(t, resp)
|
||||||
require.Equal(t, http.StatusOK, resp.StatusCode)
|
require.Equal(t, http.StatusOK, resp.StatusCode)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
type mwForceResp struct {
|
||||||
|
err error
|
||||||
|
resp *http.Response
|
||||||
|
alternate func(*http.Request) (bool, *http.Response, error)
|
||||||
|
}
|
||||||
|
|
||||||
|
func (mw *mwForceResp) Intercept(
|
||||||
|
pipeline khttp.Pipeline,
|
||||||
|
middlewareIndex int,
|
||||||
|
req *http.Request,
|
||||||
|
) (*http.Response, error) {
|
||||||
|
ok, r, e := mw.alternate(req)
|
||||||
|
if ok {
|
||||||
|
return r, e
|
||||||
|
}
|
||||||
|
|
||||||
|
return mw.resp, mw.err
|
||||||
|
}
|
||||||
|
|
||||||
|
type HTTPWrapperUnitSuite struct {
|
||||||
|
tester.Suite
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestHTTPWrapperUnitSuite(t *testing.T) {
|
||||||
|
suite.Run(t, &HTTPWrapperUnitSuite{Suite: tester.NewUnitSuite(t)})
|
||||||
|
}
|
||||||
|
|
||||||
|
func (suite *HTTPWrapperUnitSuite) TestNewHTTPWrapper_redirectMiddleware() {
|
||||||
|
ctx, flush := tester.NewContext()
|
||||||
|
defer flush()
|
||||||
|
|
||||||
|
var (
|
||||||
|
t = suite.T()
|
||||||
|
uri = "https://graph.microsoft.com"
|
||||||
|
path = "/fnords/beaux/regard"
|
||||||
|
url = uri + path
|
||||||
|
)
|
||||||
|
|
||||||
|
// can't use gock for this, or else it'll short-circuit the transport,
|
||||||
|
// and thus skip all the middleware
|
||||||
|
hdr := http.Header{}
|
||||||
|
hdr.Set("Location", "localhost:99999999/smarfs")
|
||||||
|
toResp := &http.Response{
|
||||||
|
StatusCode: 302,
|
||||||
|
Header: hdr,
|
||||||
|
}
|
||||||
|
mwResp := mwForceResp{
|
||||||
|
resp: toResp,
|
||||||
|
alternate: func(req *http.Request) (bool, *http.Response, error) {
|
||||||
|
if strings.HasSuffix(req.URL.String(), "smarfs") {
|
||||||
|
return true, &http.Response{StatusCode: http.StatusOK}, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
return false, nil, nil
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
hw := NewHTTPWrapper(appendMiddleware(&mwResp))
|
||||||
|
|
||||||
|
resp, err := hw.Request(ctx, http.MethodGet, url, nil, nil)
|
||||||
|
|
||||||
|
require.NoError(t, err, clues.ToCore(err))
|
||||||
|
require.NotNil(t, resp)
|
||||||
|
// require.Equal(t, 1, calledCorrectly, "test server was called with expected path")
|
||||||
|
require.Equal(t, http.StatusOK, resp.StatusCode)
|
||||||
|
}
|
||||||
|
|||||||
@ -173,6 +173,8 @@ type clientConfig struct {
|
|||||||
// The minimum delay in seconds between retries
|
// The minimum delay in seconds between retries
|
||||||
minDelay time.Duration
|
minDelay time.Duration
|
||||||
overrideRetryCount bool
|
overrideRetryCount bool
|
||||||
|
|
||||||
|
appendMiddleware []khttp.Middleware
|
||||||
}
|
}
|
||||||
|
|
||||||
type Option func(*clientConfig)
|
type Option func(*clientConfig)
|
||||||
@ -225,6 +227,14 @@ func MinimumBackoff(dur time.Duration) Option {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func appendMiddleware(mw ...khttp.Middleware) Option {
|
||||||
|
return func(c *clientConfig) {
|
||||||
|
if len(mw) > 0 {
|
||||||
|
c.appendMiddleware = mw
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
// Middleware Control
|
// Middleware Control
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
@ -257,5 +267,9 @@ func kiotaMiddlewares(
|
|||||||
&MetricsMiddleware{},
|
&MetricsMiddleware{},
|
||||||
}...)
|
}...)
|
||||||
|
|
||||||
|
if len(cc.appendMiddleware) > 0 {
|
||||||
|
mw = append(mw, cc.appendMiddleware...)
|
||||||
|
}
|
||||||
|
|
||||||
return mw
|
return mw
|
||||||
}
|
}
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user