Skip to content
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

Add support for cookie based authentication in the initial handshake #4

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ctradu
Copy link

@ctradu ctradu commented May 23, 2014

Add a new function open/4, which is open/3 with a forth parameter, which becomes an additional header - Cookie.
Assume this authentication cookie was obtained in a previous step using a http auth mechanism.

My usage scenario is using ibrowse to do the http authentication step in the following manner:

Answer = ibrowse:send_req(Url, Headers, post, Body, Options),
case Answer of
        {ok, Status, ResponseHeaders, _ResponseBody} ->
            Cookies = get_cookies(ResponseHeaders),
            AuthCookie = find_cookie(AuthCookiePattern, Cookies),
            {ok, AuthCookie}
       %...
end
HandshakeRequest = wsock_handshake:open(Resource, Url, Port, AuthCookie)

@@ -357,6 +357,8 @@ process_frame(open_close, _, Frame, Message) ->
frame_to_complete_message(Frame, Message);
process_frame(open_continue, _, Frame, Message) ->
frame_for_fragmented_message(Frame, Message);
process_frame(continue, begin_message, Frame, Message) ->
Copy link
Owner

Choose a reason for hiding this comment

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

@ctradu hi. I know it has been a long time since you opened this pull request but now that I'm back I would like to know the reasoning behind this change.

How could you have a Frame that's supposed to continue (be part of an ongoing message) and at the same time consider it as part of a new message?

Copy link
Author

Choose a reason for hiding this comment

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

@madtrick Hello. It has been a long time since I needed those changes also.
I know I've written a websocket client using wsock and igel and I know that I've made some changes so that I could authenticate using a http cookie.

Currently (9 months later) I don't remember why I needed to change the process_frame function.
It may have been a hack or it may have been a legitimate change.
Is it possible from a wsock perspective that even the begining_message to be so fragmented that this change have a sense ?

Copy link
Owner

Choose a reason for hiding this comment

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

Is it possible from a wsock perspective that even the begining_message to be so fragmented that this change have a sense ?

@ctradu I don't think so.

When you call decode/2 with new data this data will be tagged with the atom begin_message. Further in the processing chain we call process_frames/3 with the frames extracted from that data. Then we analize frame by frame and we decide if we that frame belongs to a full message or a fragmented one. This means that some data that was initially tagged as begin_message can be now tagged as continue_message (this is the case for fragmented messages). This means that sucessive calls to process_frame/2 for frames belonging to fragmented messages will always have the type continue_message.

And when we call process_frame/2 with the first frame of data tagged as begin_message the contextualize_frame/1 function can't return continue for frames which are the first in a message since according to the rfc the fragmented messages must start with an opcode other than 0 which means that process_frame/4 will match for open_continue and not for continue.

I don't know if this clarifies your doubts. If not please ask again

@madtrick madtrick self-assigned this Mar 22, 2015
@madtrick
Copy link
Owner

@ctradu in any case I'll take a look to the cookie thing (even if it's to late for you)

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.

2 participants