Proxy-Authorization header filtered only if proxy authentication activated

[matthiaskraaz]

First of all: thank you so much for taking on brushing up LittleProxy! I am pretty sure there is no good alternative to LittleProxy, so you are doing a really important job!

I am using LittleProxy to test a HTTP client and I have just switched from the original branch to your fork and one of my unit tests went red:

The client sends (obviously pre-emptive) proxy authorization despite LittleProxy requiring no authentication.

The original branch filtered out the the proxy-authorization header nevertheless.

LittleProxy xyz.rogfam:littleproxy:2.0.20 however passes the proxy-authorization header through to the server.

I think the old behavior is more like what you can expect from a "real" proxy.

[asolntsev]

@matthiaskraaz Hi. To be honest, I personally don't know which behaviour is more correct. But could you please share the code of the original branch (or even better - DIFF with current LittleProxy code)?

[matthiaskraaz]

@asolntsev Hi Andrei, the problem is actually much more severe, but I fixed it.

The loop in org.littleshoot.proxy.impl.ClientToProxyConnection.stripHopByHopHeaders(HttpHeaders) ends after removing the first header. It seems like Netty changed the inner workings of its class HttpHeaders. But we are all accustomed to problems when iterating over a collection and removing elements from it at the same time, right?

So my proposed fix is adding to org.littleshoot.proxy.impl.ProxyUtils:

/**
 * Removes all headers that should not be forwarded. See RFC 2616 13.5.1
 * End-to-end and Hop-by-hop Headers.
 *
 * @param headers
 *            The headers to modify
 */
public static void stripHopByHopHeaders(HttpHeaders headers) {
  // Not explicitly documented, but remove is case insensitve as a HTTP header handling function should be
  SHOULD_NOT_PROXY_HOP_BY_HOP_HEADERS.forEach(headers::remove);
}

and changing stripHopByHopHeaders to:

private void stripHopByHopHeaders(HttpHeaders headers) {
  ProxyUtils.stripHopByHopHeaders(headers);
}

You could as well remove the indirection, but I thougt this way the change is easier to understand.

[matthiaskraaz]

PS: I first thought the problem was that org.littleshoot.proxy.impl.ClientToProxyConnection.authenticationRequired removes the proxy authorization header only if proxy authentication is activated, but now I think the removal in authenticationRequired is extraneous and could be removed.

[asolntsev]

@matthiaskraaz Great. Thank you for the explanation. Don't you want to submit a pull request https://github.com/LittleProxy/LittleProxy/pulls?