Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Session value is not correct, the bahavior of server and client is not consistent #14807

Closed
weidongduan37 opened this issue Jul 8, 2020 · 6 comments · Fixed by #14817
Closed

Comments

@weidongduan37
Copy link
Contributor

weidongduan37 commented Jul 8, 2020

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.

Screen Shot 2020-07-08 at 10 57 55 PM

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
Copy link
Contributor

CC: @mayankgarg1990 @tdcmeehan

@mbasmanova
Copy link
Contributor

CC: @neeradsomanchi

@tdcmeehan
Copy link
Contributor

tdcmeehan commented Jul 8, 2020

@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
Copy link
Contributor

highker commented Jul 8, 2020

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

@tdcmeehan
Copy link
Contributor

@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
Copy link
Contributor Author

@tdcmeehan
I try to fix as following
#14817

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants