Skip to content

Configure org.eclipse.jetty.server.Request.maxFormKeys from server.properties and increase the default value#10214

Merged
DaanHoogland merged 2 commits intoapache:4.19from
shapeblue:max-form-keys
Jan 21, 2025
Merged

Configure org.eclipse.jetty.server.Request.maxFormKeys from server.properties and increase the default value#10214
DaanHoogland merged 2 commits intoapache:4.19from
shapeblue:max-form-keys

Conversation

@abh1sar
Copy link
Copy Markdown
Contributor

@abh1sar abh1sar commented Jan 20, 2025 •
edited
Loading

Description

Fixes #9667

Details This pull request introduces a new configuration option for the Apache CloudStack project to limit the maximum number of form keys allowed in API requests. The changes include:
  • Adding a new configuration property request.max.form.keys with a default value of 5000 in server.properties.in.
  • Updating the ServerDaemon class to handle request.max.form.keys:
    • Defining constants for the new property key and its default value.
    • Adding a new instance variable maxFormKeys.
    • Initializing the new property from the configuration file.
    • Setting the new property in the server attributes and web application context.
    • Adding a setter method for the new property.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • build/CI
  • test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

Tested importRoles UI using the below csv file
roletest.csv

Before change : 404 error
Works after change.

How did you try to break this feature and the system with this change?

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 20, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 15.13%. Comparing base (188eacd) to head (e3be88d).
Report is 16 commits behind head on 4.19.

Additional details and impacted files 
@@            Coverage Diff             @@
##               4.19   #10214    +/-   ##
==========================================
  Coverage     15.12%   15.13%            
- Complexity    11266    11280    +14     
==========================================
  Files          5408     5408            
  Lines        473900   474003   +103     
  Branches      57791    57821    +30     
==========================================
+ Hits          71698    71747    +49     
- Misses       394199   394235    +36     
- Partials       8003     8021    +18
Flag Coverage Δ
uitests 4.30% <ø> (+<0.01%) ⬆️
unittests 15.85% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

DaanHoogland
DaanHoogland approved these changes Jan 20, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

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

clgtm

@DaanHoogland
Copy link
Copy Markdown
Contributor

DaanHoogland commented Jan 20, 2025

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jan 20, 2025

@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jan 20, 2025

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 12131

@DaanHoogland
Copy link
Copy Markdown
Contributor

DaanHoogland commented Jan 20, 2025

@blueorangutan test keepEnv

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jan 20, 2025

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

kiranchavala
kiranchavala approved these changes Jan 21, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@kiranchavala kiranchavala left a comment

Choose a reason for hiding this comment

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

LGTM, uploaded the following csv file and the issue got fixed

https://gist.github.com/scottsignal/737022dba6f3d3b7847d3d0665adc6f3

Before fix, upload the csv file, exception is thrown
Screenshot 2025-01-21 at 1 40 48 PM

After fix there is no error and import is done successfully

Screenshot 2025-01-21 at 1 41 22 PM

@DaanHoogland DaanHoogland merged commit 0b8076c into apache:4.19 Jan 21, 2025
@DaanHoogland DaanHoogland mentioned this pull request Jan 21, 2025
Closed
@DaanHoogland DaanHoogland deleted the max-form-keys branch January 21, 2025 08:29
@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jan 21, 2025

[SF] Trillian test result (tid-12136)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 58018 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr10214-t12136-kvm-ol8.zip
Smoke tests completed. 132 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_03_secured_to_nonsecured_vm_migration Error 397.24 test_vm_life_cycle.py

DaanHoogland added a commit that referenced this pull request Jan 21, 2025
@DaanHoogland
Merge release branch 4.20 to main
81e052c 
* 4.20:
  linstor: Fix ZFS snapshot backup (#10219)
  fix listing of VMs by network (#10204)
  Configure org.eclipse.jetty.server.Request.maxFormKeys from server.properties and increase the default value (#10214)
  api: fix access for listSystemVmUsageHistory (#10032)
  Fix NPE issues during host rolling maintenance, due to host tags and custom constrained/unconstrained service offering (#9844)
rg9975 pushed a commit to rg9975/cloudstack that referenced this pull request Jan 31, 2025
@abh1sar
Configure org.eclipse.jetty.server.Request.maxFormKeys from server.pr…
50a762a 
…operties and increase the default value (apache#10214)
@Pearl1594 Pearl1594 mentioned this pull request Mar 12, 2025
Closed
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Jun 19, 2025
@abh1sar @dhslove
Configure org.eclipse.jetty.server.Request.maxFormKeys from server.pr…
068a28d 
…operties and increase the default value (apache#10214)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kiranchavala kiranchavala kiranchavala approved these changes

@DaanHoogland DaanHoogland DaanHoogland approved these changes

Assignees

No one assigned

Labels

component:management-server status:ready-for-merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@abh1sar @DaanHoogland @blueorangutan @kiranchavala