Skip to content

When User and/or Password are too long, then the Authorization Header is broken#93

Merged
codefromthecrypt merged 1 commit into
OpenFeign:masterfrom
rodrigosaito:authorization_too_long
Dec 3, 2013
Merged

When User and/or Password are too long, then the Authorization Header is broken#93
codefromthecrypt merged 1 commit into
OpenFeign:masterfrom
rodrigosaito:authorization_too_long

Conversation

@rodrigosaito

Copy link
Copy Markdown
Contributor

I created the test that shows the problem.

This is probably caused by the sun Base64Encoder: sun.misc.BASE64Encoder

I cat provide a PR with the solution, but wasn't sure about the best approach. What do you think? Implement a new Base64Encoder class, add something like commons-codec to the project, replace the \n in the string generated by the Base64Encoder or there is any other implementation of Base64 already in the project?

Thouhgts?

@cloudbees-pull-request-builder

Copy link
Copy Markdown

feign-pull-requests #122 FAILURE
Looks like there's a problem with this pull request

@codefromthecrypt

Copy link
Copy Markdown

you mind checking to see if this fixes it? If so, copy/paste is ok, as
base64 encoding doesn't fundamentally change often and better than managing
a new dep.

https://github.com/square/okhttp/blob/master/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/Base64.java

On Mon, Dec 2, 2013 at 4:03 PM, CloudBees pull request builder plugin <
notifications@github.com> wrote:

feign-pull-requests #122https://netflixoss.ci.cloudbees.com/job/feign-pull-requests/122/FAILURE
Looks like there's a problem with this pull request


Reply to this email directly or view it on GitHubhttps://github.com//pull/93#issuecomment-29671431
.

@rodrigosaito

Copy link
Copy Markdown
Contributor Author

@adriancole Yep, that worked

@codefromthecrypt

Copy link
Copy Markdown

great. make a copy and if you can get by with this being package private,
mark the class as such!

On Mon, Dec 2, 2013 at 4:24 PM, Rodrigo Saito notifications@github.comwrote:

@adriancole https://github.com/adriancole Yep, that worked


Reply to this email directly or view it on GitHubhttps://github.com//pull/93#issuecomment-29672549
.

@rodrigosaito

Copy link
Copy Markdown
Contributor Author

Updated PR

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix end of line

@codefromthecrypt

Copy link
Copy Markdown

nit sweep, then squash commits, please!

Thanks for the help

@cloudbees-pull-request-builder

Copy link
Copy Markdown

feign-pull-requests #123 SUCCESS
This pull request looks good

@cloudbees-pull-request-builder

Copy link
Copy Markdown

feign-pull-requests #124 SUCCESS
This pull request looks good

@rodrigosaito

Copy link
Copy Markdown
Contributor Author

Updated @adriancole

@codefromthecrypt

Copy link
Copy Markdown

sorry @rodrigosaito I forgot to ask you to update CHANGES. Can you do that and resquash?

@rodrigosaito

Copy link
Copy Markdown
Contributor Author

Sure, version 7.0.0? or inside 6.0.0?

On Mon, Dec 2, 2013 at 11:11 PM, Adrian Cole notifications@github.comwrote:

sorry @rodrigosaito https://github.com/rodrigosaito I forgot to ask you
to update CHANGES. Can you do that and resquash?


Reply to this email directly or view it on GitHubhttps://github.com//pull/93#issuecomment-29674994
.

Rodrigo Saito
gtalk: rodrigo.saito@gmail.com
skype: rodrigo.saito

@cloudbees-pull-request-builder

Copy link
Copy Markdown

feign-pull-requests #125 SUCCESS
This pull request looks good

@codefromthecrypt

Copy link
Copy Markdown

6.0.2 thx

@cloudbees-pull-request-builder

Copy link
Copy Markdown

feign-pull-requests #126 SUCCESS
This pull request looks good

… is broken because of sun Base64Encoder impl.

Changed for another Base64 implementation
@rodrigosaito

Copy link
Copy Markdown
Contributor Author

Done

@cloudbees-pull-request-builder

Copy link
Copy Markdown

feign-pull-requests #127 SUCCESS
This pull request looks good

codefromthecrypt pushed a commit that referenced this pull request Dec 3, 2013
When User and/or Password are too long, then the Authorization Header is broken
@codefromthecrypt codefromthecrypt merged commit dcc4c95 into OpenFeign:master Dec 3, 2013
@codefromthecrypt

Copy link
Copy Markdown

cherry-picked into 6.x

@rodrigosaito rodrigosaito deleted the authorization_too_long branch December 3, 2013 09:59
@rodrigosaito

Copy link
Copy Markdown
Contributor Author

@adriancole When are you planning to release 6.0.2?

@codefromthecrypt

Copy link
Copy Markdown

I don't have release karma. @allenxwang does :)

velo pushed a commit that referenced this pull request Oct 8, 2024
When User and/or Password are too long, then the Authorization Header is broken
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants