On 05/06/2014 14:51, Konstantin Kolinko wrote:
> 2014-06-05 17:34 GMT+04:00 <[email protected]>:
>> Author: markt
>> Date: Thu Jun 5 13:34:39 2014
>> New Revision: 1600653
>>
>> URL: http://svn.apache.org/r1600653
>> Log:
>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56573
>> Change the value returned by Session.getRequestURI() from the value obtained
>> from HttpServletRequest.getRequestURI() to the value obtained from
>> HttpServletRequest.getRequestURI() with the scheme changed to ws or wss as
>> appropriate. Note that the WebSocket Expert Group is expected to clarify the
>> expected behaviour for Session.getRequestURI() which may result in further
>> changes.
>>
>> Modified:
>> tomcat/tc7.0.x/trunk/ (props changed)
>>
>> tomcat/tc7.0.x/trunk/java/org/apache/tomcat/websocket/server/LocalStrings.properties
>>
>> tomcat/tc7.0.x/trunk/java/org/apache/tomcat/websocket/server/WsHandshakeRequest.java
>> tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
>>
>
> 1. The WebSocket API javadoc explicitly says that query string must be
> included in the result.
> (I added a comment to BZ issue with a link).
>
>> + // Based on request.getRequestURL() implementation
>> + StringBuilder sb = new StringBuilder();
>> + String scheme = request.getScheme();
>> + int port = request.getServerPort();
>> + if (port < 0)
>> + port = 80; // Work around java.net.URL bug
>> +
>> + if (scheme.equals("http")) {
>> + sb.append("ws");
>> + } else if (scheme.equals("https")) {
>> + sb.append("wss");
>> + } else {
>> + throw new IllegalArgumentException(
>> + sm.getString("wsHandshakeRequest.unknownScheme",
>> scheme));
>
> 2. I think "sb.append(scheme);" would be better here.
> Do we really need to throw IAE here?
Yes. It should only be possible for the scheme to be http or https so if
it is anything else something has gone very wrong.
Mark
>> + }
>> + sb.append("://");
>> + sb.append(request.getServerName());
>> + if ((scheme.equals("http") && (port != 80))
>> + || (scheme.equals("https") && (port != 443))) {
>
> 3. If "2." above is fixed, this condition shall be updated to uspport
> unknown schemas.
>
> if (! ((scheme.equals("http") && (port == 80))
> || (scheme.equals("https") && (port == 443)) ) ) {
>
>> + sb.append(':');
>> + sb.append(port);
>> }
>> + sb.append(request.getRequestURI());
>> try {
>> requestUri = new URI(sb.toString());
>> } catch (URISyntaxException e) {
>>
>
> Best regards,
> Konstantin Kolinko
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> For additional commands, e-mail: [email protected]
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
