Bugfix/header modifier doesn't work for user agent property #516
Conversation
| URIBuilder uriBuilder = new URIBuilder().setScheme(HTTP).setHost(server.getAPIHost()) | ||
| .setPort(server.getAPIPort()); | ||
| // Request BMP to add header | ||
| HttpPost request = new HttpPost(uriBuilder.setPath( |
There was a problem hiding this comment.
According to https://github.com/lightbody/browsermob-proxy#rest-api docs:
POST /proxy/[port]/headers
Set and override HTTP Request headers
Do those changes mean, that this is not true? There is even an example of setting the User-Agent header.
There was a problem hiding this comment.
Yes, this doesn't work anymore. You can't use this to change some headers like User-agent. Using this you can only add new header or override header that you add before.
| @@ -14,6 +14,7 @@ In order to use this modifier it must be declared before the open module in the | |||
| | --------- | ----- | ----------- | --------- | | |||
| | `key` | x | Key for the header | yes | | |||
| | `value` | y | Value for the header | yes | | |||
| | `override` | `true`/`false` | Replace existing header instead of adding new one | no (default false) | | |||
There was a problem hiding this comment.
What's the case of using the Header Modifier without override=true?
Is that the case, that the user wants to have more than one header's value?
If that's the case, override=true should be the default option and eventually let the user add more header values using override=false.
Could you please also make it more clear, in the docs, that override=false means, that if the header already occurs in the request, it will not be modified (like with User-Agent example, where Chrome instance sets it).
There was a problem hiding this comment.
Override false is use when you want add new header, true option is used to change headers that are add by Chrome instance. This is why false is default strategy.
I'll make documentation more clear about that.
|
What also concerns me is that https://github.com/lightbody/browsermob-proxy is no longer maintained: |
| @@ -58,7 +58,7 @@ public void testCollect() throws Exception { | |||
| headerModifier.setParameters(ImmutableMap.of("key", "header", "value", "value1")); | |||
| headerModifier.collect(); | |||
|
|
|||
| verify(proxyServer).addHeader("header", "value1"); | |||
| verify(proxyServer).addHeader("header", "value1", false); | |||
There was a problem hiding this comment.
Is there any way to test the override approach?
Maybe if not the unit test, some integration test (e.g. page displaying User-Agent header value?)
tkaik
deleted the
bugfix/header-modifier-doesn't-work-for-User-Agent-property
branch
Description
The problem with changing some headers like User-agent, Accept was solved by changing proxy server configuration.
Motivation and Context
#515
Screenshots (if appropriate)
Upgrade notes (if appropriate)
Types of changes
Checklist:
I hereby agree to the terms of the AET Contributor License Agreement.