FIX-15184 resolved NettyHttp Header add by set to empty String

[dheerajaraj]

What is the purpose of the change?

io.netty.handler.codec.Headers.HttpMetadataAdapter is adding a null value for PseudoHeaderName.AUTHORITY key. I checked the docs for io.netty.handler.codec.Headers and it might be linked to this issue. So, I replaced with a default value of empty string "". All the client methods checking for the key PseudoHeaderName.AUTHORITY will do a null or empty value check using StringUtils.isBlank() and StringUtils.isNotBlank

Checklist

• Make sure there is a GitHub_issue field for the change.
• Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
• Write necessary unit-test to verify your logic correction. If the new feature or significant change is committed, please remember to add sample in dubbo samples project.
• Make sure gitHub actions can pass. Why the workflow is failing and how to fix it?

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.

Project coverage is 60.72%. Comparing base (f4b0b13) to head (6fb4cb4).
Report is 84 commits behind head on 3.4.

Files with missing lines	Patch %	Lines
...bo/remoting/http12/message/DefaultHttpRequest.java	0.00%	3 Missing ⚠️
.../rpc/protocol/tri/servlet/HttpMetadataAdapter.java	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##                3.4   #15190   +/-   ##
=========================================
  Coverage     60.72%   60.72%           
- Complexity    10864    10866    +2     
=========================================
  Files          1881     1881           
  Lines         85967    85968    +1     
  Branches      12881    12882    +1     
=========================================
+ Hits          52200    52204    +4     
- Misses        28315    28320    +5     
+ Partials       5452     5444    -8     

Flag	Coverage Δ
integration-tests	32.99% <0.00%> (-0.07%)	⬇️
samples-tests	29.22% <0.00%> (+0.02%)	⬆️
unit-tests	58.87% <0.00%> (-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.

[RainYuY]

I think the target branch should be set to 3.3 instead of 3.4.

[dheerajaraj]

hi @Stellar1999 actually the target branch was mentioned to be 3.3.3 in this issue. The target class (io.netty.handler.codec.Headers.HttpMetadataAdapter) which is causing the Null Pointer Exception is occurring in 3.4 not in 3.3. Since I cannot find 3.3.3 in the upstream repo, I've set the target branch to 3.4. Is this fine? Kindly advice 🙏

[RainYuY]

hi @Stellar1999 actually the target branch was mentioned to be 3.3.3 in this issue. The target class (io.netty.handler.codec.Headers.HttpMetadataAdapter) which is causing the Null Pointer Exception is occurring in 3.4 not in 3.3. Since I cannot find 3.3.3 in the upstream repo, I've set the target branch to 3.4. Is this fine? Kindly advice 🙏

Version 3.3.3 is a subversion of 3.3, so I believe it should be merged into 3.3 first. After that, the Dubbo team usually merges it into 3.4 as well. I noticed you mentioned that this issue doesn’t affect 3.3. Does that mean you’re unable to reproduce this bug in version 3.3?

[dheerajaraj]

Hi @Stellar1999, for version 3.3.3/3.4 the place where the error is happening (org.apache.dubbo.rpc.protocol.tri.servlet.HttpMetadataAdapter.headers()#57) looks like this as also mentioned in the original issue:

However, the same line in 3.3 looks like this:

Since you mentioned that 3.3.3 will be merged to 3.3,
shall i change in 3.3 request.getServerName() to request.getHeader(HttpHeaderNames.HOST.getName()) as how it's done in 3.3.3 & 3.4?

[RainYuY]

Hi @Stellar1999, for version 3.3.3/3.4 the place where the error is happening (org.apache.dubbo.rpc.protocol.tri.servlet.HttpMetadataAdapter.headers()#57) looks like this as also mentioned in the original issue:

However, the same line in 3.3 looks like this:

Since you mentioned that 3.3.3 will be merged to 3.3, shall i change in 3.3 request.getServerName() to request.getHeader(HttpHeaderNames.HOST.getName()) as how it's done in 3.3.3 & 3.4?

Oh, I see. I think this bug has already been fixed by PR #15141. All we need to do now is merge 3.3 into 3.4. PTAL @zrlw @AlbumenJ

[dheerajaraj]

Hi @Stellar1999 if i'm not wrong, the bug is caused because of this PR #15141. @robin977 could you kindly confirm if this the case?

[AlbumenJ]

I will merge 3.3 into 3.4 later

[AlbumenJ]

I will merge 3.3 into 3.4 later

done