By analyzing the blame information on this pull request, we identified @aprock, @satya164 and @christopherdro to be potential reviewers.

Build is failing from import com.facebook.react.modules.network.ForwardingCookieHandler;
Not sure why it says it test says doesn't exist.

/cc @mkonicek Thoughts?

@srikanthkh, Buck build is failing.
With Buck we are very strict with dependencies being explicit and not cyclic.
You've added a new dependency on com.facebook.react.modules.network package that has its own BUCK file.
You need to add it as dependency in the websocket BUCK, refer to failing CircleCI script to see how to run it locally.
Thanks :)

@srikanthkh updated the pull request.

@srikanthkh updated the pull request.

@christopherdro & @bestander I have made the suggested changes. Please let me know if anymore changes are required. Thank you.

@srikanthkh updated the pull request.

@srikanthkh updated the pull request.

@srikanthkh updated the pull request.

@srikanthkh updated the pull request.

code was failing one of my tests when cookie was null, So just fixed it and re-pushed the code. Thank you

@srikanthkh updated the pull request.

By analyzing the blame information on this pull request, we identified @satya164, @zxcpoiu and @mkonicek to be potential reviewers.

@satya164 would you mind taking a look at this pull request? It's been a while since the last commit was reviewed.

@christopherdro fine with shipping this?

@satya164 👍 LGTM!

@facebook-github-bot shipit

@srikanthkh The fix itself to include cookies in WebSocket requests is good, if you can send a new pull request we should get this in.

Note that the setDefaultOrigin method should be called getDefaultOrigin - it transforms a string into another string, but doesn't (and shouldn't) mutate anything. There's also an extra line after its Javadoc. I missed this in the code review, sorry about that!

@mkonicek Yea thats my bad for not looking it over properly. 😞

No worries @christopherdro! I totally understand there are a lot of pull requests to look at and the motivation to get this fix in quickly.

@srikanthkh Is there a method similar to getCookie already available elsewhere in the codebase, maybe in the networking module?

Re: setDefault was from a previous PR of mine. I can update that based on comments after we sort this one out.

👍 Thanks @christopherdro.

Really needing this

Any updates on this getting shipped soon?

It appears this PR was reverted but I am not sure why.

/cc @mkonicek

Cookies were being sent along in WebSockets with react-native 0.26 (OkHttp2) but not anymore with react-native 0.27 (OkHttp3).

@mkonicek: "I'll revert this now and let's do it again and properly." => Any ETA? I wish I could help but I know nothing about Android dev 😓

I think @mkonicek means:

• This PR should fix coding conventions accoring to @mkonicek 's comments in commits.
• This PR should at least have a test plan

Basically, the author should fulfill conditions above.
Given that the origin author may be busying and don't have time to do this,
if someone wanna help:

1. fork branch android-websocket-cookie from @srikanthkh 's repo( to keep the authorship )
2. help to fulfill conditions mentioned above
3. send a new PR which refers to this one

I can help the forking stuff and fix coding conventions, but I am not using this functionality, so don't know what is a proper test plan.
Are you interested to help the test plan part @antoinerousseau ?

@zxcpoiu well I'm not really good at tests and I'm not sure how to write a plan for this feature...

I suppose it would be like sending a first HTTP request which response includes a test cookie, and then connecting through a WebSocket at the same address than the HTTP request, and check on the server side if the test cookie is present... at least that's how I use it (my users are authenticated using an HttpOnly cookie set through an HTTPS request, and this auth must also be readable by the server when receiving WebSocket connections).

The test plan is described as:

Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes UI.
Make sure tests pass on both Travis and Circle CI.

@antoinerousseau has created a follow up PR #7989
Let's track there.