WAUrl decoding $+ when it should not (ZnUrl too)

Hi there,

When using a recent version of Zinc, the following functional tests fail when entering a text containing the character +
- http://localhost:8080/tests/functional/WAUrlEncodingFunctionalTest/
- http://localhost:8080/tests/functional-ajaxified/WAUrlEncodingFunctionalTest/

The $+ gets eaten and replaced with a space.

The problem surfaces only with recent versions of Zinc because the encoding was changed to be less strict (but apparently correct).
In essence, the $+ is a safe character in the query and path segments of the URI [2].

What happens is the following:
- original request comes in with $+ encoded as %2B: 'http://localhost:8080/foo%2Bbar' 
- after parsing the request, a ZnUrl will print this url as: 'http://localhost:8080/foo+bar'
- the WAUrl instance is parsed from printed ZnUrl in the ZnAdaptor, leading to: 'http://localhost:8080/foo%20bar'

Following a discussion with Sven on the Pharo mailinglist, Sven noted (I think correctly) that a $+ should only be regarded as a space in the context of a get/post with mime type application/x-www-form-urlencoded [1].

Both Zinc and WAUrl are, however, (I think) wrongly decoding a $+ when it's in the path and query segments of a URI.
Hence, the changes to Zinc are leading to these bugs in Seaside:

self assert: (ZnUrl fromString: (ZnUrl fromString: 'http://localhost/?param=foo%2Bbar') printString) printString = 'http://localhost/?param=foo%20bar'.
self assert: (WAUrl decodePercent: (ZnUrl fromString: 'http://localhost/?param=foo%2Bbar') printString) printString = 'http://localhost/?param=foo bar'

Both assertions above should actually be false, imho.

So, am I correct that we should fix WAUrl ?

Feedback appreciated!
Johan

[1] http://en.wikipedia.org/wiki/Percent-encoding#The_application.2Fx-www-form-urlencoded_type
[2] http://blog.lunatech.com/2009/02/03/what-every-web-developer-must-know-about-url-encoding#Thereservedcharactersaredifferentforeachpart_______________________________________________
seaside-dev mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev

Hi Johan,

On 14 Feb 2014, at 21:23, Johan Brichau <[hidden email]> wrote:

> Both assertions above should actually be false, imho.
>
> So, am I correct that we should fix WAUrl ?
>
> Feedback appreciated!

Just wait a bit, I am working on it, but I was not yet ready.

I also think that the following is wrong (and silly):

ZnZincServerAdaptor>>#requestUrlFor: aZincRequest
        ^ (WAUrl absolute: aZincRequest uri greaseString)
                decodedWith: self codec.

I hope to get back to you over the WE.

Sven
_______________________________________________
seaside-dev mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev

Hey Sven,

Very much appreciated.

Btw, in the blogpost I reference, I do read that $+ is only permitted in the path segment. In the query segment, the $+ should be encoded (see quote below).
In the Seaside tests (and my code fragments), it happens inside the query segment.

Quote from [2]:
For HTTP URLs, a space in a path fragment part has to be encoded to "%20" (not, absolutely not "+"), while the "+" character in the path fragment part can be left unencoded.
Now in the query part, spaces may be encoded to either "+" (for backwards compatibility: do not try to search for it in the URI standard) or "%20" while the "+" character (as a result of this ambiguity) has to be escaped to "%2B".

I had to read the blogpost again after writing the previous email to notice that. Whoever came to the idea of different reserved characters per segment really never actually implemented it :-/

Johan

On 14 Feb 2014, at 21:46, Sven Van Caekenberghe <[hidden email]> wrote:

_______________________________________________
seaside-dev mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev