Bug 62252 - HTTP header merging logic does not correspond to the documentation
Summary: HTTP header merging logic does not correspond to the documentation
Status: RESOLVED FIXED
Alias: None
Product: JMeter
Classification: Unclassified
Component: HTTP (show other bugs)
Version: 4.0
Hardware: All All
: P2 major (vote)
Target Milestone: JMETER_5.0
Assignee: JMeter issues mailing list
URL:
Keywords: FixedInTrunk
Depends on:
Blocks:
 
Reported: 2018-04-03 15:14 UTC by Michael Osipov
Modified: 2018-04-11 10:12 UTC (History)
1 user (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Osipov 2018-04-03 15:14:27 UTC
1. Add a header manager in test plan with "Content-Type: text/plain"
2. Add thread group
2.1. Add header manager to TG with "Content-Type: "
2.2. Add HTTP request to TG with, e.g., PUT
3. Run the plan

Result tree says:
> Request Headers:
> Connection: keep-alive
> Content-Type: 
> Content-Length: 0
> Host: ...
> User-Agent: Apache-HttpClient/4.5.5 (Java/1.8.0_161)

The documentation says:
> JMeter now supports multiple Header Managers. The header entries are merged to form the list for the sampler. If an entry to be merged matches an existing header name, it replaces the previous entry, unless the entry value is empty, in which case any existing entry is removed. This allows one to set up a default set of headers, and apply adjustments to particular samplers.

Checking the source code: https://github.com/apache/jmeter/blob/bdb88cb72a5b71f5bbc6f35a03f36890268b468b/src/protocol/http/org/apache/jmeter/protocol/http/control/HeaderManager.java#L262-L282

I cannot see that on an empty value the parent header is removed. A non-existing header and a empty value header are different things.

See lengthly discussion in Jetty: https://github.com/eclipse/jetty.project/issues/1116

Basically, there is no remove here, but a mere override.
Comment 1 Philippe Mouawad 2018-04-05 21:25:41 UTC
(In reply to Michael Osipov from comment #0)
> 1. Add a header manager in test plan with "Content-Type: text/plain"
> 2. Add thread group
> 2.1. Add header manager to TG with "Content-Type: "
> 2.2. Add HTTP request to TG with, e.g., PUT
> 3. Run the plan
> 
> Result tree says:
> > Request Headers:
> > Connection: keep-alive
> > Content-Type: 
> > Content-Length: 0
> > Host: ...
> > User-Agent: Apache-HttpClient/4.5.5 (Java/1.8.0_161)
> 
> The documentation says:
> > JMeter now supports multiple Header Managers. The header entries are merged to form the list for the sampler. If an entry to be merged matches an existing header name, it replaces the previous entry, unless the entry value is empty, in which case any existing entry is removed. This allows one to set up a default set of headers, and apply adjustments to particular samplers.
> 
> Checking the source code:
> https://github.com/apache/jmeter/blob/
> bdb88cb72a5b71f5bbc6f35a03f36890268b468b/src/protocol/http/org/apache/jmeter/
> protocol/http/control/HeaderManager.java#L262-L282
> 
> I cannot see that on an empty value the parent header is removed. A
> non-existing header and a empty value header are different things.
> 
> See lengthly discussion in Jetty:
> https://github.com/eclipse/jetty.project/issues/1116
> 
> Basically, there is no remove here, but a mere override.

Yes documentation is wrong.

What is your opinion ? 
How should we understand the jetty discussion ? Should we be able to send empty header value ?

Thanks
Comment 2 Michael Osipov 2018-04-05 21:41:35 UTC
(In reply to Philippe Mouawad from comment #1)
> (In reply to Michael Osipov from comment #0)
> > 1. Add a header manager in test plan with "Content-Type: text/plain"
> > 2. Add thread group
> > 2.1. Add header manager to TG with "Content-Type: "
> > 2.2. Add HTTP request to TG with, e.g., PUT
> > 3. Run the plan
> > 
> > Result tree says:
> > > Request Headers:
> > > Connection: keep-alive
> > > Content-Type: 
> > > Content-Length: 0
> > > Host: ...
> > > User-Agent: Apache-HttpClient/4.5.5 (Java/1.8.0_161)
> > 
> > The documentation says:
> > > JMeter now supports multiple Header Managers. The header entries are merged to form the list for the sampler. If an entry to be merged matches an existing header name, it replaces the previous entry, unless the entry value is empty, in which case any existing entry is removed. This allows one to set up a default set of headers, and apply adjustments to particular samplers.
> > 
> > Checking the source code:
> > https://github.com/apache/jmeter/blob/
> > bdb88cb72a5b71f5bbc6f35a03f36890268b468b/src/protocol/http/org/apache/jmeter/
> > protocol/http/control/HeaderManager.java#L262-L282
> > 
> > I cannot see that on an empty value the parent header is removed. A
> > non-existing header and a empty value header are different things.
> > 
> > See lengthly discussion in Jetty:
> > https://github.com/eclipse/jetty.project/issues/1116
> > 
> > Basically, there is no remove here, but a mere override.
> 
> Yes documentation is wrong.
> 
> What is your opinion ? 
> How should we understand the jetty discussion ? Should we be able to send
> empty header value ?


The problem is two fold:
1. There is no way to drop a header from being sent, regardless of the docs.
2. The doc says that a empty header drops the previous one, but empty headers are perfectly valid. The Jetty issue was just an demonstration of a misinterpretation of the RFC.

Of course, I could use a preprocessor, but that would make the header manager obsolete. In fact, I did not even try edge cases like supplying the same header several times which is perfectly valid according to RFC 7230.
Comment 3 Philippe Mouawad 2018-04-05 22:02:10 UTC
Hello,
Can you open another bugzilla for the ability to remove a header.

Do you have an idea how we could ergonomically propose that ?

Note that Content-Type is set by HC4 in some cases.
Thanks
Comment 4 Michael Osipov 2018-04-10 13:43:53 UTC
(In reply to Philippe Mouawad from comment #3)
> Hello,
> Can you open another bugzilla for the ability to remove a header.
> 
> Do you have an idea how we could ergonomically propose that ?

I will open a new one. Yes, I have some ideas. We need basically to add a third column in the pane with the label "Action". An enum with "Add", "Replace" and "Remove".

"Add": Adds a header value even is already present in the parent or in the child. Multiple header values are valid.
"Replace": Replaces the (all) previous header value(s) with this value.
"Remove": Removes one distinct matching header value or if left blank, all values.

Make sense?

> Note that Content-Type is set by HC4 in some cases.

Which are? I will remove them right away because the use has to be in control of this.
Comment 5 Philippe Mouawad 2018-04-10 17:08:37 UTC
Author: 	pmouawad
Date: 	Tue Apr 10 17:03:44 2018 UTC
Changed paths: 	2
Log Message: 	

Bug 62252 - HTTP header merging logic does not correspond to the documentation
Bugzilla Id: 62252

Changed paths
jmeter/trunk/xdocs/changes.xml
jmeter/trunk/xdocs/usermanual/component_reference.xml
Comment 6 Michael Osipov 2018-04-11 10:12:56 UTC
(In reply to Philippe Mouawad from comment #5)
> Author: 	pmouawad
> Date: 	Tue Apr 10 17:03:44 2018 UTC
> Changed paths: 	2
> Log Message: 	
> 
> Bug 62252 - HTTP header merging logic does not correspond to the
> documentation
> Bugzilla Id: 62252
> 
> Changed paths
> jmeter/trunk/xdocs/changes.xml
> jmeter/trunk/xdocs/usermanual/component_reference.xml

Changes look good now.

Can you still point me to the spots in HC4 where content-type is set by default?