okhttp: HttpUrl.Builder.addQueryParameter is not escaping curly braces which causes Tomcat 8 to reject request

OkHttp does not properly escape URL query parameters that contain curly braces.

Tomcat 7.0.73+, 8.0.39+, and 8.5.7+ will reject these requests with the following error:

Mar 20, 2017 2:47:59 PM org.apache.coyote.http11.Http11Processor service
INFO: Error parsing HTTP request header
 Note: further occurrences of HTTP header parsing errors will be logged at DEBUG level.
java.lang.IllegalArgumentException: Invalid character found in the request target. The valid characters are defined in RFC 7230 and RFC 3986
        at org.apache.coyote.http11.Http11InputBuffer.parseRequestLine(Http11InputBuffer.java:471)
        at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:667)
        at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:66)
        at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:798)
        at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1434)
        at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
        at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
        at java.lang.Thread.run(Thread.java:745)

These are the characters that Tomcat will now reject as part of the URI request line (see org.apache.tomcat.util.http.parser.HttpParser):

            if (IS_CONTROL[i] || i > 127 ||
                    i == ' ' || i == '\"' || i == '#' || i == '<' || i == '>' || i == '\\' ||
                    i == '^' || i == '`'  || i == '{' || i == '|' || i == '}') {
                IS_NOT_REQUEST_TARGET[i] = true;  // reject the character!

To solve issue, the characters included in HttpUrl.QUERY_COMPONENT_ENCODE_SET may need to be changed. Here is current definition:

static final String QUERY_COMPONENT_ENCODE_SET = " \"'<>#&=";

These

About this issue

Original URL
State: closed
Created 7 years ago
Comments: 21 (11 by maintainers)

Commits related to this issue

Encode more characters when using addQueryParameter(). Though it's good for OkHttp to retain the user's provided encoding, we should be encoding more characters than we are when the user provides us ... — committed to square/okhttp by swankjesse 6 years ago
Encode more characters when using addQueryParameter(). Though it's good for OkHttp to retain the user's provided encoding, we should be encoding more characters than we are when the user provides us ... — committed to square/okhttp by swankjesse 6 years ago
Encode more characters when using addQueryParameter(). (#3863) Though it's good for OkHttp to retain the user's provided encoding, we should be encoding more characters than we are when the user pro... — committed to square/okhttp by swankjesse 6 years ago

Most upvoted comments

Thanks for the link. I hadn’t seen that yet.

I wouldn’t say that they ‘fixed it’, as the issue that I experienced would continue to occur unless the System property tomcat.util.http.parser.HttpParser.requestTargetAllow is set.

Not that HttpClient’s URLBuilder is the gold-standard (or I wouldn’t be using OkHttp3), but I’d respectfully suggest that modifying HttpUrl builder to escape {, }, and | would be a good thing to do 😃

arakelian on Mar 21, 2017