Presto: Session value is not correct, the bahavior of server and client is not consistent

[weidongduan37]

presto version: 0.236

case 1:

TZ=UTC java -jar presto-cli-*-executable.jar --server http://localhost:8080 --user admin --session execution_policy="a+1" --execute "SELECT 1 FROM (VALUES (ARRAY[1])) AS t (a) CROSS JOIN UNNEST(a)"

The request http header is as following:

X-Presto-User: admin
User-Agent: StatementClientV1/0.236xx
X-Presto-Source: presto-cli
X-Presto-Time-Zone: UTC
X-Presto-Language: en-US
X-Presto-Session: execution_policy=a+1
X-Presto-Transaction-Id: NONE
Content-Type: text/plain; charset=UTF-8
Content-Length: 63
Host: localhost:8080
Connection: keep-alive
Accept-Encoding: gzip

case 2:

TZ=UTC java -jar presto-cli-*-executable.jar --server http://localhost:8080 --user admin
presto> set session execution_policy='a+1';
SET SESSION
presto> SELECT 1 FROM (VALUES (ARRAY[1])) AS t (a) CROSS JOIN UNNEST(a);

The request http header is as following:

X-Presto-User: admin
User-Agent: StatementClientV1/0.236xx
X-Presto-Source: presto-cli
X-Presto-Time-Zone: UTC
X-Presto-Language: en-US
X-Presto-Session: execution_policy=a%2B1
X-Presto-Transaction-Id: NONE
Content-Type: text/plain; charset=UTF-8
Content-Length: 63
Host: localhost:8080
Connection: keep-alive
Accept-Encoding: gzip

conclusion:
X-Presto-Session is not same, and on web ui, the case 2 also shows session incorrectly.

Analysis:

For case 1, client does not encode session value in presto-cli.
However, the second case, in ExecutingStatementResource.java, server 'urlEncode' the session value when query "set session ...." comes, but does not decode the session value in HttpRequestSessionContext.java when query "SELECT 1 FROM (VALUES (ARRAY[1])) AS t (a) CROSS JOIN UNNEST(a)" comes.

I know the session value "a+1" is invalid, and in most of cases the value will be correct due to no special character involves. However, the behavior at server side and client side is indeed not consistent. Could anyone help to fix it? By the way, Does any other client also need a corresponding fix?
like any code need to be changed in https://github.com/prestodb/presto-go-client

[mbasmanova]

CC: @mayankgarg1990 @tdcmeehan

[mbasmanova]

CC: @neeradsomanchi

[tdcmeehan]

@weidongduan37 thanks for the bug report, it does indeed look like a bug. It looks like there's a simple fix for this in presto-client. Would you like to raise a PR for it? We can raise issues and track separately other clients.

[highker]

@tdcmeehan, do you think it's a good ticket for @Naveen007 to work on?

[tdcmeehan]

@tdcmeehan, do you think it's a good ticket for @Naveen007 to work on?

Yes. @Naveen007 please feel free to claim this ticket. In short, we just need to make sure that the client also percent-decodes the Presto session header, since it's being percent-encoded on the server side.

[weidongduan37]

@tdcmeehan
I try to fix as following
https://github.com/prestodb/presto/pull/14817

Thanks.