Skip to content

fix: return err if direct response size exceeds limit #5710

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.

Sign up for GitHub

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

Merged
arkodg merged 3 commits into from
Apr 11, 2025
Merged

fix: return err if direct response size exceeds limit #5710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4bc6598
fix: return err if direct response size exceeds limit
Apr 10, 2025
d7d08e1
lint
Apr 10, 2025
876e5f2
add another check
Apr 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions internal/gatewayapi/backendtrafficpolicy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -930,19 +930,37 @@ func buildResponseOverride(policy *egv1a1.BackendTrafficPolicy, resources *resou
}, nil
}

func checkResponseBodySize(b *string) error {
// Make this configurable in the future
// https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route.proto.html#max_direct_response_body_size_bytes
Comment thread
arkodg marked this conversation as resolved.
maxDirectResponseSize := 4096
lenB := len(*b)
if lenB > maxDirectResponseSize {
return fmt.Errorf("response.body size %d greater than the max size %d", lenB, maxDirectResponseSize)
}

return nil
}

func getCustomResponseBody(body *egv1a1.CustomResponseBody, resources *resource.Resources, policyNs string) (*string, error) {
if body != nil && body.Type != nil && *body.Type == egv1a1.ResponseValueTypeValueRef {
cm := resources.GetConfigMap(policyNs, string(body.ValueRef.Name))
if cm != nil {
b, dataOk := cm.Data["response.body"]
switch {
case dataOk:
if err := checkResponseBodySize(&b); err != nil {
return nil, err
}
return &b, nil
case len(cm.Data) > 0: // Fallback to the first key if response.body is not found
for _, value := range cm.Data {
b = value
break
}
if err := checkResponseBodySize(&b); err != nil {
return nil, err
}
return &b, nil
default:
return nil, fmt.Errorf("can't find the key response.body in the referenced configmap %s", body.ValueRef.Name)
Expand All @@ -952,6 +970,9 @@ func getCustomResponseBody(body *egv1a1.CustomResponseBody, resources *resource.
return nil, fmt.Errorf("can't find the referenced configmap %s", body.ValueRef.Name)
}
} else if body != nil && body.Inline != nil {
if err := checkResponseBodySize(body.Inline); err != nil {
return nil, err
}
return body.Inline, nil
}

Expand Down
34 changes: 33 additions & 1 deletion internal/gatewayapi/testdata/httproute-with-direct-response.in.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ httpRoutes:
- apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
name: direct-response-with-errors
name: direct-response-with-value-not-found
namespace: default
spec:
parentRefs:
Expand All @@ -67,6 +67,27 @@ httpRoutes:
group: gateway.envoyproxy.io
kind: HTTPRouteFilter
name: direct-response-value-ref-not-found
- apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
name: direct-response-too-long
namespace: default
spec:
parentRefs:
- name: gateway-1
namespace: envoy-gateway
sectionName: http
rules:
- matches:
- path:
type: PathPrefix
value: /too-long
filters:
- type: ExtensionRef
extensionRef:
group: gateway.envoyproxy.io
kind: HTTPRouteFilter
name: direct-response-too-long
configMaps:
- apiVersion: v1
kind: ConfigMap
Expand Down Expand Up @@ -117,3 +138,14 @@ httpFilters:
group: ""
kind: ConfigMap
name: value-ref-response
- apiVersion: gateway.envoyproxy.io/v1alpha1
kind: HTTPRouteFilter
metadata:
name: direct-response-too-long
namespace: default
spec:
directResponse:
contentType: text/plain
body:
type: Inline
inline: "-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------"
45 changes: 43 additions & 2 deletions internal/gatewayapi/testdata/httproute-with-direct-response.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ gateways:
protocol: HTTP
status:
listeners:
- attachedRoutes: 2
- attachedRoutes: 3
conditions:
- lastTransitionTime: null
message: Sending translated listener configuration to the data plane
Expand Down Expand Up @@ -95,7 +95,7 @@ httpRoutes:
kind: HTTPRoute
metadata:
creationTimestamp: null
name: direct-response-with-errors
name: direct-response-with-value-not-found
namespace: default
spec:
parentRefs:
Expand Down Expand Up @@ -131,6 +131,47 @@ httpRoutes:
name: gateway-1
namespace: envoy-gateway
sectionName: http
- apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
creationTimestamp: null
name: direct-response-too-long
namespace: default
spec:
parentRefs:
- name: gateway-1
namespace: envoy-gateway
sectionName: http
rules:
- filters:
- extensionRef:
group: gateway.envoyproxy.io
kind: HTTPRouteFilter
name: direct-response-too-long
type: ExtensionRef
matches:
- path:
type: PathPrefix
value: /too-long
status:
parents:
- conditions:
- lastTransitionTime: null
message: 'Invalid filter HTTPRouteFilter: response.body size 4097 greater
than the max size 4096'
reason: UnsupportedValue
status: "False"
type: Accepted
- lastTransitionTime: null
message: Resolved all the Object references for the Route
reason: ResolvedRefs
status: "True"
type: ResolvedRefs
controllerName: gateway.envoyproxy.io/gatewayclass-controller
parentRef:
name: gateway-1
namespace: envoy-gateway
sectionName: http
infraIR:
envoy-gateway/gateway-1:
proxy:
Expand Down