Skip to content

Conversation

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

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

@codefromthecrypt
Copy link
Contributor

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
Contributor Author

@adriancole Yep, that worked

@codefromthecrypt
Copy link
Contributor

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
Contributor Author

Updated PR

Copy link
Contributor

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
Contributor

nit sweep, then squash commits, please!

Thanks for the help

@cloudbees-pull-request-builder

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

@cloudbees-pull-request-builder

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

@rodrigosaito
Copy link
Contributor Author

Updated @adriancole

@codefromthecrypt
Copy link
Contributor

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

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

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

@codefromthecrypt
Copy link
Contributor

6.0.2 thx

@cloudbees-pull-request-builder

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
Contributor Author

Done

@cloudbees-pull-request-builder

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
Contributor

cherry-picked into 6.x

@rodrigosaito rodrigosaito deleted the authorization_too_long branch December 3, 2013 09:59
@rodrigosaito
Copy link
Contributor Author

@adriancole When are you planning to release 6.0.2?

@codefromthecrypt
Copy link
Contributor

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