Hi Jeanfrancois,
Later, about this issue, if you need more clarifying or I should do any actions,
Please let me know.
Thanks!
--
Bongjae Chang
----- Original Message -----
From: "Jeanfrancois Arcand" <Jeanfrancois.Arcand_at_Sun.COM>
To: <dev_at_grizzly.dev.java.net>
Sent: Saturday, August 29, 2009 2:36 AM
Subject: Re: About URI with jsessionid and Request's URIs
> 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
>>>
>>>
>>>
>>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_grizzly.dev.java.net
> For additional commands, e-mail: dev-help_at_grizzly.dev.java.net
>
>
>
>