fix method invocation automatically unescape by Taction · Pull Request #4020 · dapr/dapr

Taction · 2021-12-11T08:03:04Z

Description

Do not automatically unescape params in method invocation.

Issue reference

Please reference the issue this PR will close: #4008

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

Code compiles correctly
Created/updated tests
Unit tests passing
End-to-end tests passing
Extended the documentation / Created issue in the https://github.com/dapr/docs/ repo: dapr/docs#[issue number]
Specification has been updated / Created issue in the https://github.com/dapr/docs/ repo: dapr/docs#[issue number]
Provided sample for the feature / Created issue in the https://github.com/dapr/docs/ repo: dapr/docs#[issue number]

codecov · 2021-12-11T08:09:07Z

Codecov Report

Merging #4020 (8761da3) into master (050c71a) will decrease coverage by 0.03%.
The diff coverage is 81.25%.

@@            Coverage Diff             @@
##           master    #4020      +/-   ##
==========================================
- Coverage   63.55%   63.52%   -0.04%     
==========================================
  Files         102      102              
  Lines        9743     9751       +8     
==========================================
+ Hits         6192     6194       +2     
- Misses       3070     3073       +3     
- Partials      481      484       +3

Impacted Files	Coverage Δ
pkg/channel/http/http_channel.go	`68.31% <66.66%> (-0.84%)`	⬇️
pkg/http/api.go	`79.92% <100.00%> (+0.01%)`	⬆️
pkg/http/server.go	`64.62% <100.00%> (ø)`
pkg/actors/internal/placement.go	`87.56% <0.00%> (-1.56%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 050c71a...8761da3. Read the comment docs.

Signed-off-by: zhangchao <[email protected]> Signed-off-by: Taction <[email protected]>

Signed-off-by: Taction <[email protected]>

yaron2 · 2021-12-22T01:24:51Z

@Taction can you check why e2e tests are failing?

Taction · 2021-12-22T01:33:20Z

@Taction can you check why e2e tests are failing?

Sure, I'll look into it asap.

Signed-off-by: Taction <[email protected]>

* master: Charts - Supports costomize livenessProbe and readinessProbe Signed-off-by: Taction <[email protected]>

Taction · 2021-12-27T01:28:28Z

@yaron2 I have fixed the e2e. This code https://github.com/dapr/dapr/blob/master/pkg/channel/http/http_channel.go#L193 will format the URL to http://localhost:3000//method in some cases, if fasthttp don't parse it will cause an error.

The latest e2e is a connection error, I don't think it is related to this pr and will try to look into it later.

yaron2 · 2021-12-27T05:41:37Z

Thanks @Taction.

@artursouza can you please look at this and share your opinion about the possibility of this being a breaking change in the encoding parsing format?

* master: Saves 'describe' output of K8s resources for E2E tests. (dapr#4082) Fix flakiness on get metadata e2e. (dapr#4080) Return error instead of crashing when receiving grpc_proxy request on uninitialized daprd Fix actor reminder e2e tests + fixes to reminder partition (dapr#4021) Upgrading fasthttp to fix shutdown issue Fixed create timer duetime remove max graceful shutdown duration limit fix lint add more test case fix lint Make the grace period configurable Signed-off-by: Taction <[email protected]>

mcrio · 2022-01-13T20:39:55Z

Hi, do you think this can be merged for the next release : )

artursouza · 2022-01-13T20:45:03Z

Can we have E2E test for this?

Signed-off-by: zhangchao <[email protected]>

Taction · 2022-01-14T14:28:08Z

@artursouza e2e test added.

mcrio · 2022-02-03T10:44:29Z

Hi, do you think this can be merged for the next release? Cheers.

BrianMcBrayer · 2022-02-04T19:33:05Z

Tossing in my request for a merge too.

yaron2

LGTM

yaron2 · 2022-02-04T20:31:48Z

@artursouza please approve too. E2E are added.

yaron2 · 2022-02-08T05:42:38Z

Thanks for the fix @Taction!

Taction requested review from a team as code owners December 11, 2021 08:03

fix method invocation automatically unescape

ff666d0

Signed-off-by: zhangchao <[email protected]> Signed-off-by: Taction <[email protected]>

Taction force-pushed the fix_invocation_decode branch from 7f4ec5a to ff666d0 Compare December 11, 2021 08:11

daixiang0 and others added 2 commits December 13, 2021 09:55

Merge branch 'master' into fix_invocation_decode

59bb567

disable path normalize for direct call

284b480

Signed-off-by: Taction <[email protected]>

Taction mentioned this pull request Dec 13, 2021

REQUEST: New approver status for @Taction dapr/dapr dapr/community#118

Closed

4 tasks

yaron2 added 2 commits December 17, 2021 11:07

Merge branch 'master' into fix_invocation_decode

20525d7

Merge branch 'master' into fix_invocation_decode

bb70368

Merge branch 'master' into fix_invocation_decode

1c7add6

Taction force-pushed the fix_invocation_decode branch from 0a10093 to d5c97e8 Compare December 25, 2021 14:59

e2e test: fix path format

e8a58ef

Signed-off-by: Taction <[email protected]>

Taction force-pushed the fix_invocation_decode branch from d5c97e8 to e8a58ef Compare December 25, 2021 15:37

Merge branch 'master' into fix_invocation_decode

1aa2152

* master: Charts - Supports costomize livenessProbe and readinessProbe Signed-off-by: Taction <[email protected]>

yaron2 and others added 3 commits December 26, 2021 21:41

Merge branch 'master' into fix_invocation_decode

4ee77a3

Merge branch 'master' into fix_invocation_decode

7b00c6a

Merge branch 'master' into fix_invocation_decode

8e55dac

Taction added 2 commits January 14, 2022 19:53

add e2e tests

471d352

Signed-off-by: zhangchao <[email protected]>

fix e2e

71f61d7

Signed-off-by: zhangchao <[email protected]>

Taction force-pushed the fix_invocation_decode branch from fa56b46 to 71f61d7 Compare January 14, 2022 13:55

Merge branch 'master' into fix_invocation_decode

7807522

Merge branch 'master' into fix_invocation_decode

b75e186

mcrio mentioned this pull request Feb 4, 2022

Method invocation / Dapr automatically decodes %2f in URL #4008

Closed

Merge branch 'master' into fix_invocation_decode

0cfbda0

yaron2 approved these changes Feb 4, 2022

View reviewed changes

Merge branch 'master' into fix_invocation_decode

8761da3

artursouza approved these changes Feb 8, 2022

View reviewed changes

artursouza merged commit a73e4de into dapr:master Feb 8, 2022

Uh oh!

Conversation

Taction commented Dec 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issue reference

Checklist

Uh oh!

codecov Bot commented Dec 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

yaron2 commented Dec 22, 2021

Uh oh!

Taction commented Dec 22, 2021

Uh oh!

Taction commented Dec 27, 2021

Uh oh!

yaron2 commented Dec 27, 2021

Uh oh!

mcrio commented Jan 13, 2022

Uh oh!

artursouza commented Jan 13, 2022

Uh oh!

Taction commented Jan 14, 2022

Uh oh!

mcrio commented Feb 3, 2022

Uh oh!

BrianMcBrayer commented Feb 4, 2022

Uh oh!

yaron2 left a comment

Choose a reason for hiding this comment

Uh oh!

yaron2 commented Feb 4, 2022

Uh oh!

yaron2 commented Feb 8, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Taction commented Dec 11, 2021 •

edited

Loading

codecov Bot commented Dec 11, 2021 •

edited

Loading