-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3584/#review4537
-----------------------------------------------------------



/server.config.example
<https://reviews.apache.org/r/3584/#comment10127>

    Empty line



/src/org/waveprotocol/box/server/CoreSettings.java
<https://reviews.apache.org/r/3584/#comment10129>

    I think the preferred practice is that boolean variables start with "is, 
has" etc..
    Can we rename ENABLE_SSL to IS_SSL_ENABLED
    (also in other places)



/src/org/waveprotocol/box/server/rpc/ServerRpcProvider.java
<https://reviews.apache.org/r/3584/#comment10128>

    Probably we need to ensure consistent state
    here. 
    if (sslEnabled == true) {
      Preconditions.checkState(sslKeystorePassword != null && 
!sslKeystorePassword.isEmpty();
    Preconditions.checkState(
    }


- Yuri


On 2012-01-22 22:57:25, Ali Lown wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3584/
> -----------------------------------------------------------
> 
> (Updated 2012-01-22 22:57:25)
> 
> 
> Review request for wave, Michael MacFadden and Yuri Zelikov.
> 
> 
> Summary
> -------
> 
> Adds SSL capabilities as:
> 
> - Adds 3 new config options (globally enable ssl, keystore, keystore password)
> - Changes to use the SSLSelectChannelConnector if needed
> - Client-side websocket (wss/ws) detection based upon window.location.protocol
> - Updates socket.io.js (separate from this since it couldn't be diff'd in 
> this conventional means being a binary blob) (I couldn't find a nicer way of 
> doing this detection, which is the same as the above js, to check 
> window.location.protocol then decide whether to set 'this.options.base.secure 
> = true'
> As such, this has been attached to the 'Wave with HTTPS' mailing-list thread.
> 
> 
> Diffs
> -----
> 
>   /server-config.xml 1234540 
>   /server.config.example 1234540 
>   /src/org/waveprotocol/box/server/CoreSettings.java 1234540 
>   
> /src/org/waveprotocol/box/server/robots/operations/GravatarProfilesFetcher.java
>  1234540 
>   /src/org/waveprotocol/box/server/rpc/ServerRpcProvider.java 1234540 
>   /src/org/waveprotocol/box/webclient/client/WebClient.java 1234540 
> 
> Diff: https://reviews.apache.org/r/3584/diff
> 
> 
> Testing
> -------
> 
> _Correctly_ setting up the keystore is quite difficult if you want to use 
> existing certificates, but once setup:
> - Tested with enableSsl = true -> works
> - Tested with enableSsl = false -> works
> 
> 
> Thanks,
> 
> Ali
> 
>

Reply via email to