Query string incorrectly encoded in the HTTP request

[azinemanas]

Making a connection to ws://localhost/test?q=value1%26value2 If I look at the request line it is: GET /test?q=value1&value2 HTTP/1.1

Analizing the source code of GrizzlyClientFilter.java, it seems like the query string is decoded in the method getHttpContent:

    final String query = uri.getQuery();
    if (query != null) {
        sb.append('?').append(query);
    }

I think it must be uri.getRawQuery();

Tested with version 1.12 and 1.19. The same code seems to be in version 2.1.

[jansupol]

I am not convinced that getRawQuery is the right method here, since it creates from value1%26value2 encoded q=value1%2526value2, which is likely not what is meant to be there.

Can you more elaborate why you think the code is incorrect? What do you mean by the request line?

[azinemanas]

@jansupol i have this code to make a connection to a server:

final URI uri = new URI(url + "?q=" + URLEncoder.encode("value1&value2", "UTF-8")); client.connectToServer(endpoint, config, uri);

where the result uri is http://localhost/test?q=value1%26value2.

But in the server in the onOpen event when I try to get the query param q I get the value 'value1'.

Looking at the raw HTTP request (with tcpdump to discard a problem in the server side) the HTTP request is:

`GET /test?q=value1&value2

...`

The & is not escaped in the request.

Debugging the code, the line in the first post is where I see that the URI is rebuild to trim the scheme and host part to get the final URI.

[jansupol]

Usually, query is name1=value1&name2=value2&....nameN=valueN. In your case, it is not quite clear whether value2 is a query name without a value, or whether it should be something else. Did you expect the value of q to be whole value1&value2?

[azinemanas]

Yes, I want the value of q to be value1&value2. The choice of names perhaps is not clear. In this case I have a value with an & in it, is not an nam/value separator.

[jansupol]

How do you parse the value from Session#getQueryString?

[azinemanas]

I use session.getRequestParameterMap().get("q").get(0) and I get value1, not value1&value2.

[jansupol]

There is the following behaviour:

• new URI(uri + "?q=" + URLEncoder.encode("value1&value2", "UTF-8")) -> q=value1%26value2
• new URI("ws", null, host, port, path, "q=value1&value2", null) -> q=value1&value2.
• new URI("ws", null, host, port, path, "q=" + URLEncoder.encode("value1&value2", "UTF-8"), null) ->q=value1%2526value2

Basically your query q=value1%26value2 cannot be created using the non-string URI constructor. That makes me think that the way you do it is not correct.

[azinemanas]

I made a test project to make more clear the problem. It test the initial websocket request with a plain HTTP request made with URLConnection. The code to construct the URI o URL is the same but the request produced not.

test-tyrus.zip

[jansupol]

The following could help to understand both ways of creating the URI:

        final String query = uri.getQuery();
        if (query != null) {
            sb.append('?').append(query.contains("%") ? query : uri.getRawQuery());
        }

[azinemanas]

I this case query is 'q=value1&value2' not 'q=value1%26value2' because getQuery decodes the query string. So the result is the same.

[jansupol]

Nope. For your case, query is decoded, it contains value1&value2, hence no "%" and sb.append(uri.getRawQuery()).

[azinemanas]

You are right. What would be the reason to not always use getRawQuery?

[jansupol]

When someone would use new URI("ws", null, host, port, path, "q=" + URLEncoder.encode("value1&value2", "UTF-8"), null), it would fail

[azinemanas]

I think in this case is not correct to use URLEncoder.encode because that constructor also encodes the query string.

[jansupol]

But it does not encode new URI("ws", null, host, port, path, "q=value1&value2", null)

[azinemanas]

I think in this case you can't construct the URI using this constructor if you want q to be value1&value2. But this is a restriction of the URI class. In this cases I guess the option is to use a less error prone alternative like UriBuilder from Apache HTTPClient.

[jansupol]

Maybe, but it is what is currently working and someone can use it that way. The new behaviour would break their app. I do not believe a framework can allow that. However, sb.append(query.contains("%") ? query : uri.getRawQuery()) does not work if there is query argument containing something like val=20%. I am afraid the fix would be more complicated.

[azinemanas]

Totally agree that someone can use it that way. I think is not possible to keep compatibility only interpreting the string, you always have possible ambiguities. The solution I think is include a flag to change the behaviour.

[jansupol]

Something like this perhaps