Skip to content

It would be nice if the request/response body did not assume utf-8 strings#61

Closed
stephenc wants to merge 2 commits intoscribejava:masterfrom
stephenc:master
Closed

It would be nice if the request/response body did not assume utf-8 strings#61
stephenc wants to merge 2 commits intoscribejava:masterfrom
stephenc:master

Conversation

@stephenc
Copy link

@stephenc stephenc commented Feb 2, 2011

The following hacky change does some work to allow access to the raw bytes as not every byte array can be converted into a valid utf-8 string

@fernandezpablo85
Copy link
Collaborator

This implies changing quite a few public methods, and breaks backwards compatibility...

As it is, I don't think we gain too much from this, there are people using scribe in China and Korea so the encoding stuff must be good enough.

But, good enough is sometimes not enough and it's indeed something I'd like to include in Scribe :) I just don't want to break public methods yet (until 1.2 at least)... let me think about this for a while.

Thank you very much for contributing!

@stephenc
Copy link
Author

stephenc commented Feb 2, 2011

I think I managed to make the changes without affecting public methods ;-) only extending and adding additional methods to allow the byte[] to be accessed.

@stephenc
Copy link
Author

stephenc commented Feb 2, 2011

ok getBodyContents() is public... I missed that... I'll update with a getBodyContentsAsByteArray() version

@fernandezpablo85
Copy link
Collaborator

Also StreamUtils#getStreamContents signature changed

@stephenc
Copy link
Author

stephenc commented Feb 2, 2011

yes but that is only used in one place in your code. ;-) Again it could be worked around, but given that it's current behaviour is not symmetric with respect to the request's complete lack of charset specification, you could probably get away with changing it (as I suspect nobody would be relying on it) ;-)

@fernandezpablo85
Copy link
Collaborator

Once a method is public it cannot be changed unless it's a minor version (e.g. 1.2). I really don't know what people are doing with that public method, so I can't change it

@fernandezpablo85
Copy link
Collaborator

Closing

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants