ssvm: fix post request header case mismatch

[shwstppr]

Description

Fixes #7442

With d74f64a2e16 key for the host header was changed to lowercase which is causing host be null while processing upload in SSVM

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)

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?

[codecov]

Codecov Report

Merging #7445 (83d1f19) into 4.18 (dabefca) will increase coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##               4.18    #7445    +/-   ##
==========================================
  Coverage     12.70%   12.70%            
  Complexity     8673     8673            
==========================================
  Files          2717     2715     -2     
  Lines        256186   256071   -115     
  Branches      39929    39926     -3     
==========================================
  Hits          32541    32541            
+ Misses       219507   219391   -116     
- Partials       4138     4139     +1     

see 5 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[shwstppr]

@blueorangutan package

[blueorangutan]

@shwstppr a 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]

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 5927

[shwstppr]

@blueorangutan test

[blueorangutan]

@shwstppr a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

[blueorangutan]

Trillian test result (tid-6421)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 43109 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7445-t6421-kvm-centos7.zip
Smoke tests completed. 108 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

[yadvr]

LGTM, didn't test it. We must test it manually before merging. cc @borisstoyanov @vladimirpetrov @kiranchavala

[JoaoJandre]

Suggested change

	if (HEADER_SIGNATURE.equalsIgnoreCase(entry.getKey())) {
	signature = entry.getValue();
	} else if (HEADER_METADATA.equalsIgnoreCase(entry.getKey())) {
	metadata = entry.getValue();
	} else if (HEADER_EXPIRES.equalsIgnoreCase(entry.getKey())) {
	expires = entry.getValue();
	} else if (HEADER_HOST.equalsIgnoreCase(entry.getKey())) {
	hostname = entry.getValue();
	} else if (HttpHeaders.Names.CONTENT_LENGTH.equalsIgnoreCase(entry.getKey())) {
	contentLength = Long.parseLong(entry.getValue());
	switch (entry.getKey().toLowerCase()) {
	case HEADER_SIGNATURE:
	signature = entry.getValue();
	break;
	case HEADER_METADATA:
	metadata = entry.getValue();
	break;
	case HEADER_EXPIRES:
	expires = entry.getValue();
	break;
	case HEADER_HOST:
	hostname = entry.getValue();
	break;
	case CONTENT_LENGTH:
	contentLength = Long.parseLong(entry.getValue());
	break;

I think we should leave the switch case, which is more efficient and readable, and simply use toLowerCase() on the entry. Apart from this suggestion, you would have to define the CONTENT_LENGTH static variable (which should be on lowercase), as it is done with the rest of the headers in this class.
Lastly, down on the writeResponse method, the line response.headers().set(CONTENT_LENGTH, buf.readableBytes()); would have to be changed to response.headers().set(HttpHeaders.Names.CONTENT_LENGTH, buf.readableBytes()); in order to maintain current behavior.

[shwstppr]

I prefer current changes for minimal edits

[JoaoJandre]

The change that I proposed would touch only 5 lines of code, instead of 17 😄

[nvazquez]

Code LGTM

[JoaoJandre]

@shwstppr nice refactoring! just a couple of issues

[shwstppr]

@blueorangutan package

[blueorangutan]

@shwstppr a 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.

[JoaoJandre]

CLGTM, but we should still test this manually

[blueorangutan]

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 5956

[shwstppr]

@blueorangutan test

[blueorangutan]

@shwstppr a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

58.6% Coverage
0.0% Duplication

[blueorangutan]

Trillian test result (tid-6442)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 38638 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7445-t6442-kvm-centos7.zip
Smoke tests completed. 108 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

[LiThuim]

Hi. Where can we get the latest image with this fix? We have the same hostname=null issue

[shwstppr]

@LiThuim sorry for the late response. This PR is merged in 4.18 branch so for the official release you may have to wait for the upcoming 4.18.1 release.
Alternatively, you test a nightly build from here, https://download.cloudstack.org/testing/nightly/

[LiThuim]

@shwstppr do you perhaps know when the new release will be available?

[shwstppr]

@LiThuim you may have to follow the mailing lists for the timeline, https://markmail.org/thread/passo7fzhsrzxv5h
Expecting it to be released in a month or so