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