AbstractProxyServlet.onServerResponseHeaders does not support headers with empty values

[danrohrer]

https://github.com/eclipse/jetty.project/blob/8404eb0db1a362b20d4dc35a7d48ac2fe41ddfce/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/AbstractProxyServlet.java#L666

There were changes made some time ago to support setting headers with empty values (see: #1116) however the change did not carry through to onServerResponseHeaders().

Wondering if this was an oversight or intentional.

[joakime]

Wait, I thought a null headerValue means it doesn't exist, and an empty header value "" means it exists, but has no value?
@gregw @sbordet thoughts?

[danrohrer]

In this particular code, a null could be returned if the header name was being explicitly filtered out, hence the check for null in line 666 above. I'm referring to the case where a header (with an actual name) is assigned a value that is empty (as in myheader: \r\n). In such a case, the value's length will be 0, and the code will skip adding the response header.

[danrohrer]

Any further thoughts on this? RFC 9110 Section 5.5 seems to allow for an empty header field value. Same was true of earlier RFCs. Section 5.1 also states "A proxy MUST forward unrecognized header fields unless ... the proxy is specifically configured to block, or
otherwise transform, such fields." Just wondering if this will be addressed or if I'll need to override the behavior in my own code.

[gregw]

@danrohrer what version of Jetty? We are not making any changes like this to Jetty-9.4, which is end of life for community support. But sounds like something we should address in >= 10

[danrohrer]

>= 10 will work for me

[sbordet]

@danrohrer can you check whether #8904 works for you?

[danrohrer]

This code change is exactly what we’ll need. I am not currently able to build and test the actual code change at this time, but if that’s a requirement, I’ll need a few more days to coordinate and confirm. Once approved for merge, will this change be pushed into jetty-11.0.x as well? We will be moving to that jetty release in the coming months. Thanks, Dan Rohrer Siemens PLM Software From: Simone Bordet ***@***.***> Sent: Wednesday, November 16, 2022 11:24 AM To: eclipse/jetty.project ***@***.***> Cc: Rohrer, Dan (DI SW LCS CF CLP CLI) ***@***.***>; Mention ***@***.***> Subject: Re: [eclipse/jetty.project] AbstractProxyServlet.onServerResponseHeaders does not support headers with empty values (Issue #8750) @danrohrer<https://github.com/danrohrer> can you check whether #8904<#8904> works for you? — Reply to this email directly, view it on GitHub<#8750 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/A3XMTUAJYKJ7WPC2K6NHHXDWIUKD7ANCNFSM6AAAAAARKQ7AIY>. You are receiving this because you were mentioned.Message ID: ***@***.******@***.***>>

[sbordet]

Yes, we'll merge the PR to Jetty 10 and then merge forward to Jetty 11 and Jetty 12.