Re: About URI with jsessionid and Request's URIs
Salut,
Bongjae Chang wrote:
> Hi Jeanfrancois,
>
> Later, about this issue, if you need more clarifying or I should do any actions,
>
> Please let me know.
Go ahead and commit.
Thanks!
-- Jeanfrancois
>
> 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
>>
>>
>>
>>