Skip to content

Commit 1082814

Browse files
authored
Implement RFC-0027: Add configurable route options size limit (cloudfoundry#4934)
* Implement RFC-0027: Add max_route_options_size limit validation Adds configurable size limit for route options JSON payload. Requests exceeding the limit are rejected with HTTP 422. - Add max_route_options_size config key to api_schema and worker_schema - Add default value of 1024 bytes to cloud_controller.yml - Add size validation in OptionsValidator (message layer) - Add validate_route_options_size in Route model (DB layer) - Handle options_size_exceeded errors in RouteCreate and RouteUpdate actions - Add unit tests for all new validation paths * Fix rubocop Style/IfUnlessModifier offense in route_create.rb
1 parent 049267b commit 1082814

11 files changed

Lines changed: 176 additions & 2 deletions

File tree

app/actions/route_create.rb

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,12 @@ def validation_error_quota!(error, space)
9393
end
9494

9595
def validation_error_route!(error)
96-
return unless error.errors.on(:route)&.include?(:hash_header_missing)
96+
error!('Hash header must be present when loadbalancing is set to hash.') if error.errors.on(:route)&.include?(:hash_header_missing)
9797

98-
error!('Hash header must be present when loadbalancing is set to hash.')
98+
return unless error.errors.on(:route)&.include?(:options_size_exceeded)
99+
100+
max_size = Config.config.get(:max_route_options_size)
101+
error!("Route options size exceeded: options must be smaller than #{max_size} bytes.")
99102
end
100103

101104
def validation_error_routing_api!(error)

app/actions/route_update.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,12 @@ def validation_error!(error)
2626
# Handle hash_header validation error for hash loadbalancing
2727
raise Error.new('Hash header must be present when loadbalancing is set to hash.') if error.errors.on(:route)&.include?(:hash_header_missing)
2828

29+
# Handle route options size exceeded error
30+
if error.errors.on(:route)&.include?(:options_size_exceeded)
31+
max_size = Config.config.get(:max_route_options_size)
32+
raise Error.new("Route options size exceeded: options must be smaller than #{max_size} bytes.")
33+
end
34+
2935
# Fallback for any other validation errors
3036
raise Error.new(error.message)
3137
end

app/messages/validators.rb

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,8 @@ def validate(record)
247247
return
248248
end
249249

250+
validate_size(record)
251+
250252
opt = record.options_message
251253

252254
return if opt.valid?
@@ -255,6 +257,17 @@ def validate(record)
255257
record.errors.add(:options, message:)
256258
end
257259
end
260+
261+
private
262+
263+
def validate_size(record)
264+
max_size = VCAP::CloudController::Config.config.get(:max_route_options_size)
265+
options_size = record.options.to_json.bytesize
266+
267+
return unless options_size > max_size
268+
269+
record.errors.add(:options, message: "must be smaller than #{max_size} bytes (actual size is #{options_size} bytes)")
270+
end
258271
end
259272

260273
class ToOneRelationshipValidator < ActiveModel::EachValidator

app/models/runtime/route.rb

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,20 @@ def validate_total_reserved_route_ports
329329
def validate_route_options
330330
return if options.blank?
331331

332+
validate_route_options_size
333+
validate_route_options_hash_header
334+
end
335+
336+
def validate_route_options_size
337+
max_size = Config.config.get(:max_route_options_size)
338+
options_size = options.to_json.bytesize
339+
340+
return unless options_size > max_size
341+
342+
errors.add(:route, :options_size_exceeded)
343+
end
344+
345+
def validate_route_options_hash_header
332346
route_options = options.is_a?(Hash) ? options : options.symbolize_keys
333347
loadbalancing = route_options[:loadbalancing] || route_options['loadbalancing']
334348

config/cloud_controller.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,7 @@ routing_api:
162162
route_services_enabled: true
163163
volume_services_enabled: true
164164
disable_private_domain_cross_space_context_path_route_sharing: false
165+
max_route_options_size: 1024
165166

166167
quota_definitions:
167168
default:

lib/cloud_controller/config_schemas/api_schema.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ class ApiSchema < VCAP::Config
3535
optional(:system_domain_organization) => enum(String, NilClass),
3636
app_domains: Array,
3737
disable_private_domain_cross_space_context_path_route_sharing: bool,
38+
max_route_options_size: Integer,
3839

3940
cpu_weight_min_memory: Integer,
4041
cpu_weight_max_memory: Integer,

lib/cloud_controller/config_schemas/worker_schema.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ class WorkerSchema < VCAP::Config
1414
external_protocol: String,
1515
internal_service_hostname: String,
1616
disable_private_domain_cross_space_context_path_route_sharing: bool,
17+
max_route_options_size: Integer,
1718
readiness_port: {
1819
cloud_controller_worker: Integer
1920
},

spec/unit/actions/route_create_spec.rb

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,37 @@ module VCAP::CloudController
190190
end.to raise_error(RouteCreate::Error, 'Hash header must be present when loadbalancing is set to hash.')
191191
end
192192
end
193+
194+
context 'when options size exceeds the configured limit' do
195+
before do
196+
TestConfig.override(max_route_options_size: 50)
197+
# Enable hash_based_routing feature to allow hash_header in options
198+
VCAP::CloudController::FeatureFlag.make(name: 'hash_based_routing', enabled: true, error_message: nil)
199+
end
200+
201+
let(:message_with_large_options) do
202+
RouteCreateMessage.new({
203+
relationships: {
204+
space: {
205+
data: { guid: space.guid }
206+
},
207+
domain: {
208+
data: { guid: domain.guid }
209+
}
210+
},
211+
options: {
212+
loadbalancing: 'hash',
213+
hash_header: 'X-Custom-Header-Name'
214+
}
215+
})
216+
end
217+
218+
it 'raises an error indicating options size exceeded' do
219+
expect do
220+
subject.create(message: message_with_large_options, space: space, domain: domain)
221+
end.to raise_error(RouteCreate::Error, /Route options size exceeded: options must be smaller than 50 bytes/)
222+
end
223+
end
193224
end
194225

195226
context 'when creating a route with other loadbalancing options' do

spec/unit/actions/route_update_spec.rb

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,30 @@ module VCAP::CloudController
511511
end
512512
end
513513
end
514+
515+
context 'when options size exceeds the configured limit' do
516+
before do
517+
TestConfig.override(max_route_options_size: 50)
518+
# Enable hash_based_routing feature to allow hash_header in options
519+
VCAP::CloudController::FeatureFlag.make(name: 'hash_based_routing', enabled: true, error_message: nil)
520+
route[:options] = '{"loadbalancing": "round-robin"}'
521+
end
522+
523+
let(:body) do
524+
{
525+
options: {
526+
loadbalancing: 'hash',
527+
hash_header: 'a' * 100
528+
}
529+
}
530+
end
531+
532+
it 'raises an error indicating options size exceeded' do
533+
expect do
534+
subject.update(route:, message:)
535+
end.to raise_error(RouteUpdate::Error, /Route options size exceeded: options must be smaller than 50 bytes/)
536+
end
537+
end
514538
end
515539
end
516540
end

spec/unit/messages/route_create_message_spec.rb

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -629,6 +629,49 @@ module VCAP::CloudController
629629
end
630630
end
631631
end
632+
633+
context 'when options size exceeds the configured limit' do
634+
before do
635+
TestConfig.override(max_route_options_size: 50)
636+
end
637+
638+
let(:params) do
639+
{
640+
host: 'some-host',
641+
relationships: {
642+
space: { data: { guid: 'space-guid' } },
643+
domain: { data: { guid: 'domain-guid' } }
644+
},
645+
options: { loadbalancing: 'round-robin', some_large_key: 'a' * 100 }
646+
}
647+
end
648+
649+
it 'is not valid' do
650+
expect(subject).not_to be_valid
651+
expect(subject.errors[:options].to_s).to include('must be smaller than 50 bytes')
652+
end
653+
end
654+
655+
context 'when options size is within the configured limit' do
656+
before do
657+
TestConfig.override(max_route_options_size: 1024)
658+
end
659+
660+
let(:params) do
661+
{
662+
host: 'some-host',
663+
relationships: {
664+
space: { data: { guid: 'space-guid' } },
665+
domain: { data: { guid: 'domain-guid' } }
666+
},
667+
options: { loadbalancing: 'round-robin' }
668+
}
669+
end
670+
671+
it 'is valid' do
672+
expect(subject).to be_valid
673+
end
674+
end
632675
end
633676
end
634677

0 commit comments

Comments
 (0)