Skip to content

Commit 98d99d5

Browse files
shblue21aldas
authored andcommitted
fix proxy panic when balancer has no target
1 parent d17c907 commit 98d99d5

2 files changed

Lines changed: 116 additions & 0 deletions

File tree

middleware/proxy.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,9 @@ func (config ProxyConfig) ToMiddleware() (echo.MiddlewareFunc, error) {
365365
if err != nil {
366366
return config.ErrorHandler(c, err)
367367
}
368+
if tgt == nil || tgt.URL == nil {
369+
return config.ErrorHandler(c, echo.NewHTTPError(http.StatusBadGateway, "no proxy target available"))
370+
}
368371

369372
c.Set(config.ContextKey, tgt)
370373

middleware/proxy_test.go

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,100 @@ func TestRoundRobinBalancerWithNoTargets(t *testing.T) {
457457
assert.NoError(t, err)
458458
}
459459

460+
func TestProxyWithNoTargetReturnsBadGateway(t *testing.T) {
461+
targetURL, _ := url.Parse("http://127.0.0.1:8080")
462+
target := &ProxyTarget{Name: "target", URL: targetURL}
463+
emptyAfterRemove := NewRoundRobinBalancer([]*ProxyTarget{target})
464+
assert.True(t, emptyAfterRemove.RemoveTarget("target"))
465+
466+
testCases := []struct {
467+
name string
468+
balancer ProxyBalancer
469+
}{
470+
{
471+
name: "random balancer with nil targets",
472+
balancer: NewRandomBalancer(nil),
473+
},
474+
{
475+
name: "round-robin balancer with nil targets",
476+
balancer: NewRoundRobinBalancer(nil),
477+
},
478+
{
479+
name: "round-robin balancer after removing last target",
480+
balancer: emptyAfterRemove,
481+
},
482+
{
483+
name: "custom balancer with nil target",
484+
balancer: &customBalancer{},
485+
},
486+
{
487+
name: "custom balancer with nil target URL",
488+
balancer: &customBalancer{target: &ProxyTarget{Name: "target"}},
489+
},
490+
}
491+
492+
for _, tc := range testCases {
493+
t.Run(tc.name, func(t *testing.T) {
494+
e := echo.New()
495+
errorHandlerCalled := false
496+
e.Use(ProxyWithConfig(ProxyConfig{
497+
Balancer: tc.balancer,
498+
ErrorHandler: func(c *echo.Context, err error) error {
499+
errorHandlerCalled = true
500+
httpErr, ok := err.(*echo.HTTPError)
501+
assert.True(t, ok, "expected http error to be passed to handler")
502+
assert.Equal(t, http.StatusBadGateway, httpErr.Code, "expected http bad gateway error to be passed to handler")
503+
return err
504+
},
505+
}))
506+
507+
req := httptest.NewRequest(http.MethodGet, "/", nil)
508+
rec := httptest.NewRecorder()
509+
510+
assert.NotPanics(t, func() {
511+
e.ServeHTTP(rec, req)
512+
})
513+
assert.True(t, errorHandlerCalled)
514+
assert.Equal(t, http.StatusBadGateway, rec.Code)
515+
})
516+
}
517+
}
518+
519+
func TestProxyWithNoTargetDoesNotRetry(t *testing.T) {
520+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
521+
w.WriteHeader(http.StatusOK)
522+
}))
523+
defer server.Close()
524+
targetURL, _ := url.Parse(server.URL)
525+
526+
balancer := &sequenceBalancer{
527+
targets: []*ProxyTarget{
528+
nil,
529+
{Name: "target", URL: targetURL},
530+
},
531+
}
532+
533+
retryFilterCalled := false
534+
e := echo.New()
535+
e.Use(ProxyWithConfig(ProxyConfig{
536+
Balancer: balancer,
537+
RetryCount: 1,
538+
RetryFilter: func(c *echo.Context, err error) bool {
539+
retryFilterCalled = true
540+
return true
541+
},
542+
}))
543+
544+
req := httptest.NewRequest(http.MethodGet, "/", nil)
545+
rec := httptest.NewRecorder()
546+
547+
e.ServeHTTP(rec, req)
548+
549+
assert.False(t, retryFilterCalled)
550+
assert.Equal(t, 1, balancer.calls)
551+
assert.Equal(t, http.StatusBadGateway, rec.Code)
552+
}
553+
460554
func TestProxyRetries(t *testing.T) {
461555
newServer := func(res int) (*url.URL, *httptest.Server) {
462556
server := httptest.NewServer(
@@ -788,6 +882,25 @@ func (b *customBalancer) Next(c *echo.Context) (*ProxyTarget, error) {
788882
return b.target, nil
789883
}
790884

885+
type sequenceBalancer struct {
886+
targets []*ProxyTarget
887+
calls int
888+
}
889+
890+
func (b *sequenceBalancer) AddTarget(target *ProxyTarget) bool {
891+
return false
892+
}
893+
894+
func (b *sequenceBalancer) RemoveTarget(name string) bool {
895+
return false
896+
}
897+
898+
func (b *sequenceBalancer) Next(c *echo.Context) (*ProxyTarget, error) {
899+
target := b.targets[b.calls]
900+
b.calls++
901+
return target, nil
902+
}
903+
791904
func TestModifyResponseUseContext(t *testing.T) {
792905
server := httptest.NewServer(
793906
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

0 commit comments

Comments
 (0)