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

Adding percent encoding length check for Path.Uri #1553 #1590

Merged
merged 2 commits into from Dec 4, 2017

Conversation

@kencharos
Copy link
Contributor

kencharos commented Dec 3, 2017

  • Add percent encoding length check at Uri.decode. Bcause
    StringIndexOutOfBoundsException raises when Uri.Path call by illegal
    format(ex. %AA%, %AA%1).
  • Add test case
* Add percent encoding length check at Uri.decode. Bcause
StringIndexOutOfBoundsException raises when Uri.Path call by illegal
format(ex. %AA%, %AA%1).
* Add test case
@lightbend-cla-validator

This comment has been minimized.

Copy link

lightbend-cla-validator commented Dec 3, 2017

Hi @uls-kentaro-maeda,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement:

http://www.lightbend.com/contribute/cla

@akka-ci

This comment has been minimized.

Copy link
Collaborator

akka-ci commented Dec 3, 2017

Can one of the repo owners verify this patch?

@ktoso

This comment has been minimized.

Copy link
Member

ktoso commented Dec 3, 2017

Was nice hakking yesterday, thanks for completing the PR @kencharos !
We'll give it a look with the team and also check the performance impact with benchmarks :)

As the bot above explained we need a confirmation as explained in the above link via the CLA so we can accept a patch (it's basically about you confirming that you're the author of the source and are OK to contribute it).

@ktoso

This comment has been minimized.

Copy link
Member

ktoso commented Dec 3, 2017

OK TO TEST

@akka-ci

This comment has been minimized.

Copy link
Collaborator

akka-ci commented Dec 3, 2017

Test FAILed.

@kencharos

This comment has been minimized.

Copy link
Contributor Author

kencharos commented Dec 3, 2017

I did sign the CLA

@jrudolph jrudolph closed this Dec 4, 2017
@jrudolph jrudolph reopened this Dec 4, 2017
@jrudolph

This comment has been minimized.

Copy link
Member

jrudolph commented Dec 4, 2017

PLS BUILD

@akka-ci

This comment has been minimized.

Copy link
Collaborator

akka-ci commented Dec 4, 2017

Test FAILed.

@jrudolph

This comment has been minimized.

Copy link
Member

jrudolph commented Dec 4, 2017

PLS BUILD

@akka-ci

This comment has been minimized.

Copy link
Collaborator

akka-ci commented Dec 4, 2017

Test PASSed.

Copy link
Member

jrudolph left a comment

LGTM apart from small typo in comment.

@@ -597,6 +603,14 @@ class UriSpec extends WordSpec with Matchers {
" ^")
}

// illgal percent-encoding ends with %

This comment has been minimized.

Copy link
@jrudolph

jrudolph Dec 4, 2017

Member

Typo in comment ;)

@akka-ci akka-ci added validating tested and removed tested labels Dec 4, 2017
@akka-ci akka-ci removed the validating label Dec 4, 2017
@akka-ci

This comment has been minimized.

Copy link
Collaborator

akka-ci commented Dec 4, 2017

Test PASSed.

@raboof
raboof approved these changes Dec 4, 2017
@jrudolph

This comment has been minimized.

Copy link
Member

jrudolph commented Dec 4, 2017

Thanks for the quick fix, @ktoso. I keep forgetting that you can now edit right in place in the PR review making these kinds of edits quite convenient.

@jrudolph jrudolph merged commit 6a6ad30 into akka:master Dec 4, 2017
3 checks passed
3 checks passed
Jenkins PR Validation Test PASSed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
typesafe-cla-validator All users have signed the CLA
Details
@jrudolph

This comment has been minimized.

Copy link
Member

jrudolph commented Dec 4, 2017

Fixes #1553.

@kencharos

This comment has been minimized.

Copy link
Contributor Author

kencharos commented Dec 4, 2017

@jrudolph @raboof Thanks for reviewing.
@koto Thanks for fixing code and advising on last Saturday.
I'm very glad. I'm going to try other issue next time.

@ktoso

This comment has been minimized.

Copy link
Member

ktoso commented Dec 4, 2017

Thanks for the quick fix, @ktoso. I keep forgetting that you can now edit right in place in the PR review making these kinds of edits quite convenient.

Yeah :-) Indeed it's much quicker turnaround if it's just some small fix like here to fix it in-place than do a review cycle.

Thanks a lot @kencharos! ありがとうございました! ;-)
初めてプルリクエストおめでとうございます!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.