-
Notifications
You must be signed in to change notification settings - Fork 24.5k
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
OkHttp is more strict than other http libraries. #21231
OkHttp is more strict than other http libraries. #21231
Conversation
It crashes with IllegalStateException in case you pass wrong URL. It crashes if it meets unexpected symbols in the header name and value, while standard says it is not recommneded to use those symbols not that they are prohibited. The headers handing is a special use case as client might have an auth tokens in the header. In this case we want to get 401 error response from the server to find out that token is wrong. In case of the onerror client will continue to retry with an existing token. Verify that android app receives an error instead of crashing in case of following network request: "contacts/v2/users/" Verify the case with passing the headers with Release Notes [ANDROID] [BUGFIX] [Networking] - Passing invalid url not crashes the app instead onerror callback of HttpClient is called. Invalid symbols are stripped from the headers to allow http query to fail with 401 errorcode in case of the broken token. @MsSourceId: ede64de66f9bb19ff1253eabe872f7d3fd60c98c
This is a workaround for the problem. You can refer to this okhttp issue: The ugly part of this PR comes from the validation of the headers in okhttp. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hramos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
@hramos, did it merged to the master? |
I looked at this and it's unclear why it wasn't landed. It was approved back in October. I'm going to rebase and run it through our tests again, then land. |
This pull request was successfully merged by @dryganets in aad4dbb. When will my fix make it into a release? | Upcoming Releases |
Summary: It crashes with IllegalStateException in case you pass a wrong URL. It crashes if it meets unexpected symbols in the header name and value, while standard says it is not recommended to use those symbols not that they are prohibited. The headers handing is a special use case as a client might have an auth token in the header. In this case, we want to get 401 error response from the server to find out that token is wrong. In case of the onerror client will continue to retry with an existing token. [ANDROID][Fixed] - Networking: Passing invalid URL not crashes the app instead onerror callback of HttpClient is called. Invalid symbols are stripped from the headers to allow HTTP query to fail with 401 error code in case of the broken token. Pull Request resolved: #21231 Reviewed By: axe-fb Differential Revision: D10222129 Pulled By: hramos fbshipit-source-id: b23340692d0fb059a90e338fa85ad4d9612275f2
It crashes with IllegalStateException in case you pass a wrong URL.
It crashes if it meets unexpected symbols in the header name and value,
while standard says it is not recommended to use those symbols not that
they are prohibited.
The headers handing is a special use case as a client might have an auth token in the header. In this case, we want to get 401 error response
from the server to find out that token is wrong. In case of the onerror
client will continue to retry with an existing token.
Test Plan:
Verify that android app receives an error instead of crashing in case of following network request:
"contacts/v2/users/"
Verify the case with passing the headers with
Release Notes:
[ANDROID] [BUGFIX] [Networking] - Passing invalid URL not crashes the app instead onerror callback of HttpClient is called. Invalid symbols are stripped from the headers to allow HTTP query to fail with 401 error code in case of the broken token.