Salut,
Bongjae Chang wrote:
> Hi Jeanfrancois,
>
> Jeanfrancois wrote:
>> I think your proposed patch will not be needed if we do it like that.
>> What do you think?
>
> No, the problem was still reproduced when I applied your patch and tested it.
>
> Maybe there are two problems.
>
> It seems that your idea is that the decodedURI should be rolled back.
>
> But the decodedURI was not rolled back completely because of the following reason.
>
> When the decodedURI is once decoded by HttpRequestURIDecoder, decodedURI type is changed to T_CHARS.
>
> If we restored the original decodedURI with decodedURI.setBytes(), the type is changed back to T_BYTES.
>
> Then, decodedURI.getByteChunk() is equal to "/examples/index.html;jsessionid=123" and decodedURI.getCharChunk() is still equal to "/examples/index.html" and decodedURI.toString() is equal to "/examples/index.html;jsessionid=123". (I think that this is not safe because the decodedURI MessageBytes has two different values)
Agree. I've missed that one.
>
> Problem1) Anyway, GrizzlyRequest#parseSessionId() didn't uses decodedURI.getByteChunk() but decodedURI.getCharChunk(). So, the session id can't be parsed.
Yes and we seems the suffer the issue in GlassFish v3 as well, but I
haven't add a chance to look at it. This helps me a lot. I would think
your patch looks good then. You can commit it.
>
> Problem2) Even though GrizzlyRequest#parseSessionId() uses decodedURI.getByteChunk(), the requestURI is equal to "/examples/index.html;jsessionid=123". Then StaticResourcesAdapter will not service resources correctly because "/examples/index.html;jsessionid=123" is not existed.
>
> What do you think?
Thanks for clarifying. I've missed the type change completely.
--Jeanfrancois
> --
> Bongjae Chang
>
>
> ----- Original Message -----
> From: "Jeanfrancois Arcand" <Jeanfrancois.Arcand_at_Sun.COM>
> To: <dev_at_grizzly.dev.java.net>
> Sent: Friday, August 28, 2009 1:49 AM
> Subject: Re: About URI with jsessionid and Request's URIs
>
>
>> Salut,
>>
>> first, It's really nice you work on this component. I really want to
>> make it Sevlet 2.5 compliant and your work helps a lot.
>>
>> Bongjae Chang wrote:
>>> Hi,
>>>
>>> When I tested the request URI with jsessionid, some problems occurred.
>>>
>>> When I requested "/examples/index.html;jsessionid=123", it seemed that
>>> "index.html" was not served.
>>>
>>> When I debugged, StaticResourcesAdapter tried to service the resource
>>> with "/examples/index.html;jsessionid=123" and "404 not found" returned.
>>>
>>> So I reviewed related codes(GrizzlyRequest.java, GrizzlyAdaperChain.java
>>> and Request.java) and I had some questions.
>>>
>>>
>>>
>>> com.sun.grizzly.tcp.Request.java has the decodedURI, the requestURI and
>>> etc....
>>>
>>> It seems that the decodedURI should not contain ";jsessionid=123" and it
>>> should become to be only "/examples/index.html" because of the following
>>> logic.
>>>
>>> I can see the following logic in GrizzlyAdapterChain#service()
>>> ---
>>> // Map the request without any trailling.
>>> ByteChunk uriBB = decodedURI.getByteChunk();
>>> int semicolon = uriBB.indexOf(';', 0);
>>> if (semicolon > 0 ) {
>>> decodedURI.setBytes(uriBB.getBuffer(), uriBB.getStart(), semicolon);
>>> }
>>> ---
>>>
>>> Above logic removed ";jsessionid=123" from original URI. I understood it.
>>>
>>> But, GrizzlyRequest#parseSessionId() used the decodedURI which didn't
>>> contain ";jsessionid", so session id was never parsed.
>>>
>>> See the following code.
>>>
>>> In GrizzlyRequest.java
>>> ---
>>> protected void parseSessionId() {
>>> CharChunk uriCC = request.decodedURI().getCharChunk();
>>> int semicolon = uriCC.indexOfmatch, 0, match.length(), 0);
>>> *// semicolon is always equal to -1!!*
>>> if(semicolon > 0 ) {
>>> ...
>>> }
>>> }
>>> ---
>>>
>>> And StaticResourcesAdapter didn't used the decodedURI but the
>>> requestURI, so the request got "404 not found".
>>>
>>> So,
>>>
>>> Q1) I don't know whether the requestURI should contain ";jsessionid" or
>>> not when it is used for being serviced.
>> Yes it should. I think the GrizzlyAdapterChain must be changed to:
>>
>>> Index: utils/src/main/java/com/sun/grizzly/tcp/http11/GrizzlyAdapterChain.java
>>> ===================================================================
>>> --- utils/src/main/java/com/sun/grizzly/tcp/http11/GrizzlyAdapterChain.java (revision 3544)
>>> +++ utils/src/main/java/com/sun/grizzly/tcp/http11/GrizzlyAdapterChain.java (working copy)
>>> @@ -155,6 +155,8 @@
>>>
>>> // Map the request without any trailling.
>>> ByteChunk uriBB = decodedURI.getByteChunk();
>>> + int start = uriBB.getStart();
>>> + int end = uriBB.getEnd();
>>> int semicolon = uriBB.indexOf(';', 0);
>>> if (semicolon > 0) {
>>> decodedURI.setBytes(uriBB.getBuffer(), uriBB.getStart(), semicolon);
>>> @@ -167,7 +169,10 @@
>>>
>>> // Map the request to its Adapter
>>> mapper.map(req.serverName(), decodedURI, mappingData);
>>> -
>>> +
>>> + if (semicolon > 0 && end != 0) {
>>> + decodedURI.setBytes(uriBB.getBuffer(), start, end);
>>> + }
>>> GrizzlyAdapter adapter = null;
>>> if (mappingData.context != null && mappingData.context instanceof GrizzlyAdapter) {
>>> if (mappingData.wrapper != null) {
>> I think your proposed patch will not be needed if we do it like that.
>> What do you think?
>>
>> A+
>>
>> --jeanfrancois
>>
>>
>>
>>>
>>> Q2) I don't know if parseSessionId() should use the decodedURI or the
>>> requestURI.
>>>
>>> Experimentally, I modified some codes like this.
>>>
>>> About Q1), I thought that the requestURI should not contain
>>> ";jsessionid" like the decodedURI when the adapter used it for servicing
>>> resources.
>>>
>>> About Q2), I thought that parseSessionId() should use the requestURI
>>> which contained ";jsessionid". (I thought that ";jsessionid" should be
>>> removed from the original requestURI after parseSessionId()'s call).
>>>
>>> Here is my proposed patch.
>>>
>>> Index: com/sun/grizzly/tcp/http11/GrizzlyRequest.java
>>> ===================================================================
>>> --- com/sun/grizzly/tcp/http11/GrizzlyRequest.java (revision 3556)
>>> +++ com/sun/grizzly/tcp/http11/GrizzlyRequest.java (working copy)
>>> @@ -2284,23 +2284,23 @@
>>> */
>>> protected void parseSessionId() {
>>>
>>> - CharChunk uriCC = request.decodedURI().getCharChunk();
>>> - int semicolon = uriCC.indexOf(match, 0, match.length(), 0);
>>> + ByteChunk uriBC = request.requestURI().getByteChunk();
>>> + int semicolon = uriBC.indexOf(match, 0, match.length(), 0);
>>>
>>> if (semicolon > 0) {
>>>
>>> // Parse session ID, and extract it from the decoded
>>> request URI
>>> - int start = uriCC.getStart();
>>> - int end = uriCC.getEnd();
>>> + int start = uriBC.getStart();
>>> + int end = uriBC.getEnd();
>>>
>>> int sessionIdStart = start + semicolon + match.length();
>>> - int semicolon2 = uriCC.indexOf(';', sessionIdStart);
>>> + int semicolon2 = uriBC.indexOf(';', sessionIdStart);
>>> String sessionId = null;
>>> if (semicolon2 >= 0) {
>>> - sessionId = new String(uriCC.getBuffer(), sessionIdStart,
>>> + sessionId = new String(uriBC.getBuffer(), sessionIdStart,
>>> semicolon2 - semicolon -
>>> match.length());
>>> } else {
>>> - sessionId = new String(uriCC.getBuffer(), sessionIdStart,
>>> + sessionId = new String(uriBC.getBuffer(), sessionIdStart,
>>> end - sessionIdStart);
>>> }
>>> int jrouteIndex = sessionId.lastIndexOf(':');
>>> @@ -2331,7 +2331,8 @@
>>>
>>> int start, end, sessionIdStart, semicolon, semicolon2;
>>>
>>> - ByteChunk uriBC = request.requestURI().getByteChunk();
>>> + MessageBytes requestURI = request.requestURI();
>>> + ByteChunk uriBC = requestURI.getByteChunk();
>>> start = uriBC.getStart();
>>> end = uriBC.getEnd();
>>> semicolon = uriBC.indexOf(match, 0, match.length(), 0);
>>> @@ -2340,12 +2341,13 @@
>>> sessionIdStart = start + semicolon;
>>> semicolon2 = uriBC.indexOf
>>> (';', semicolon + match.length());
>>> - uriBC.setEnd(start + semicolon);
>>> byte[] buf = uriBC.getBuffer();
>>> if (semicolon2 >= 0) {
>>> - System.arraycopy(buf, start + semicolon2, buf, start +
>>> semicolon, end - start - semicolon2);
>>> - uriBC.setBytes(buf, start, semicolon
>>> + System.arraycopy(buf, start + semicolon2, buf,
>>> sessionIdStart, end - start - semicolon2);
>>> + requestURI.setBytes(buf, start, semicolon
>>> + (end - start - semicolon2));
>>> + } else {
>>> + requestURI.setBytes(buf, start, semicolon);
>>> }
>>> }
>>> }
>>>
>>> I think that the patch can be changed in according to Q1) and Q2)'s answer.
>>>
>>> Please review it and advice me.
>>>
>>> Thanks.
>>>
>>> --
>>> Bongjae Chang
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe_at_grizzly.dev.java.net
>> For additional commands, e-mail: dev-help_at_grizzly.dev.java.net
>>
>>
>>
>>