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

Issue in SSL implementation : protocole ws:// is always use in Draft_76.java #294

Closed
snooze67 opened this issue Nov 25, 2014 · 6 comments
Closed

Comments

@snooze67
Copy link

hi,

First thank's a lot for this project and library.
I test the ssl implementation and got some issues with Safari on windows and ipad (chrome and safari)

I found why.
In Draft_76.java on the method postProcessHandshakeResponseAsServer

There is always the ws:// protocol.

But on the ssl we need to specify wss:// protocol instead

Don't know how to fix it properly.

So i modify the code that way, passwing the fact to use ws or wss in the ressourceDescriptor

public HandshakeBuilder postProcessHandshakeResponseAsServer( ClientHandshake request, ServerHandshakeBuilder response ) throws InvalidHandshakeException {
                String protocole = "ws://";    

                if(request.getResourceDescriptor().equals("/wss"))
                    protocole = "wss://";

                response.setHttpStatusMessage( "WebSocket Protocol Handshake" );
        response.put( "Upgrade", "WebSocket" );
        response.put( "Connection", request.getFieldValue( "Connection" ) ); // to respond to a Connection keep alive
        response.put( "Sec-WebSocket-Origin", request.getFieldValue( "Origin" ) );

        String location = protocole + request.getFieldValue( "Host" ) + request.getResourceDescriptor();
        response.put( "Sec-WebSocket-Location", location );
        String key1 = request.getFieldValue( "Sec-WebSocket-Key1" );
        String key2 = request.getFieldValue( "Sec-WebSocket-Key2" );
        byte[] key3 = request.getContent();
        if( key1 == null || key2 == null || key3 == null || key3.length != 8 ) {
            throw new InvalidHandshakeException( "Bad keys" );
        }
        response.setContent( createChallenge( key1, key2, key3 ) );
        return response;
    }
@BrushfireDigitalServices
Copy link
Collaborator

@snooze67 Thank you for pointing out this error! We need to get it fixed without introducing other bugs. Does someone else in the project have more direct experience implementing the drafts?

@BrushfireDigitalServices
Copy link
Collaborator

Calling for input from the community:

Please review this proposed fix to the implementation of Draft76. Even if you don't have direct experience in this area of the codebase, the more eyes and opinions look at proposed patches, the better.

If it all looks good, we still need to get unit tests written to make future contribution and debugging easier, as per #318.

As for me, I don't see any glaring issues, but am new to SSL-related projects. Am hesitant to commit without further input.

@marci4 marci4 modified the milestones: 1.3.4, Release 1.4.0 Apr 26, 2017
@zhoulifu
Copy link
Contributor

zhoulifu commented May 11, 2017

getResourceDescriptor()will return the request URI according to its JavaDoc, it knows nothing about the protocol, so does classDraft. I think there should be a filed inClientHandshaketo hold the scheme of the handshake request.

@marci4 WDYT

@marci4
Copy link
Collaborator

marci4 commented May 11, 2017

hey @zhoulifu,

right now I am playing with the thought of deprecating Draft_10, Draft_75 and Draft_76 and not supporting them any more.
In my opinion this should have been done a long time ago.

Greetings
marci4

@zhoulifu
Copy link
Contributor

Sounds great, I was going to ask whether this issue should be fixed or not since Draft75/76 was marked deprecated in wiki =D

@marci4
Copy link
Collaborator

marci4 commented May 11, 2017

This draft is now deprecated, see #478.

Closing this issue.
Greetings
marci4

@marci4 marci4 closed this as completed May 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants